Re: [RFC][PATCH 0/5] Introduce checks for preemptable code for this_cpu_read/write()

Mathieu Desnoyers
2011-09-20T18:12:07Z

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Tue, 2011-09-20 at 13:25 -0400, Mathieu Desnoyers wrote:
> > * Steven Rostedt (rostedt@goodmis.org) wrote:
> > > On Tue, 2011-09-20 at 19:09 +0200, Peter Zijlstra wrote:
> > > > On Tue, 2011-09-20 at 12:56 -0400, Steven Rostedt wrote:
> > > > > random_cpu_*() // Thomas's idea
> > > >
> > > > I like this one best..
> > >
> > > I like it too, but not really the most appropriate.
> > >
> > > >
> > > > But you forgot do deal with the irqsafe_cpu() crap, that's the same
> > > > brainfart as this_cpu() but more expensive because it frobs IRQ state.
> > >
> > > But irqsafe_cpu_*() doesn't really have any real meaning to me. That is
> > > something when I see it, I go and read the comments about it. It doesn't
> > > contain "this_cpu" which is something that seems to explain what it is,
> > > even though the obvious is not what it is.
> >
> > Throwing ideas from the IRC discussion into the mix (Paul McKenney and I
> > came up with it at the same time):
> >
> > preempt_protected_percpu_*()
> > irq_protected_percpu_*()
> >
> > Seems to be quite self-explanatory.
> >
>
> For use where the per_cpu data is protected with preemption disabled?
> But isn't that the default case? Why make it hard to type for when you
> should use it in the normal case.
>
> It should be hard to type when it is a hack. As I recommended on IRC, we
> probably should have it as:
>
> use_this_if_you_really_do_not_care_what_cpu_you_are_on_but_are_anal_about_performance_cpu_*()
>
> 1) it is very self descriptive.
> 2) it would limit the usage as people wont like to have it in their
> code ;)

Not quite. What I was proposing more precisely:

- this_cpu_*() for the case where the caller needs to disable
preemption. This is the default case. This is exactly what you
proposed, with WARN_ON debug checks. This could even be "percpu_*()"
now that I think of it. There is no real point in the "this_cpu"
prefix.

- preempt_protected_percpu_*() and irq_protected_percpu_*() for
statistics/slub use. Those primitives disable preemption or irq
internally on non-x86 architectures. The caller of these primitives
is not required to disable preemption nor irqs.

Thoughts ?

Thanks,

Mathieu

--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.