Messages in this thread | | | Date | Wed, 11 Apr 2012 11:32:57 +0530 | From | "Srivatsa S. Bhat" <> | Subject | Re: CPU Hotplug rework |
| |
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 :-)
> 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
| |