Messages in this thread |  | | Date | Wed, 12 Feb 2014 00:50:10 +0530 | From | "Srivatsa S. Bhat" <> | Subject | Re: [PATCH 01/51] CPU hotplug: Provide lockless versions of callback registration functions |
| |
On 02/11/2014 11:05 PM, Toshi Kani wrote: > On Tue, 2014-02-11 at 22:48 +0530, Gautham R Shenoy wrote: >> On Tue, Feb 11, 2014 at 09:33:56AM -0700, Toshi Kani wrote: >>> >>> I agree that introducing a reader-writer semaphore allows concurrent >>> executions. Adding yet another hotplug lock is a bit unfortunate, >>> though. >>> >> >> I agree with this last part. We already have enough locks for >> cpu-hotplug. Another one sounds one too many!! >> >> >>> This may be a dumb question, but can't we simply do this way? >>> >>> get_online_cpus(); >>> >>> for_each_online_cpu(cpu) >>> init_cpu(cpu); >>> >>> put_online_cpus(); >>> >> -------- Someone chooses to hotplug a cpu here ------ >> -------- But this subsystem might miss out on knowing >> about it since it hasn't registered its >> notifier yet! >> >>> register_cpu_notifier(&foobar_cpu_notifier); > > > How about this? foo_cpu_notifier returns NOP when foo_notifier_ready is > false. > > register_cpu_notifier(&foobar_cpu_notifier); > > get_online_cpus(); > > for_each_online_cpu(cpu) > init_cpu(cpu); > > foo_notifier_ready = true; > > put_online_cpus(); >
Nah, that looks a lot like some quick-n-dirty hack ;-( It would also amount to burdening the various subsystems to add weird-looking pieces of code such as this in their callbacks:
if (!foo_notifier_ready) return NOTIFY_OK;
This only makes it all the more evident that the callback registration APIs exposed by the CPU hotplug core is poorly designed.
What we need instead, is an elegant, well-defined and easy-to-use set of interfaces/APIs exposed by the core CPU hotplug code to the various subsystems. I don't think we should worry so much about the fact that we can't use the familiar get/put_online_cpus() in this type of callback registration scenario. We can introduce a sane set of APIs that work well in such situations and use them consistently.
For example, something like the code snippet shown below looks pretty neat to me:
cpu_notifier_register_begin();
for_each_online_cpu(cpu) init_cpu(cpu);
register_cpu_notifier(&foobar_cpu_notifier);
cpu_notifier_register_done();
What do you think?
Regards, Srivatsa S. Bhat
|  |