Re: [RFC][PATCH 09/13] hotplug: Replace hotplug lock with percpu-rwsem

[Peter Zijlstra]

On Tue, Jun 23, 2015 at 12:57:39AM +0200, Oleg Nesterov wrote:
> > +
> > +	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?

While we will not call the actual lock, lockdep will still get confused
by the inconsistent locking order observed.

Change it and boot, you'll find lockdep output pretty quickly.

> > --- 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().

Probably true, in which case we could still use the callback to insert a
WARN_ON_ONCE(p->cpuhp_ref) :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/