Messages in this thread | | | Date | Wed, 25 Apr 2018 22:09:17 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v1] kthread/smpboot: Serialize kthread parking against wakeup |
| |
On Wed, Apr 25, 2018 at 02:03:19PM +0530, Gaurav Kohli wrote: > diff --git a/kernel/smpboot.c b/kernel/smpboot.c > index 5043e74..c5c5184 100644 > --- a/kernel/smpboot.c > +++ b/kernel/smpboot.c > @@ -122,7 +122,45 @@ static int smpboot_thread_fn(void *data) > } > > if (kthread_should_park()) { > + /* > + * Serialize against wakeup. * * Prior wakeups must complete and later wakeups * will observe TASK_RUNNING. * * This avoids the case where the TASK_RUNNING * store from ttwu() competes with the * TASK_PARKED store from kthread_parkme(). * * If the TASK_PARKED store looses that * competition, kthread_unpark() will go wobbly. > + */ > + raw_spin_lock(¤t->pi_lock); > __set_current_state(TASK_RUNNING); > + raw_spin_unlock(¤t->pi_lock); > preempt_enable(); > if (ht->park && td->status == HP_THREAD_ACTIVE) { > BUG_ON(td->cpu != smp_processor_id());
Does that work for you?
But looking at this a bit more; don't we have the exact same problem with the TASK_RUNNING store in the !ht->thread_should_run() case? Suppose a ttwu() happens concurrently there, it can end up competing against the TASK_INTERRUPTIBLE store, no?
Of course, that race is not fatal, we'll just end up going around the loop once again I suppose. Maybe a comment there too?
/* * A similar race is possible here, but loosing * the TASK_INTERRUPTIBLE store is harmless and * will make us go around the loop once more. */
And of course, I suspect we actually want to use TASK_IDLE, smpboot threads don't want signals do they? But that probably ought to be a separate patch.
| |