Messages in this thread | | | Date | Tue, 29 Apr 2014 08:40:04 +0530 | From | Preeti U Murthy <> | Subject | Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted |
| |
On 04/28/2014 02:54 PM, Peter Zijlstra wrote: > 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. > Hmm this makes sense. Thanks!
Regards Preeti U Murthy
| |