All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Kirill Tkhai <ktkhai@parallels.com>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH] sched: Make dl_task_time() use task_rq_lock()
Date: Tue, 17 Feb 2015 14:32:26 +0100	[thread overview]
Message-ID: <20150217133226.GW24151@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20150217123139.GN5029@twins.programming.kicks-ass.net>

On Tue, Feb 17, 2015 at 01:31:39PM +0100, Peter Zijlstra wrote:
> On Tue, Feb 17, 2015 at 02:33:58PM +0300, Kirill Tkhai wrote:
> 
> > So, we move task_rq_lock() to sched.h, and dl_task_timer() uses it?
> 
> Yep, like this. I've also modified your earlier patch to dl_task_time()
> back to its original form.
> 
> ---
> Subject: sched: Make dl_task_time() use task_rq_lock()
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Tue Feb 17 13:22:25 CET 2015
> 
> Kirill reported that a dl task can be throttled and dequeued at the
> same time. This happens, when it becomes throttled in schedule(),
> which is called to go to sleep:
> 
> current->state = TASK_INTERRUPTIBLE;
> schedule()
>     deactivate_task()
>         dequeue_task_dl()
>             update_curr_dl()
>                 start_dl_timer()
>             __dequeue_task_dl()
>     prev->on_rq = 0;
> 
> This invalidates the assumption from commit 0f397f2c90ce ("sched/dl:
> Fix race in dl_task_timer()"):
> 
>   "The only reason we don't strictly need ->pi_lock now is because
>    we're guaranteed to have p->state == TASK_RUNNING here and are
>    thus free of ttwu races".
> 
> And therefore we have to use the full task_rq_lock() here.
> 
> This further amends the fact that we forgot to update the rq lock loop
> for TASK_ON_RQ_MIGRATE, from commit cca26e8009d1 ("sched: Teach
> scheduler to understand TASK_ON_RQ_MIGRATING state").
> 
> Reported-by: Kirill Tkhai <ktkhai@parallels.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/sched/core.c     |   76 ------------------------------------------------
>  kernel/sched/deadline.c |   12 +------
>  kernel/sched/sched.h    |   76 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 79 insertions(+), 85 deletions(-)

Duh, I put it under CONFIG_SMP, /me changes patch to not do this.

  reply	other threads:[~2015-02-17 13:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-17 10:46 [PATCH 1/2] sched: Move __task_rq_{, un}lock() to kernel/sched/sched.h Kirill Tkhai
2015-02-17 11:11 ` Peter Zijlstra
2015-02-17 11:20   ` Kirill Tkhai
2015-02-17 11:26   ` Peter Zijlstra
2015-02-17 11:33     ` Kirill Tkhai
2015-02-17 12:31       ` [PATCH] sched: Make dl_task_time() use task_rq_lock() Peter Zijlstra
2015-02-17 13:32         ` Peter Zijlstra [this message]
2015-02-18 17:06         ` [tip:sched/core] " tip-bot for Peter Zijlstra

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=20150217133226.GW24151@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=jpoimboe@redhat.com \
    --cc=ktkhai@parallels.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.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.