Messages in this thread | | | Subject | Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted | From | Jason Low <> | Date | Mon, 28 Apr 2014 11:04:34 -0700 |
| |
On Sun, 2014-04-27 at 14:01 +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;
Hi Preeti,
If jiffies is after this_rq->next_balance, doesn't that mean that it's actually due for a periodic balance and we wouldn't need to modify it? In rebalance_domains(), we do load_balance if time_after_eq(jiffies, sd->last_balance + interval).
> > Besides this: > Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Thanks for the review.
| |