Ugh.
I think this is bogus.
Why?
On Thu, Mar 19, 2015 at 3:02 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> The generic version of this_cpu_read() and write() are:
>
> #define this_cpu_generic_read(pcp) \
> ({ typeof(pcp) ret__; \
> preempt_disable(); \
> ret__ = *this_cpu_ptr(&(pcp)); \
> preempt_enable(); \
> ret__; \
> })
>
> #define this_cpu_generic_to_op(pcp, val, op) \
> do { \
> unsigned long flags; \
> raw_local_irq_save(flags); \
> *__this_cpu_ptr(&(pcp)) op val; \
> raw_local_irq_restore(flags); \
> } while (0)
Let's just fix the generic versions of this_cpu_read/write.
Now, it is true that for the "op" versions of "this_cpu_xyz" we need
to disable preemption or interrupts or something for the generic case
(where "generic case" is weasel-wording for "the architecture is crap
and has horrible problems with any kinds of atomics, even just per-cpu
ones").
But that is *not* true for plain read/write. There is no point in
disabling preemption, because in the end, if preemption was enabled,
it happens on a random CPU anyway (and that may be fine - many
heuristics may not care *which* exact CPU it's about), and unlike the
"op" cases, the actual read or write is a single access, so there's no
reason to disable preemption/interrupts to make it "atomic" on that
random CPU.
If you pair a this_cpu_read with a this_cpu_write and expect them to
go to the same cpu, such a user obviously needs to disable preemption
etc for bigger reasons - but disabling preemption inside the operation
itself does absolutely nothing.
So is there really any reason to not make those simpler forms just do
something simpler like
#define this_cpu_generic_read(pcp) \
READ_ONCE(*this_cpu_ptr(&(pcp)))
#define this_cpu_generic_read(pcp, val) \
WRITE_ONCE(*this_cpu_ptr(&(pcp)), val)
instead?
Even if we end up being preempted in the middle, do we *care*? It's
going to one or the other CPU.
The only issue might be CPU hotplug in between (the previous CPU going
away), but I'm not sure even that matterts.
So I don't think the ring-buffer change is necessarily _wrong_, but if
this is a performance issue, why don't we just fix it up for the
generic case rather than for just one user?
Or is the hotplug issue a big deal? I thought we already had some
rcu-sched point for the cpu going away, so that even the hotplug case
should be ok.
Linus
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html