Messages in this thread | | | Date | Mon, 28 Apr 2014 11:24:03 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted |
| |
On Sun, Apr 27, 2014 at 02:01:45PM +0530, Preeti Murthy wrote: > Hi Jason, Peter, > > The below patch looks good to me except for one point. > > In idle_balance() the below code snippet does not look right: > > - if (pulled_task || time_after(jiffies, this_rq->next_balance)) { > - /* > - * We are going idle. next_balance may be set based on > - * a busy processor. So reset next_balance. > - */ > +out: > + /* Move the next balance forward */ > + if (time_after(this_rq->next_balance, next_balance)) > this_rq->next_balance = next_balance; > - } > > By not checking this_rq->next_balance against jiffies, > we might end up not updating this parameter when it > has expired. > > So shouldn't it be: > > if (time_after(jiffies, this_rq->next_balance) || > time_after(this_rq->next_balance, next_balance)) > this_rq->next_balance = next_balance;
So the reason I didn't do that is that nothing else does that either. Also, note that the value we set rq->next_balance to might itself already be expired. There is no guarantee that last_balance + interval is in the future.
| |