Messages in this thread | | | Date | Wed, 12 Dec 2012 18:17:20 +0100 | From | Oleg Nesterov <> | Subject | Re: [RFC PATCH v4 1/9] CPU hotplug: Provide APIs to prevent CPU offline from atomic context |
| |
On 12/11, Srivatsa S. Bhat wrote: > > IOW, the hotplug readers just increment/decrement their per-cpu refcounts > when no writer is active.
plus cli/sti ;) and increment/decrement are atomic.
At first glance looks correct to me, but I'll try to read it carefully later.
A couple of minor nits,
> +static DEFINE_PER_CPU(bool, writer_signal);
Why it needs to be per-cpu? It can be global and __read_mostly to avoid the false-sharing. OK, perhaps to put reader_percpu_refcnt/writer_signal into a single cacheline...
> +void get_online_cpus_atomic(void) > +{ > + unsigned long flags; > + > + preempt_disable(); > + > + if (cpu_hotplug.active_writer == current) > + return; > + > + local_irq_save(flags);
Yes... this is still needed, we are going to increment reader_percpu_refcnt unconditionally and this makes reader_nested_percpu() == T.
But,
> +void put_online_cpus_atomic(void) > +{ > + unsigned long flags; > + > + if (cpu_hotplug.active_writer == current) > + goto out; > + > + local_irq_save(flags); > + > + /* > + * We never allow heterogeneous nesting of readers. So it is trivial > + * to find out the kind of reader we are, and undo the operation > + * done by our corresponding get_online_cpus_atomic(). > + */ > + if (__this_cpu_read(reader_percpu_refcnt)) > + __this_cpu_dec(reader_percpu_refcnt); > + else > + read_unlock(&hotplug_rwlock); > + > + local_irq_restore(flags); > +out: > + preempt_enable(); > +}
Do we really need local_irq_save/restore in put_ ?
Oleg.
| |