Messages in this thread | | | Date | Wed, 11 Apr 2012 05:28:43 -0700 | From | "Paul E. McKenney" <> | Subject | Re: CPU Hotplug rework |
| |
On Wed, Apr 11, 2012 at 11:32:57AM +0530, Srivatsa S. Bhat wrote: > On 04/11/2012 06:30 AM, Paul E. McKenney wrote: > > > On Tue, Apr 10, 2012 at 08:37:18PM -0400, Steven Rostedt wrote: > >> On Tue, 2012-04-10 at 17:28 -0700, Paul E. McKenney wrote: > >> > >>>> Just to throw out the stupid silly approach. > >>>> > >>>> What about creating a "__register_cpu_notifier()" that just does: > >>>> > >>>> int __ref __register_cpu_notifier(struct notifier_block *nb) > >>>> { > >>>> return raw_notifier_chain_register(&cpu_chain, nb); > >>>> } > >>>> > >>>> Also making cpu_maps_update_begin/done() global (and probably rename > >>>> them). > >> > >> I just noticed that the cpu_maps_update_begin/done() are already global. > >> > >>>> > >>>> and then in the above code do: > >>>> > >>>> cpu_maps_update_begin(); > >>>> __register_cpu_notifier(nb); > >>>> do_setup(); > >>>> cpu_maps_update_done(); > >>>> > >>>> > > > Wow! Believe it or not, this is precisely the crux of the approach I was > suggesting all along!! :-) Just that when put to code, it looked slightly > different than this.. Sorry for not being clear. > > So here is what I proposed, in a simplified form: > > Modify the existing register_cpu_notifier() to this (by possibly giving > it a different name): > > int __ref register_cpu_notifier(struct notifier_block *nb, > int (*do_setup)(void)) > { > int ret; > > cpu_maps_update_begin(); > ret = raw_notifier_chain_register(&cpu_chain, nb); > do_setup(); > cpu_maps_update_done(); > > return ret; > } > > and then, in the caller, do: > > register_cpu_notifier(nb, do_setup); > > If the caller doesn't need any such extra setup, just do: > > register_cpu_notifier(nb, NULL); > > > Of course, register_cpu_notifier() should handle NULL properly. > (My patch [1] handles it, along with some other special cases.) > > That's it! > > Also, it is to be noted that cpu_maps_update_begin/done() are global, but > not exported symbols - so modules can't use them. With the above approach, > we need not make them exported symbols, since the caller need not care about > these locks at all. > > >>>> Just saying, > >>> > >>> That does have some attractive properties, now that you mention it. ;-) > >> > >> Which property? Stupid or Silly ;-) > > > > As with any piece of software, no matter how small, both. ;-) > > > > Of course, __register_cpu_notifier() would need lockdep checking to make > > sure that it wasn't called without the benefit of cpu_maps_update_begin(). > > > Not with my approach ;-) Its all automatically handled :-)
Good point, looks good!
Thanx, Paul
> > I might be missing something, but as long as that was in place, seems > > like it is a lot simpler and easier to use than the alternatives that > > Srivatsa and I were kicking around. > > > > > Hehe :-) Thanks for simplifying things, Steve! > > > [1]. https://lkml.org/lkml/2012/3/1/39 > > Regards, > Srivatsa S. Bhat
| |