> Replaying what Tejun wrote: > > On 12/06/2012 12:13 AM, Srivatsa S. Bhat wrote: >> Some of the atomic hotplug readers cannot tolerate CPUs going offline while >> they are in their critical section. That is, they can't get away with just >> synchronizing with the updates to the cpu_online_mask; they really need to >> synchronize with the entire CPU tear-down sequence, because they are very >> much involved in the hotplug related code paths. >> >> Such "full" atomic hotplug readers need a way to *actually* and *truly* >> prevent CPUs from going offline while they are active. >> > > I don't think this is a good idea. You really should just need > get/put_online_cpus() and get/put_online_cpus_atomic(). The former > the same as they are. The latter replacing what > preempt_disable/enable() was protecting. Let's please not go > overboard unless we know they're necessary. I strongly suspect that > breaking up reader side from preempt_disable and making writer side a > bit lighter should be enough. Conceptually, it really should be a > simple conversion - convert preempt_disable/enable() pairs protecting > CPU on/offlining w/ get/put_cpu_online_atomic() and wrap the > stop_machine() section with the matching write lock. >
Yes, that _sounds_ sufficient, but IMHO it won't be, in practice. The *number* of call-sites that you need to convert from preempt_disable/enable to get/put_online_cpus_atomic() won't be too many, however the *frequency* of usage of those call-sites can potentially be very high. For example, the IPI path (smp_call_function_*) needs to use the new APIs instead of preempt_disable(); and this is quite a hot path. So if we replace preempt_disable/enable() with a synchronization mechanism that spins the reader *throughout* the CPU offline operation, and provide no light-weight alternative API, then even such very hot readers will have to bear the wrath. And IPIs and interrupts are the work-generators in a system. Since they can be hotplug readers, if we spin them like this, we effectively end up recreating the stop_machine() "effect", without even using stop_machine(). This is what I meant in my yesterday's reply too: https://lkml.org/lkml/2012/12/4/349 That's why we need a light-weight variant IMHO, so that we can use them atleast where feasible, like IPI path (smp_call_function_*) for example. That'll help us avoid the "stop_machine effect", hoping that most readers are of the light-type. As I mentioned in the cover-letter, most readers _are_ of the light-type (eg: 5 patches in this series deal with light readers, only 1 patch deals with a heavy/full reader). I don't see why we should unnecessarily slow down every reader just because a minority of readers actually need full synchronization with CPU offline. Regards, Srivatsa S. Bhat -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
