Messages in this thread | | | Date | Fri, 31 Oct 2014 23:13:40 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/7] sched,wait: Fix a kthread race with wait_woken() |
| |
On 10/31, Peter Zijlstra wrote: > > There is a race between kthread_stop() and the new wait_woken() that > can result in a lack of progress.
Likewise, the user of wait_woken() can miss any other event which is not associated with wq we are going to sleep on. Please see below.
> +static inline bool is_kthread_should_stop(void) > +{ > + return (current->flags & PF_KTHREAD) && kthread_should_stop(); > +} > > /* > * DEFINE_WAIT_FUNC(wait, woken_wake_func); > @@ -326,7 +331,7 @@ long wait_woken(wait_queue_t *wait, unsi > * woken_wake_function() such that if we observe WQ_FLAG_WOKEN we must > * also observe all state before the wakeup. > */ > - if (!(wait->flags & WQ_FLAG_WOKEN)) > + if (!(wait->flags & WQ_FLAG_WOKEN) && !is_kthread_should_stop()) > timeout = schedule_timeout(timeout);
Well yes, this is more straightforward than other hacks we discussed before. But see above, this doesn't look flexible enough.
And. This assumes that the user must also check kthread_should_stop(), otherwise the waiting loop becomes a busy-wait loop.
So I won't argue, but I still think it would be better to allow the user to do set_task_state() by hand if it needs to check the additional conditions.
Oleg.
| |