From: Kirill Tkhai <tkhai@yandex.ru>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Steven Rostedt <rostedt@goodmis.org>,
Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH] sched: Add logic to handle parallel try_to_wake_up() of the same task
Date: Tue, 16 Jul 2013 00:22:24 +0400 [thread overview]
Message-ID: <222821373919744@web21e.yandex.ru> (raw)
In-Reply-To: <20130715201945.GQ17211@twins.programming.kicks-ass.net>
16.07.2013, 00:19, "Peter Zijlstra" <peterz@infradead.org>:
> On Mon, Jul 15, 2013 at 06:14:34PM +0400, Kirill Tkhai wrote:
>
>>>> #ifdef CONFIG_SMP
>>>> + p->state = TASK_WAKING;
>>>> + smp_wmb();
>>>> +
>>> This too is broken; the loop below needs to be completed first,
>>> otherwise we change p->state while the task is still on the CPU and it
>>> might read the wrong p->state.
>> This place is below (on_rq && ttwu_remote) check, so the task
>> either 'dequeued and on_cpu == 0'
>> or it's in the middle of schedule() on arch, which wants unlocked
>> context switch.
>>
>> Nobody scheduler's probes p->state between prepare_lock_switch() and
>> finish_lock_switch(). Archs with unlocked ctx switch (mips and ia64)
>> don't change or probe state of previous process during context_switch.
>
> It means its after deactivate_task(), but before context_switch(). It so
> happens that
> context_switch()->prepare_task_switch()->trace_sched_switch() inspects
> p->state.
>
> Even if this was not the case, touching a task that is 'life' on another
> CPU is very _very_ bad practise.
Thanks for the explanation.
Kirill
prev parent reply other threads:[~2013-07-15 20:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-13 15:45 [PATCH] sched: Add logic to handle parallel try_to_wake_up() of the same task Kirill Tkhai
2013-07-14 5:55 ` Mike Galbraith
2013-07-15 6:31 ` Peter Zijlstra
2013-07-15 14:14 ` Kirill Tkhai
2013-07-15 20:19 ` Peter Zijlstra
2013-07-15 20:22 ` Kirill Tkhai [this message]
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=222821373919744@web21e.yandex.ru \
--to=tkhai@yandex.ru \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox