Messages in this thread | | | Date | Fri, 5 Jul 2019 23:00:32 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH] cpu/hotplug: Cache number of online CPUs |
| |
On Fri, 5 Jul 2019, Thomas Gleixner wrote: > On Fri, 5 Jul 2019, Mathieu Desnoyers wrote: > > ----- On Jul 5, 2019, at 4:49 AM, Ingo Molnar mingo@kernel.org wrote: > > > * Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: > > >> The semantic I am looking for here is C11's relaxed atomics. > > > > > > What does this mean? > > > > C11 states: > > > > "Atomic operations specifying memory_order_relaxed are relaxed only with respect > > to memory ordering. Implementations must still guarantee that any given atomic access > > to a particular atomic object be indivisible with respect to all other atomic accesses > > to that object." > > > > So I am concerned that num_online_cpus() as proposed in this patch > > try to access __num_online_cpus non-atomically, and without using > > READ_ONCE(). > > > > > > Similarly, the update-side should use WRITE_ONCE(). Protecting with a mutex > > does not provide mutual exclusion against concurrent readers of that variable. > > Again. This is nothing new. The current implementation of num_online_cpus() > has no guarantees whatsoever. > > bitmap_hweight() can be hit by a concurrent update of the mask it is > looking at. > > num_online_cpus() gives you only the correct number if you invoke it inside > a cpuhp_lock held section. So why do we need that fuzz about atomicity now? > > It's racy and was racy forever and even if we add that READ/WRITE_ONCE muck > then it still wont give you a reliable answer unless you hold cpuhp_lock at > least for read. So fore me that READ/WRITE_ONCE is just a cosmetic and > misleading reality distortion.
That said. If it makes everyone happy and feel better, I'm happy to add it along with a bit fat comment which explains that it's just preventing a theoretical store/load tearing issue and does not provide any guarantees other than that.
Thanks,
tglx
| |