Messages in this thread | | | Date | Wed, 27 Jan 2016 10:39:44 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] signals: work around random wakeups in sigsuspend() |
| |
On Wed, 27 Jan 2016 17:41:54 +0100 Oleg Nesterov <oleg@redhat.com> wrote:
> > But it did get me to > > look at the patch again: > > > > + while (!signal_pending(current)) { > > + __set_current_state(TASK_INTERRUPTIBLE); > > + schedule(); > > + } > > > > That should very much be: > > > > for (;;) { > > set_current_state(TASK_INTERRUPTIBLE); > > if (signal_pending(current)) > > break; > > schedule(); > > } > > __set_current_state(TASK_RUNNING); > > Why? It should work either way. Yes, signal_wakeup() can come right before > __set_current_state(TASK_INTERRUPTIBLE) but this is fine, __schedule() must not > sleep if signal_pending() == T, that is why it checks signal_pending_state(). > See also the comment above smp_mb__before_spinlock() in schedule(). > > IOW, signal_pending() is the "special" condition, you do not need to serialize > this check with task->state setting, exactly because schedule() knows about the > signals.
So it's non-buggy because signal_pending() is special. But it *looks* buggy! And there's no comment there explaining why it looks buggy but isn't, so someone may later come along and "fix" it for us.
| |