All of lore.kernel.org
 help / color / mirror / Atom feed
* Memory barrier needed with wake_up_process()?
@ 2016-09-02 18:10 Alan Stern
  2016-09-02 18:47 ` Paul E. McKenney
  2016-09-02 19:18 ` Peter Zijlstra
  0 siblings, 2 replies; 41+ messages in thread
From: Alan Stern @ 2016-09-02 18:10 UTC (permalink / raw)
  To: Paul E. McKenney, Peter Zijlstra, Ingo Molnar
  Cc: Felipe Balbi, USB list, Kernel development list

Paul, Peter, and Ingo:

This must have come up before, but I don't know what was decided.

Isn't it often true that a memory barrier is needed before a call to 
wake_up_process()?  A typical scenario might look like this:

	CPU 0
	-----
	for (;;) {
		set_current_state(TASK_INTERRUPTIBLE);
		if (signal_pending(current))
			break;
		if (wakeup_flag)
			break;
		schedule();
	}
	__set_current_state(TASK_RUNNING);
	wakeup_flag = 0;


	CPU 1
	-----
	wakeup_flag = 1;
	wake_up_process(my_task);

The underlying pattern is:

	CPU 0				CPU 1
	-----				-----
	write current->state		write wakeup_flag
	smp_mb();
	read wakeup_flag		read my_task->state

where set_current_state() does the write to current->state and 
automatically adds the smp_mb(), and wake_up_process() reads 
my_task->state to see whether the task needs to be woken up.

The kerneldoc for wake_up_process() says that it has no implied memory
barrier if it doesn't actually wake anything up.  And even when it
does, the implied barrier is only smp_wmb, not smp_mb.

This is the so-called SB (Store Buffer) pattern, which is well known to
require a full smp_mb on both sides.  Since wake_up_process() doesn't
include smp_mb(), isn't it correct that the caller must add it
explicitly?

In other words, shouldn't the code for CPU 1 really be:

	wakeup_flag = 1;
	smp_mb();
	wake_up_process(task);

If my reasoning is correct, then why doesn't wake_up_process() include 
this memory barrier automatically, the way set_current_state() does?  
There could be an alternate version (__wake_up_process()) which omits 
the barrier, just like __set_current_state().

Alan Stern

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2017-01-16 19:23 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-02 18:10 Memory barrier needed with wake_up_process()? Alan Stern
2016-09-02 18:47 ` Paul E. McKenney
2016-09-02 20:29   ` Alan Stern
2016-09-03  9:07     ` Paul E. McKenney
2016-09-03 12:31     ` Peter Zijlstra
2016-09-03 14:26       ` Alan Stern
2016-09-03 14:49         ` Alan Stern
2016-09-05  8:33           ` Peter Zijlstra
2016-09-05 15:29             ` Alan Stern
2016-09-06 11:36               ` Peter Zijlstra
2016-09-06 11:43                 ` Felipe Balbi
2016-09-06 11:49                   ` Peter Zijlstra
2016-09-06 12:20                     ` Peter Zijlstra
2016-09-06 14:46                       ` Alan Stern
2016-09-06 15:05                         ` Peter Zijlstra
2016-09-07 10:12                         ` Felipe Balbi
2016-09-09 10:36                           ` Felipe Balbi
2016-09-09 16:12                             ` Alan Stern
2016-09-19 11:11                               ` Felipe Balbi
2016-09-19 17:35                                 ` Alan Stern
2016-09-20 10:12                                   ` Felipe Balbi
2016-09-20 12:53                                     ` Felipe Balbi
2016-09-20 14:40                                     ` Alan Stern
2017-01-16 11:12                                       ` Felipe Balbi
2017-01-16 17:09                                         ` Alan Stern
2017-01-16 19:04                                           ` Felipe Balbi
2017-01-16 19:19                                             ` Felipe Balbi
2016-09-02 19:18 ` Peter Zijlstra
2016-09-02 20:16   ` Alan Stern
2016-09-02 22:14     ` Peter Zijlstra
2016-09-02 22:16       ` Peter Zijlstra
2016-09-05  9:43         ` Will Deacon
2016-09-06 11:10           ` Peter Zijlstra
2016-09-02 22:16     ` Peter Zijlstra
2016-09-03  6:58       ` Felipe Balbi
2016-09-03 12:19         ` Peter Zijlstra
2016-09-03 13:51           ` Felipe Balbi
2016-09-05  8:09             ` Peter Zijlstra
2016-09-03 14:16           ` Alan Stern
2016-09-05  8:08             ` Peter Zijlstra
2016-09-05 14:33               ` Alan Stern

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.