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

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

* 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.
http://www.efficios.com