On 06/22, Peter Zijlstra wrote: > > The cpu hotplug lock is a rwsem with read-in-write and read-in-read > recursion. Implement it as such.
And this patch fixes the problem afaics. Currently cpu_hotplug_begin() can livelock because it doesn't stop the new readers. With this patch this is no longer possible. > -static inline void percpu_down_read(struct percpu_rw_semaphore *sem) > +static inline void _percpu_down_read(struct percpu_rw_semaphore *sem) > { > might_sleep(); > > - rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_); > - > preempt_disable(); > /* > * We are in an RCU-sched read-side critical section, so the writer > @@ -46,6 +44,12 @@ static inline void percpu_down_read(stru > */ > } > > +static inline void percpu_down_read(struct percpu_rw_semaphore *sem) > +{ > + rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_); > + _percpu_down_read(sem); > +} ... > void get_online_cpus(void) > { > might_sleep(); > - if (cpu_hotplug.active_writer == current) > + > + /* read in write recursion */ > + if (cpu_hotplug.writer == current) > + return; > + > + /* read in read recursion */ > + if (current->cpuhp_ref++) > return; > - cpuhp_lock_acquire_read(); > - mutex_lock(&cpu_hotplug.lock); > - atomic_inc(&cpu_hotplug.refcount); > - mutex_unlock(&cpu_hotplug.lock); > + > + lock_map_acquire_read(&cpu_hotplug.rwsem.rw_sem.dep_map); > + _percpu_down_read(&cpu_hotplug.rwsem); > } Confused... Why do we need _percpu_down_read()? Can't get_online_cpus() just use percpu_down_read() ? Yes, percpu_down_read() is not recursive, like the normal down_read(). But this does not matter because we rely on ->cpuhp_ref anyway? > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -1410,6 +1410,8 @@ static struct task_struct *copy_process( > p->sequential_io_avg = 0; > #endif > > + cpu_hotplug_init_task(p); This is probably unnecessary, copy_process() should not be called under get_online_cpus(). Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/