Current -next has only the trace subsystem left as a user of local_t
Tracing uses local_t for per cpu safe atomic operations in the form
of cmpxchg and additions. We already have a cmpxchg_local but no "local"
form of addition.
The patchset introduces a similar local primitive add_local() and then
uses cmpxchg_local() and add_local() to remove local_t use from the
trace subsystem.
The last patch removes local_t support from the kernel tree.
The support for add_local() is pretty basic. We can add more
fancy inc/dec variants and more optimization in the next
revision of the patchset.
* Christoph Lameter ([email protected]) wrote:
> Current -next has only the trace subsystem left as a user of local_t
>
> Tracing uses local_t for per cpu safe atomic operations in the form
> of cmpxchg and additions. We already have a cmpxchg_local but no "local"
> form of addition.
>
> The patchset introduces a similar local primitive add_local() and then
> uses cmpxchg_local() and add_local() to remove local_t use from the
> trace subsystem.
>
> The last patch removes local_t support from the kernel tree.
>
> The support for add_local() is pretty basic. We can add more
> fancy inc/dec variants and more optimization in the next
> revision of the patchset.
>
Hi Christoph,
Yes, removing the local_t type could make sense. However, local_t maps
to a volatile long, not just "long". Secondly, I am concerned about the
fact that the patch you propose:
- does not create the primitives I use in lttng
- only deals with x86
In lttng (which is out of tree, but widely used), I need the equivalent
of:
local_read
local_set
local_add
local_cmpxchg
local_add_return
local_inc
The approach of just doing the x86 implementation and leaving all the
other architectures "for later" with a slow/non atomic generic fallback
is, imho, a no-go, given that some people (myself, actually) already
took the time to go through all the kernel architectures to create the
optimized local.h headers. Basically, you are destroying all that work,
asking for it to be done all over again.
I therefore argue that we should keep local.h as-is as long as the
replacement lacks the wide architecture support and primitive variety
found in local.h.
Thanks,
Mathieu
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, 5 Jan 2010, Mathieu Desnoyers wrote:
> Yes, removing the local_t type could make sense. However, local_t maps
> to a volatile long, not just "long". Secondly, I am concerned about the
> fact that the patch you propose:
Volatile is discouraged as far as I can tell.
> - does not create the primitives I use in lttng
> - only deals with x8
As I said its an RFC. This provides all the functionality you need
through. The rest is sugar coating.
> In lttng (which is out of tree, but widely used), I need the equivalent
> of:
>
> local_read
> local_set
> local_add
> local_cmpxchg
> local_add_return
> local_inc
Please read the patch! This is all provided. add_local_return in the RFC
provides what is needed to replace local_add, local_inc. We can add these
explicitly.
local_cmpxchg replacement is already in there in the form of
cmpxchg_local().
> The approach of just doing the x86 implementation and leaving all the
> other architectures "for later" with a slow/non atomic generic fallback
> is, imho, a no-go, given that some people (myself, actually) already
> took the time to go through all the kernel architectures to create the
> optimized local.h headers. Basically, you are destroying all that work,
> asking for it to be done all over again.
AS I said this is an RFC. I can easily generate all these things from the
existing local.hs for the architectures.
> I therefore argue that we should keep local.h as-is as long as the
> replacement lacks the wide architecture support and primitive variety
> found in local.h.
Cool down and please review the patch.
* Christoph Lameter ([email protected]) wrote:
> On Tue, 5 Jan 2010, Mathieu Desnoyers wrote:
>
> > Yes, removing the local_t type could make sense. However, local_t maps
> > to a volatile long, not just "long". Secondly, I am concerned about the
> > fact that the patch you propose:
>
> Volatile is discouraged as far as I can tell.
If you want to ensure that a simple variable assignment or read
(local_set, local_read) are not performed piecewise by the compiler
which can cause odd effects when shared with interrupt handlers, this
will however be necessary.
>
> > - does not create the primitives I use in lttng
> > - only deals with x8
>
> As I said its an RFC. This provides all the functionality you need
> through. The rest is sugar coating.
OK
>
> > In lttng (which is out of tree, but widely used), I need the equivalent
> > of:
> >
> > local_read
> > local_set
> > local_add
> > local_cmpxchg
> > local_add_return
> > local_inc
>
> Please read the patch! This is all provided. add_local_return in the RFC
> provides what is needed to replace local_add, local_inc. We can add these
> explicitly.
>
> local_cmpxchg replacement is already in there in the form of
> cmpxchg_local().
>
> > The approach of just doing the x86 implementation and leaving all the
> > other architectures "for later" with a slow/non atomic generic fallback
> > is, imho, a no-go, given that some people (myself, actually) already
> > took the time to go through all the kernel architectures to create the
> > optimized local.h headers. Basically, you are destroying all that work,
> > asking for it to be done all over again.
>
> AS I said this is an RFC. I can easily generate all these things from the
> existing local.hs for the architectures.
>
> > I therefore argue that we should keep local.h as-is as long as the
> > replacement lacks the wide architecture support and primitive variety
> > found in local.h.
>
> Cool down and please review the patch.
OK :)
Mathieu
>
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, 5 Jan 2010, Mathieu Desnoyers wrote:
> > Volatile is discouraged as far as I can tell.
>
> If you want to ensure that a simple variable assignment or read
> (local_set, local_read) are not performed piecewise by the compiler
> which can cause odd effects when shared with interrupt handlers, this
> will however be necessary.
Piecewise? Assignment of scalars or a pointer is an atomic operation by
default. Lots of things will break if that is not true.