On Fri, 12 May 2017 21:49:56 +0200 Peter Zijlstra <pet...@infradead.org> wrote:
> On Fri, May 12, 2017 at 01:15:44PM -0400, Steven Rostedt wrote: > > 2) Allow for get_online_cpus() to nest > > So Thomas and me have been avoiding doing this. > > In general we avoid nested locking in the kernel. Nested locking makes > an absolute mockery of locking rules and what all gets protected. > > Yes, its much easier.. But we managed to kill the BKL, so surely we can > fix the hotplug lock too, right ;-) Well, is it really a lock in that sense? Or more like a preempt_disable()? Which, one can argue is a BKL in its own right. get_online_cpus() is more like a preempt_disable() than a lock, as it is preventing something from happening and not really protecting data. preempt_disable() prevents a schedule from happening. get_online_cpus() prevents CPUs from going offline or coming online. Can you image the mess it would be if we prevent preempt_disable() from nesting? get_online_cpus() is similar, but maybe not so horrific. The problem I see with going the route of not letting get_online_cpus() from nesting, is that we are going to have to start encapsulating large areas where get_online_cpus() must be taken. Any time a low level function needs to take get_online_cpus() and there happens to be a higher level function that has a lock that also must have get_online_cpus() held, that calls that lower level function (take tracepoints_mutex for example), that means we need to remove the get_online_cpus() from the lower level function, and make it a requirement to be taken before calling that lower level function everywhere. It moves the get_online_cpus() away from what really needs to have protection, and makes it more into a global lock like the BKL. Look at all the places that needed get_online_cpus() in your patches where the function itself really didn't care about cpus going on or off line. -- Steve
