linux-kernel - Re: [PATCH] sched: Distinguish sched_wakeup event when wake up a task which did schedule out or not.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20140510152902.GW11096@twins.programming.kicks-ass.net>
Date:	Sat, 10 May 2014 17:29:02 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Dongsheng Yang <yangds.fnst@...fujitsu.com>
Cc:	rostedt@...dmis.org, bsegall@...gle.com, mingo@...hat.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched: Distinguish sched_wakeup event when wake up a
 task which did schedule out or not.

On Tue, May 06, 2014 at 10:52:34AM +0900, Dongsheng Yang wrote:
> ttwu_do_wakeup() is called when the task's state is switched back to
> TASK_RUNNING, whether or not the task actually scheduled out. Tracing
> the wakeup event when the task never scheduled out is quite confusing.
> 
> This patch take the use of 'success' parameter in sched_wakeup tracepoint,
> to indicate that the task we are waking up did schedule out or just
> change its state to TASK_RUNNING.
> 
> success is true:
> 	task was out of run queue and sleeping, we are really wake it up.
> success is false:
> 	taks was on run queue all the time and we just change its state
> 	to TASK_RUNNING.

No, I think this patch is crap and doesn't actually change anything
much, at best it simply reduces the size of the race window.

I also think you're trying to solve something that cannot be solved.

So the fundamental wait loop is:

  for (;;) {
	set_current_state(TASK_UNINTERRUPTIBLE);
	if (cond)
		break;
	schedule();
  }
  __set_task_state(TASK_RUNNING);

And the fundamental wakeup is:

  cond = true;
  wake_up_process(TASK_NORMAL);

And this is very much on purpose a lock-free but strictly ordered
scenario. It is a variation of:

  X = Y = 0

  (wait)	(wake)
  [w] X = 1	[w] Y = 1
  MB		MB
  [r] Y		[r] X

[ where: X := state, Y := cond ]

And we all 'know' that the only provided guarantee is that:
  X==0 && Y==0
is impossible -- but only that, all 3 other states are observable.

This guarantee means that its impossible to both miss the condition and
the wakeup; iow. it guarantees fwd progress.

OTOH its fundamentally racy, nothing guarantees we will not 'observe' both
the condition and the wakeup.

The setting of .success=false when ->on_rq is actively wrong, suppose
the waiter has already observed cond==false but has not yet gotten to
schedule(), at that point the wakeup happens and sees ->on_rq==1. The
wakeup is still very much a real wakeup.

If it were not for that wakeup the waiter would have gone to sleep and
missed the update of condition.

So no.. not going to happen. And certainly not with such a crappy
Changelog.

And a frozen seafood of choice lobbed at Steven for not seeing this :-)

Content of type "application/pgp-signature" skipped