Messages in this thread | | | Date | Wed, 10 Dec 2014 19:59:09 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH v3] CPU hotplug: active_writer not woken up in some cases - deadlock |
| |
On 12/10, David Hildenbrand wrote: > > > Why active_writer should see .puts_pending != 0 if this is called > > right after cpu_hotplug_begin() takes cpu_hotplug.lock but before > > it sets TASK_UNINTERRUPTIBLE? > > get_online_cpus() increased the refcount. > put_online_cpus() will increment puts_pending and trigger a wake up (if the > lock is alread taken - might be by cpu_hotplug_begin() or by some other > get_online_cpus()). > > So refcount == 1, puts_pending == 1 > > cpu_hotplug_begin() gets the lock and sees refcount == 1 and puts_pending == 0 > or puts_pending == 1 (race with put_online_cpus()). > > If that answers your question :)
Sorry for confusion ;)
I meant that without mb() cpu_hotplug_begin() can miss puts_pending != 0, so it needs set_current_state() before atomic_read().
But this doesn't matter, your v4 uses wake_up/prepare_to_wait.
> > IOW, > > > > > void cpu_hotplug_begin(void) > > > { > > > + spin_lock(&cpu_hotplug.awr_lock); > > > cpu_hotplug.active_writer = current; > > > + spin_unlock(&cpu_hotplug.awr_lock); > > > > > > cpuhp_lock_acquire(); > > > for (;;) { > > > mutex_lock(&cpu_hotplug.lock); > > > + __set_current_state(TASK_UNINTERRUPTIBLE); > > > > don't we need set_current_state() here ? > > Hm, good question, this was only a move of existing code. But I thing the > checked variant would be better. > > > > > Oleg. > > > > Thanks! > > David >
| |