Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1475404
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: Memory barrier needed with wake_up_process()? |
| Date | 2016-09-02 20:50 +0200 |
| Message-ID | <sd1Hj-36f-23@gated-at.bofh.it> (permalink) |
| References | <sd1eh-2Tg-7@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Fri, Sep 02, 2016 at 02:10:13PM -0400, Alan Stern wrote:
> 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().
A common case uses locking, in which case additional memory barriers
inside of the wait/wakeup functions are not needed. Any accesses made
while holding the lock before invoking the wakeup function (e.g.,
wake_up()) are guaranteed to be seen after acquiring that same
lock following return from the wait function (e.g., wait_event()).
In this case, adding barriers to the wait and wakeup functions would
just add overhead.
But yes, this decision does mean that people using the wait/wakeup
functions without locking need to be more careful. Something like
this:
/* prior accesses. */
smp_mb();
wakeup_flag = 1;
wake_up(...);
And on the other task:
wait_event(... wakeup_flag == 1 ...);
smp_mb();
/* The waker's prior accesses will be visible here. */
Or am I missing your point?
Thanx, Paul
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-02 20:20 +0200
Re: Memory barrier needed with wake_up_process()? "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-09-02 20:50 +0200
Re: Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-02 22:30 +0200
Re: Memory barrier needed with wake_up_process()? "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-09-03 11:10 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-03 14:40 +0200
Re: Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-03 16:30 +0200
Re: Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-03 16:50 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-05 10:40 +0200
Re: Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-05 17:30 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-06 13:40 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-06 13:50 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-06 14:30 +0200
Re: Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-06 16:50 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-06 17:10 +0200
Re: Memory barrier needed with wake_up_process()? Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-09-07 12:20 +0200
Re: Memory barrier needed with wake_up_process()? Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-09-06 13:50 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-02 21:30 +0200
Re: Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-02 22:20 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-03 00:20 +0200
Re: Memory barrier needed with wake_up_process()? Will Deacon <will.deacon@arm.com> - 2016-09-05 11:50 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-06 13:20 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-03 00:20 +0200
Re: Memory barrier needed with wake_up_process()? Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-09-03 09:00 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-03 14:50 +0200
Re: Memory barrier needed with wake_up_process()? Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-09-03 16:00 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-05 10:10 +0200
Re: Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-03 16:20 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-05 10:10 +0200
Re: Memory barrier needed with wake_up_process()? Alan Stern <stern@rowland.harvard.edu> - 2016-09-05 16:40 +0200
Re: Memory barrier needed with wake_up_process()? Peter Zijlstra <peterz@infradead.org> - 2016-09-03 00:20 +0200
csiph-web