Messages in this thread | | | From | Wanpeng Li <> | Date | Tue, 7 Feb 2017 06:06:06 +0800 | Subject | Re: [PATCH] sched/fair: Fix the nohz.next_balance update mess |
| |
2017-02-06 21:23 GMT+08:00 Vincent Guittot <vincent.guittot@linaro.org>: > On 6 February 2017 at 09:33, Wanpeng Li <kernellwp@gmail.com> wrote: >> Hi Vincent, >> 2017-02-06 16:07 GMT+08:00 Vincent Guittot <vincent.guittot@linaro.org>: >>> Hi Wanpeng >>> >>> On 5 February 2017 at 10:57, Wanpeng Li <kernellwp@gmail.com> wrote: >>>> From: Wanpeng Li <wanpeng.li@hotmail.com> >>>> >>>> The commit: >>>> c5afb6a87f2 ("sched/fair: Fix nohz.next_balance update") >>>> >>>> intends to update nohz.next_balance in two steps. >>>> >>>> 1) The ILB CPU utilizes next_balance variable in nohz_idle_balance() >>>> to gather the shortest next balance of other idle CPUs before >>>> updating nohz.next_balance. >>>> 2) The ILB CPU updates the nohz.next_balance according to its own >>>> next_balance after load balance on behalf of other idle CPUs. >>>> >>>> However, there is a mess which breaks the original intention of the >>> >>> Have you got details of the mess that this generates ? >>> >>>> first step, every idle CPUs update nohz.next_balance during ILB CPU >>>> on behalf of them to do load balance, and then the ILB CPU utilizes >>>> next_balance variable in nohz_idle_balance() to gather the shortest >>>> next balance of other idle CPUs before updating nohz.next_balance. >>>> >>>> This patch fixes it by don't update nohz.next_balance for other idle >>>> CPUs when ILB CPU on behalf of them to do load balance. >>> >>> But how do you take into account the next balance of other idle CPUs ? >> >> The step 1) which I describe above for your original commit takes it >> into account. In addition, please refers to the comments which you >> added(rebalance_domains()) in the original commit: > > yes sorry I mixed rebalance_domains and nohz_idle_balance code > > But are you sure that this additional condition will change anything ? > > When an ILB is triggered, it means that nohz.next_balance is before jiffies. > Then, for all Idle CPUs (except the ILB CPU), the rq->next_balance > will be for sure after nohz.next_balance once we have finished the > for_each_domain loop of rebalance_domain() so it can't trig > nohz.next_balance = rq->next_balance and the current condition if > fine.
Oh, I miss it. Thanks for your pointing out.
Regards, Wanpeng Li
| |