Christoph,
Any comment on this?
-- Steve
On Thu, 19 Mar 2015 15:16:25 -0700
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> 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