All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Peter Zijlstra <peterz@infradead.org>, Steve Muckle <smuckle@google.com>
Cc: Miguel de Dios <migueldedios@google.com>,
	Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org, kernel-team@android.com,
	Todd Kjos <tkjos@google.com>, Paul Turner <pjt@google.com>,
	Quentin Perret <quentin.perret@arm.com>,
	Patrick Bellasi <Patrick.Bellasi@arm.com>,
	Chris Redpath <Chris.Redpath@arm.com>,
	Morten Rasmussen <Morten.Rasmussen@arm.com>,
	John Dias <joaodias@google.com>
Subject: Re: [PATCH] sched/fair: vruntime should normalize when switching from fair
Date: Tue, 28 Aug 2018 15:53:53 +0100	[thread overview]
Message-ID: <2ed346fa-dbe8-4928-928b-a34338b2d8c9@arm.com> (raw)
In-Reply-To: <20180827111458.GB24124@hirez.programming.kicks-ass.net>

On 08/27/2018 12:14 PM, Peter Zijlstra wrote:
> On Fri, Aug 24, 2018 at 02:24:48PM -0700, Steve Muckle wrote:
>> On 08/24/2018 02:47 AM, Peter Zijlstra wrote:
>>>>> On 08/17/2018 11:27 AM, Steve Muckle wrote:
>>>
>>>>>> When rt_mutex_setprio changes a task's scheduling class to RT,
>>>>>> we're seeing cases where the task's vruntime is not updated
>>>>>> correctly upon return to the fair class.
>>>
>>>>>> Specifically, the following is being observed:
>>>>>> - task is deactivated while still in the fair class
>>>>>> - task is boosted to RT via rt_mutex_setprio, which changes
>>>>>>      the task to RT and calls check_class_changed.
>>>>>> - check_class_changed leads to detach_task_cfs_rq, at which point
>>>>>>      the vruntime_normalized check sees that the task's state is TASK_WAKING,
>>>>>>      which results in skipping the subtraction of the rq's min_vruntime
>>>>>>      from the task's vruntime
>>>>>> - later, when the prio is deboosted and the task is moved back
>>>>>>      to the fair class, the fair rq's min_vruntime is added to
>>>>>>      the task's vruntime, even though it wasn't subtracted earlier.
>>>
>>> I'm thinking that is an incomplete scenario; where do we get to
>>> TASK_WAKING.
>>
>> Yes there's a missing bit of context here at the beginning that the task to
>> be boosted had already been put into TASK_WAKING.
> 
> See, I'm confused...
> 
> The only time TASK_WAKING is visible, is if we've done a remote wakeup
> and it's 'stuck' on the remote wake_list. And in that case we've done
> migrate_task_rq_fair() on it.
> 
> So by the time either rt_mutex_setprio() or __sched_setscheduler() get
> to calling check_class_changed(), under both pi_lock and rq->lock, the
> vruntime_normalized() thing should be right.
> 
> So please detail the exact scenario. Because I'm not seeing it.

Using Steve's test program (https://lkml.org/lkml/2018/8/24/686) I see the
issue but only if the two tasks (rt_task, fair_task) run on 2 cpus which
don't share LLC (e.g. CPU0 and CPU4 on hikey960).

So the wakeup goes the TTWU_QUEUE && !share_cache (ttwu_queue_remote) path.

...
rt_task-3579  [000] 35.391573: sched_waking:     comm=fair_task pid=3580 prio=120 target_cpu=004
rt_task-3579  [000] 35.391580: bprint:           try_to_wake_up: try_to_wake_up: task=fair_task pid=3580 task_cpu(p)=4 p->on_rq=0
rt_task-3579  [000] 35.391584: bprint:           try_to_wake_up: ttwu_queue: task=fair_task pid=3580
rt_task-3579  [000] 35.391588: bprint:           try_to_wake_up: ttwu_queue_remote: task=fair_task pid=3580
rt_task-3579  [000] 35.391591: bprint:           try_to_wake_up: ttwu_queue_remote: cpu=4 smp_send_reschedule
rt_task-3579  [000] 35.391627: sched_pi_setprio: comm=fair_task pid=3580 oldprio=120 newprio=19
rt_task-3579  [000] 35.391635: bprint:           rt_mutex_setprio: task=fair_task pid=3580 prio=120->19 queued=0 running=0 state=0x200 vruntime=46922871 cpu=4 cfs_rq->min_vruntime=7807420844
rt_task-3579  [000] 35.391641: bprint:           rt_mutex_setprio: p->prio set: task=fair_task pid=3580 prio=19 queued=0 running=0 state=0x200 vruntime=46922871
rt_task-3579  [000] 35.391646: bprint:           rt_mutex_setprio: queued checked: task=fair_task pid=3580 prio=19 queued=0 running=0 state=0x200 vruntime=46922871
rt_task-3579  [000] 35.391652: bprint:           rt_mutex_setprio: running checked: task=fair_task pid=3580 prio=19 queued=0 running=0 state=0x200 vruntime=46922871
rt_task-3579  [000] 35.391657: bprint:           rt_mutex_setprio: fair_class=0xffff000008da2c80 rt_class=0xffff000008da2d70 prev_class=0xffff000008da2c80 p->sched_class=0xffff000008da2d70 oldprio=120 p->prio=19
rt_task-3579  [000] 35.391661: bprint:           detach_task_cfs_rq: task=fair_task pid=3580 cpu=4 vruntime_normalized=1
rt_task-3579  [000] 35.391706: sched_switch:     rt_task:3579 [19] D ==> swapper/0:0 [120] 
 <idle>-0     [004] 35.391828: bprint:           ttwu_do_activate: ttwu_do_activate: task=fair_task pid=3580
 <idle>-0     [004] 35.391832: bprint:           ttwu_do_activate: ttwu_activate: task=fair_task pid=3580
 <idle>-0     [004] 35.391833: bprint:           ttwu_do_wakeup: ttwu_do_wakeup: task=fair_task pid=3580
 <idle>-0     [004] 35.391834: sched_wakeup:     fair_task:3580 [19] success=1 CPU:004

It doesn't happen on hikey960 when I use two cpus of the same LLC or on my
laptop (i7-4750HQ).

  reply	other threads:[~2018-08-28 14:54 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-17 18:27 [PATCH] sched/fair: vruntime should normalize when switching from fair Steve Muckle
2018-08-20 23:54 ` Miguel de Dios
2018-08-23 16:52   ` Dietmar Eggemann
2018-08-24  6:54     ` Juri Lelli
2018-08-24 21:17       ` Steve Muckle
2018-09-06 23:25       ` Dietmar Eggemann
2018-09-07  7:16         ` Juri Lelli
2018-09-07  7:58           ` Vincent Guittot
2018-09-11  6:24             ` Dietmar Eggemann
2018-08-24  9:32   ` Peter Zijlstra
2018-08-24  9:47     ` Peter Zijlstra
2018-08-24 21:24       ` Steve Muckle
2018-08-27 11:14         ` Peter Zijlstra
2018-08-28 14:53           ` Dietmar Eggemann [this message]
2018-08-29 10:54             ` Dietmar Eggemann
2018-08-29 11:59               ` Peter Zijlstra
2018-08-29 15:33                 ` Dietmar Eggemann
2018-08-31 22:24                   ` Steve Muckle
2018-09-26  9:50             ` Wanpeng Li
2018-09-26 22:38               ` Dietmar Eggemann
2018-09-27  1:19                 ` Wanpeng Li
2018-09-27 13:22                   ` Dietmar Eggemann
2018-09-28  0:43                     ` Wanpeng Li
2018-09-28 16:10                       ` Steve Muckle
2018-09-28 16:45                         ` Joel Fernandes
2018-09-28 17:35                         ` Dietmar Eggemann
2018-09-29  1:07                           ` Wanpeng Li
2018-09-28 17:11                       ` Dietmar Eggemann
2018-09-28 16:43                   ` Joel Fernandes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2ed346fa-dbe8-4928-928b-a34338b2d8c9@arm.com \
    --to=dietmar.eggemann@arm.com \
    --cc=Chris.Redpath@arm.com \
    --cc=Morten.Rasmussen@arm.com \
    --cc=Patrick.Bellasi@arm.com \
    --cc=joaodias@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=migueldedios@google.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=quentin.perret@arm.com \
    --cc=smuckle@google.com \
    --cc=tkjos@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.