Re: [my_cpu_ptr 1/5] Introduce my_cpu_ptr()

Christoph Lameter
2009-05-29T15:38:16Z

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.