Re: [PATCH] sched/fair: vruntime should normalize when switching from fair

From: Joel Fernandes
Date: Fri Sep 28 2018 - 12:45:48 EST


On Fri, Sep 28, 2018 at 9:10 AM, 'Steve Muckle' via kernel-team
<kernel-team@xxxxxxxxxxx> wrote:
> On 09/27/2018 05:43 PM, Wanpeng Li wrote:
>>>>
>>>> On your CPU4:
>>>> scheduler_ipi()
>>>> -> sched_ttwu_pending()
>>>> -> ttwu_do_activate() => p->sched_remote_wakeup should be
>>>> false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not
>>>> -> ttwu_activate()
>>>> -> activate_task()
>>>> -> enqueue_task()
>>>> -> enqueue_task_fair()
>>>> -> enqueue_entity()
>>>> bool renorm = !(flags &
>>>> ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
>>>> so renorm is false in enqueue_entity(), why you mentioned that the
>>>> cfs_rq->min_vruntime is still added to the se->vruntime in
>>>> enqueue_task_fair()?
>>>
>>>
>>> Maybe this is a misunderstanding on my side but didn't you asked me to
>>> '... Could you point out when the fair rq's min_vruntime is added to the
>>> task's vruntime in your *later* scenario? ...'
>>
>>
>> Yeah, if the calltrace above and my analysis is correct, then the fair
>> rq's min_vruntime will not be added to the task's vruntime in your
>> *later* scenario, which means that your patch is not necessary.
>
>
> In the scenario I observed, the task is not waking - it is running and being
> deboosted from priority inheritance, transitioning from RT to CFS.
>
> Dietmar and I both were able to reproduce the issue with the testcase I
> posted earlier in this thread.

Can this issue still show up say if the wake up was not remote? Say
the task was locally awakened. In that case do we still need to check
the class in vruntime_normalized like John was doing? Just want to
make sure we caught all scenarios.

- Joel