Messages in this thread | | | Date | Fri, 29 May 2009 11:37:48 -0400 (EDT) | From | Christoph Lameter <> | Subject | Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr() |
| |
On Fri, 29 May 2009, Rusty Russell wrote:
> > Have not seen it but it would be a bit confusing since > > we already have get_cpu* which must be paired with put_cpu* > > because of the refcount taking (get_cpu_var and get_cpu). > > get_cpu_ptr() would not have to be paired. > > To clarify, get_cpu_ptr() would be paired with put_cpu_ptr(). __get_cpu_ptr() > would be the "raw" one: > > #define get_cpu_ptr(xx) per_cpu_ptr(xx, get_cpu()) > #define __get_cpu_ptr(xx) per_cpu_ptr(xx, smp_processor_id())
Hmmm.. That would be a major change in semantics. I tend to think about these ops more as atomic like operations rather than preempt sections. The operation to obtain a the current instance for a given processor is an operation done in a given preemption context. The get_cpu_ptr() approach establishes the preemption context during the same operation.
That would mean use
*get_cpu_ptr(per_cpu_ptr1)++ *__get_cpu_ptr(per_cpu_ptr2)++ put_cpu_ptr()
instead of
preempt_disable() *__this_cpu_ptr(per_cpu_ptr1)++ *__this_cpu_ptr(per_cpu_ptr2)++ preempt_enable().
And for context in which we know that preemption is off
*__get_cpu_ptr(per_cpu_ptr1)++ *__get_cpu_ptr(per_cpu_ptr2)++
vs
*__this_cpu_ptr(per_cpu_ptr2)++ *__this_cpu_ptr(per_cpu_ptr2)++
So its the same
How would that look for atomic per cpu ops?
get_cpu_ptr_inc(per_cpu_ptr1); __get_cpu_ptr_inc(per_cpu_ptr2) put_cpu_ptr()
vs.
this_cpu_inc(per_cpu_ptr1) this_cpu_inc(per_cpu_ptr2)
and known preemption off context
__get_cpu_ptr_inc(per_cpu_ptr1) __get_cpu_ptr_inc(per_cpu_ptr2)
vs __this_cpu_inc(percpu_ptr1) __this_cpu_inc(percpu_ptr2)
Same again
Seems to be added complexity to also push a change of preemption into these operations. I'd rather keep the preempt status unchanged by these operations in particular because some counters may need interrupt protection on some platforms and not on others. We will likely need some additional operations like
this_cpu_inc_irqsafe( ....)
that will require an irq off / on on some platforms and be a simple increment on others.
| |