Re: [patch 04/41] cpu ops: Core piece for generic atomic per cpu operations

Christoph Lameter
2008-06-09T19:00:47Z

On Fri, 6 Jun 2008, Rusty Russell wrote:

> > Also, the above cpu_local_wrap(...) adds:
> >
> > #define cpu_local_wrap(l) \
> > ({ \
> > preempt_disable(); \
> > (l); \
> > preempt_enable(); \
> > }) \
> >
> > ... and there isn't a non-preemption version that I can find.
>
> Yes, this should be fixed. I thought i386 had optimized versions pre-merge,
> but I was wrong (%gs for per-cpu came later, and noone cleaned up these naive
> versions). Did you want me to write them?

How can that be fixed? You have no atomic instruction that calculates the
per cpu address in one go. And as long as that is the case you need to
disable preempt. Otherwise you may increment the per cpu variable of
another processor because the process was rescheduled after the address
was calculated but before the increment was done.

> > One other distinction is CPU_INC increments an arbitrary sized variable
> > while local_inc requires a local_t variable. This may not make it usable
> > in all cases.
>
> You might be right, but note that local_t is 64 bit on 64-bit platforms. And
> speculation of possible use cases isn't a good reason to rip out working
> infrastructure :)

Its fundamentally broken since because of the preemption issue. This is
also why local_t is rarely used.