lkml.org 
[lkml]   [2014]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v4 01/12] sched: fix imbalance flag reset
Hi Vincent,
On 11/25/14, 5:04 PM, Vincent Guittot wrote:
> On 25 November 2014 at 00:47, Wanpeng Li <kernellwp@gmail.com> wrote:
>> Hi Vincent,
>> On 7/29/14, 1:51 AM, Vincent Guittot wrote:
>>> The imbalance flag can stay set whereas there is no imbalance.
>>>
>>> Let assume that we have 3 tasks that run on a dual cores /dual cluster
>>> system.
>>> We will have some idle load balance which are triggered during tick.
>>> Unfortunately, the tick is also used to queue background work so we can
>>> reach
>>> the situation where short work has been queued on a CPU which already runs
>>> a
>>> task. The load balance will detect this imbalance (2 tasks on 1 CPU and an
>>> idle
>>> CPU) and will try to pull the waiting task on the idle CPU. The waiting
>>> task is
>>> a worker thread that is pinned on a CPU so an imbalance due to pinned task
>>> is
>>> detected and the imbalance flag is set.
>>> Then, we will not be able to clear the flag because we have at most 1 task
>>> on
>>> each CPU but the imbalance flag will trig to useless active load balance
>>> between the idle CPU and the busy CPU.
>>>
>>> We need to reset of the imbalance flag as soon as we have reached a
>>> balanced
>>> state. If all tasks are pinned, we don't consider that as a balanced state
>>> and
>>> let the imbalance flag set.
>>>
>>> Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>>> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>>> ---
>>> kernel/sched/fair.c | 23 +++++++++++++++++++----
>>> 1 file changed, 19 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index 923fe32..7eb9126 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -6672,10 +6672,8 @@ static int load_balance(int this_cpu, struct rq
>>> *this_rq,
>>> if (sd_parent) {
>>> int *group_imbalance =
>>> &sd_parent->groups->sgc->imbalance;
>>> - if ((env.flags & LBF_SOME_PINNED) && env.imbalance
>>>> 0) {
>>> + if ((env.flags & LBF_SOME_PINNED) && env.imbalance
>>>> 0)
>>> *group_imbalance = 1;
>>> - } else if (*group_imbalance)
>>> - *group_imbalance = 0;
>>
>> As you mentioned above " We need to reset of the imbalance flag as soon as
>> we have reached a balanced state. " I think the codes before your patch have
>> already do this, where I miss? Great thanks for your patient. ;-)
> The previous code was called only when busiest->nr_running > 1. The
> background activity will be on the rq only 1 tick per few seconds and
> we will set qroup_imbalance when the background activity is on the rq.
> Then, during the next load balances, the qroup_imbalance is still set
> but we can't clear qroup_imbalance because we have only 1 task per rq

There is no load balance I think since busiest->nr_running > 1 is not
true even if the patch is not applied.

Regards,
Wanpeng Li

>
> Regards,
> Vincent
>
>> Regards,
>> Wanpeng Li
>>
>>
>>> }
>>> /* All tasks on this runqueue were pinned by CPU affinity
>>> */
>>> @@ -6686,7 +6684,7 @@ static int load_balance(int this_cpu, struct rq
>>> *this_rq,
>>> env.loop_break = sched_nr_migrate_break;
>>> goto redo;
>>> }
>>> - goto out_balanced;
>>> + goto out_all_pinned;
>>> }
>>> }
>>> @@ -6760,6 +6758,23 @@ static int load_balance(int this_cpu, struct rq
>>> *this_rq,
>>> goto out;
>>> out_balanced:
>>> + /*
>>> + * We reach balance although we may have faced some affinity
>>> + * constraints. Clear the imbalance flag if it was set.
>>> + */
>>> + if (sd_parent) {
>>> + int *group_imbalance = &sd_parent->groups->sgc->imbalance;
>>> +
>>> + if (*group_imbalance)
>>> + *group_imbalance = 0;
>>> + }
>>> +
>>> +out_all_pinned:
>>> + /*
>>> + * We reach balance because all tasks are pinned at this level so
>>> + * we can't migrate them. Let the imbalance flag set so parent
>>> level
>>> + * can try to migrate them.
>>> + */
>>> schedstat_inc(sd, lb_balanced[idle]);
>>> sd->nr_balance_failed = 0;
>>



\
 
 \ /
  Last update: 2014-11-25 11:41    [W:0.043 / U:0.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site