All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Steve Muckle <smuckle@google.com>,
	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: Wed, 29 Aug 2018 16:33:32 +0100	[thread overview]
Message-ID: <f7c5a86f-7d76-bf26-4763-94a9ef4bba2e@arm.com> (raw)
In-Reply-To: <20180829115954.GS24124@hirez.programming.kicks-ass.net>

On 08/29/2018 12:59 PM, Peter Zijlstra wrote:
> On Wed, Aug 29, 2018 at 11:54:58AM +0100, Dietmar Eggemann wrote:
>> I forgot to mention that since fair_task's cpu affinity is restricted to
>> CPU4, there is no call to set_task_cpu()->migrate_task_rq_fair() since if
>> (task_cpu(p) != cpu) fails.
>>
>> I think the combination of cpu affinity of the fair_task to CPU4 and the
>> fact that the scheduler runs on CPU1 when waking fair_task (with the two
>> cpus not sharing LLC) while TTWU_QUEUE is enabled is the situation in which
>> this vruntime issue can happen.
> 
> Ohhh, D'0h. A remote wakeup that doesn't migrate.

Ah, there is this WF_MIGRATED flag, perfect for the distinction whether 
a task migrated or not.

> That would suggest something like so:
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index b39fb596f6c1..b3b62cf37fb6 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9638,7 +9638,8 @@ static inline bool vruntime_normalized(struct task_struct *p)
>   	 * - A task which has been woken up by try_to_wake_up() and
>   	 *   waiting for actually being woken up by sched_ttwu_pending().
>   	 */
> -	if (!se->sum_exec_runtime || p->state == TASK_WAKING)
> +	if (!se->sum_exec_runtime ||
> +	    (p->state == TASK_WAKING && p->sched_remote_wakeup))
>   		return true;
>   
>   	return false;
Yes, this solves the issue for the case I described. Using 
'p->sched_remote_wakeup' (WF_MIGRATED) looks more elegant than using 
'p->sched_class == &fair_sched_class'.

  reply	other threads:[~2018-08-29 15:33 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
2018-08-29 10:54             ` Dietmar Eggemann
2018-08-29 11:59               ` Peter Zijlstra
2018-08-29 15:33                 ` Dietmar Eggemann [this message]
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=f7c5a86f-7d76-bf26-4763-94a9ef4bba2e@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.