Use the new generic cmpxchg_local (disables interrupt). Also use the generic
cmpxchg as fallback if SMP is not set.
Signed-off-by: Mathieu Desnoyers <[email protected]>
CC: [email protected]
CC: [email protected]
---
include/asm-parisc/atomic.h | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
Index: linux-2.6-lttng/include/asm-parisc/atomic.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-parisc/atomic.h 2007-07-20 19:44:40.000000000 -0400
+++ linux-2.6-lttng/include/asm-parisc/atomic.h 2007-07-20 19:44:47.000000000 -0400
@@ -122,6 +122,35 @@ __cmpxchg(volatile void *ptr, unsigned l
(unsigned long)_n_, sizeof(*(ptr))); \
})
+#include <asm-generic/cmpxchg-local.h>
+
+static inline unsigned long __cmpxchg_local(volatile void *ptr,
+ unsigned long old,
+ unsigned long new_, int size)
+{
+ switch (size) {
+#ifdef CONFIG_64BIT
+ case 8: return __cmpxchg_u64((unsigned long *)ptr, old, new_);
+#endif
+ case 4: return __cmpxchg_u32(ptr, old, new_);
+ default:
+ return __cmpxchg_local_generic(ptr, old, new_, size);
+ }
+}
+
+/*
+ * cmpxchg_local and cmpxchg64_local are atomic wrt current CPU. Always make
+ * them available.
+ */
+#define cmpxchg_local(ptr,o,n) \
+ (__typeof__(*(ptr)))__cmpxchg_local((ptr), (unsigned long)(o), \
+ (unsigned long)(n), sizeof(*(ptr)))
+#ifdef CONFIG_64BIT
+#define cmpxchg64_local(ptr,o,n) cmpxchg_local((ptr), (o), (n))
+#else
+#define cmpxchg64_local(ptr,o,n) __cmpxchg64_local_generic((ptr), (o), (n))
+#endif
+
/* Note that we need not lock read accesses - aligned word writes/reads
* are atomic, so a reader never sees unconsistent values.
*
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Sun, Aug 12, 2007 at 10:54:49AM -0400, Mathieu Desnoyers wrote:
> Use the new generic cmpxchg_local (disables interrupt). Also use the generic
> cmpxchg as fallback if SMP is not set.
Mathieu,
thanks for adding __cmpxchg_local to parisc.... but why do we need it?
By definition, atomic operators are, well, atomic.
I searched for __cmpxchg_local and found this reference:
http://www.ussg.iu.edu/hypermail/linux/kernel/0612.2/1337.html
but the "root" of that thread (Dec 20, 2006):
http://www.ussg.iu.edu/hypermail/linux/kernel/0612.2/1334.html
Doesn't explain the difference between "local" and "non-local" either.
Per CPU data should only need memory barriers (in some cases) and
protection against interrupts (in probably more cases). So I'm not
understanding why a new set of APIs is needed.
Can you add a description to Documentation/atomic_ops.txt ?
*sigh* sorry for being "late to the party" on this one...
cheers,
grant
>
> Signed-off-by: Mathieu Desnoyers <[email protected]>
> CC: [email protected]
> CC: [email protected]
> ---
> include/asm-parisc/atomic.h | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> Index: linux-2.6-lttng/include/asm-parisc/atomic.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/asm-parisc/atomic.h 2007-07-20 19:44:40.000000000 -0400
> +++ linux-2.6-lttng/include/asm-parisc/atomic.h 2007-07-20 19:44:47.000000000 -0400
> @@ -122,6 +122,35 @@ __cmpxchg(volatile void *ptr, unsigned l
> (unsigned long)_n_, sizeof(*(ptr))); \
> })
>
> +#include <asm-generic/cmpxchg-local.h>
> +
> +static inline unsigned long __cmpxchg_local(volatile void *ptr,
> + unsigned long old,
> + unsigned long new_, int size)
> +{
> + switch (size) {
> +#ifdef CONFIG_64BIT
> + case 8: return __cmpxchg_u64((unsigned long *)ptr, old, new_);
> +#endif
> + case 4: return __cmpxchg_u32(ptr, old, new_);
> + default:
> + return __cmpxchg_local_generic(ptr, old, new_, size);
> + }
> +}
> +
> +/*
> + * cmpxchg_local and cmpxchg64_local are atomic wrt current CPU. Always make
> + * them available.
> + */
> +#define cmpxchg_local(ptr,o,n) \
> + (__typeof__(*(ptr)))__cmpxchg_local((ptr), (unsigned long)(o), \
> + (unsigned long)(n), sizeof(*(ptr)))
> +#ifdef CONFIG_64BIT
> +#define cmpxchg64_local(ptr,o,n) cmpxchg_local((ptr), (o), (n))
> +#else
> +#define cmpxchg64_local(ptr,o,n) __cmpxchg64_local_generic((ptr), (o), (n))
> +#endif
> +
> /* Note that we need not lock read accesses - aligned word writes/reads
> * are atomic, so a reader never sees unconsistent values.
> *
>
> --
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
> _______________________________________________
> parisc-linux mailing list
> [email protected]
> http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
* Grant Grundler ([email protected]) wrote:
> On Sun, Aug 12, 2007 at 10:54:49AM -0400, Mathieu Desnoyers wrote:
> > Use the new generic cmpxchg_local (disables interrupt). Also use the generic
> > cmpxchg as fallback if SMP is not set.
>
> Mathieu,
> thanks for adding __cmpxchg_local to parisc.... but why do we need it?
>
> By definition, atomic operators are, well, atomic.
>
> I searched for __cmpxchg_local and found this reference:
> http://www.ussg.iu.edu/hypermail/linux/kernel/0612.2/1337.html
>
> but the "root" of that thread (Dec 20, 2006):
> http://www.ussg.iu.edu/hypermail/linux/kernel/0612.2/1334.html
>
> Doesn't explain the difference between "local" and "non-local" either.
> Per CPU data should only need memory barriers (in some cases) and
> protection against interrupts (in probably more cases). So I'm not
> understanding why a new set of APIs is needed.
>
> Can you add a description to Documentation/atomic_ops.txt ?
> *sigh* sorry for being "late to the party" on this one...
>
Does Documentation/local_ops.txt answer your questions ? If not, please
tell me and I'll gladly explain more.
Mathieu
> cheers,
> grant
>
> >
> > Signed-off-by: Mathieu Desnoyers <[email protected]>
> > CC: [email protected]
> > CC: [email protected]
> > ---
> > include/asm-parisc/atomic.h | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > Index: linux-2.6-lttng/include/asm-parisc/atomic.h
> > ===================================================================
> > --- linux-2.6-lttng.orig/include/asm-parisc/atomic.h 2007-07-20 19:44:40.000000000 -0400
> > +++ linux-2.6-lttng/include/asm-parisc/atomic.h 2007-07-20 19:44:47.000000000 -0400
> > @@ -122,6 +122,35 @@ __cmpxchg(volatile void *ptr, unsigned l
> > (unsigned long)_n_, sizeof(*(ptr))); \
> > })
> >
> > +#include <asm-generic/cmpxchg-local.h>
> > +
> > +static inline unsigned long __cmpxchg_local(volatile void *ptr,
> > + unsigned long old,
> > + unsigned long new_, int size)
> > +{
> > + switch (size) {
> > +#ifdef CONFIG_64BIT
> > + case 8: return __cmpxchg_u64((unsigned long *)ptr, old, new_);
> > +#endif
> > + case 4: return __cmpxchg_u32(ptr, old, new_);
> > + default:
> > + return __cmpxchg_local_generic(ptr, old, new_, size);
> > + }
> > +}
> > +
> > +/*
> > + * cmpxchg_local and cmpxchg64_local are atomic wrt current CPU. Always make
> > + * them available.
> > + */
> > +#define cmpxchg_local(ptr,o,n) \
> > + (__typeof__(*(ptr)))__cmpxchg_local((ptr), (unsigned long)(o), \
> > + (unsigned long)(n), sizeof(*(ptr)))
> > +#ifdef CONFIG_64BIT
> > +#define cmpxchg64_local(ptr,o,n) cmpxchg_local((ptr), (o), (n))
> > +#else
> > +#define cmpxchg64_local(ptr,o,n) __cmpxchg64_local_generic((ptr), (o), (n))
> > +#endif
> > +
> > /* Note that we need not lock read accesses - aligned word writes/reads
> > * are atomic, so a reader never sees unconsistent values.
> > *
> >
> > --
> > Mathieu Desnoyers
> > Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
> > _______________________________________________
> > parisc-linux mailing list
> > [email protected]
> > http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
From: Grant Grundler <[email protected]>
Date: Mon, 27 Aug 2007 15:04:32 -0600
> Doesn't explain the difference between "local" and "non-local" either.
> Per CPU data should only need memory barriers (in some cases) and
> protection against interrupts (in probably more cases). So I'm not
> understanding why a new set of APIs is needed.
>
> Can you add a description to Documentation/atomic_ops.txt ?
My understanding is that the local versions can run without
memory barriers. Only local cpu execution and local interrupt
threads of control can access the variable, and interrupts
force all stores to become visible to the local cpu so an
interrupt can't see the datum from before the atomic update
if one was in progress when the interrupt triggered, and
stuff like that.
On Mon, Aug 27, 2007 at 05:11:40PM -0400, Mathieu Desnoyers wrote:
...
> > Can you add a description to Documentation/atomic_ops.txt ?
> > *sigh* sorry for being "late to the party" on this one...
>
> Does Documentation/local_ops.txt answer your questions ? If not, please
> tell me and I'll gladly explain more.
Yes, it does mostly - thanks.
A few questions/nits:
o Did you attempt quantify how many places in the kernel could use this?
I'm just trying to get a feel for how useful this really is vs just
using existing mechanisms (that people understand) to implement a
non-SMP-safe counter that protects updates (writes) against interrupts.
If you did, adding some referencs to local_ops.txt would be helpful
so folks could look for examples of "correct usage".
o Wording in local_ops.txt: "on the
"... it will then appear to be written out of order wrt
other memory writes on the owner CPU."
I'd like to suggest "by the owner CPU".
o How can a local_t counter protect updates (writes) against interrupts
but not preemption?
I always thought preemption required some sort of interrupt or trap.
Maybe the local_ops.txt explains that and I just missed it.
DaveM explained updates "in flight" would not be visible to interrupts
and I suspect that's the answer to my question....but then I don't "feel
good" the local_ops are safe to update in interrupts _and_ the process
context kernel. Maybe the relationship between local_ops, preemption,
and interrupts could be explained more carefully in local_ops.txt.
o OK to add a reference for local_ops.txt to atomic_ops.txt?
They are obviously related and anyone "discovering" one of the docs
should be made aware of the other.
Patch+log entry appended below. Please sign-off if that's ok with you.
thanks,
grant
Diff+Commit entry against 2.6.22.5:
local_t is a variant of atomic_t and has related ops to match.
Add reference for local_t documentation to atomic_ops.txt.
Signed-off-by: Grant Grundler <[email protected]>
--- 2.6.22.5-ORIG/Documentation/atomic_ops.txt 2007-08-27 22:50:27.000000000 -0700
+++ 2.6.22.5-ggg/Documentation/atomic_ops.txt 2007-08-27 22:54:44.000000000 -0700
@@ -14,6 +14,10 @@
typedef struct { volatile int counter; } atomic_t;
+local_t is very similar to atomic_t. If the counter is per CPU and only
+updated by one CPU, local_t is probably more appropriate. Please see
+Documentation/local_ops.txt for the semantics of local_t.
+
The first operations to implement for atomic_t's are the
initializers and plain reads.
* Grant Grundler ([email protected]) wrote:
> On Mon, Aug 27, 2007 at 05:11:40PM -0400, Mathieu Desnoyers wrote:
> ...
> > > Can you add a description to Documentation/atomic_ops.txt ?
> > > *sigh* sorry for being "late to the party" on this one...
> >
> > Does Documentation/local_ops.txt answer your questions ? If not, please
> > tell me and I'll gladly explain more.
>
> Yes, it does mostly - thanks.
>
> A few questions/nits:
> o Did you attempt quantify how many places in the kernel could use this?
> I'm just trying to get a feel for how useful this really is vs just
> using existing mechanisms (that people understand) to implement a
> non-SMP-safe counter that protects updates (writes) against interrupts.
> If you did, adding some referencs to local_ops.txt would be helpful
> so folks could look for examples of "correct usage".
>
Good question. Since it is useful to implement fast, interrupt
reentrant, counters of any kind without disabling interrupts, I think it
could be vastely used in the kernel. I also use it in my LTTng kernel
tracer implementation to provide very fast buffer management. It is used
in LTTng, but could be used for most kind of buffering management too;
meaning that we could manage buffers without disabling interrupts.
So I don't expect to come with an "upper bound" about where it can be
used...
> o Wording in local_ops.txt: "on the
> "... it will then appear to be written out of order wrt
> other memory writes on the owner CPU."
>
> I'd like to suggest "by the owner CPU".
>
Ok, fixing.. thanks!
> o How can a local_t counter protect updates (writes) against interrupts
> but not preemption?
> I always thought preemption required some sort of interrupt or trap.
> Maybe the local_ops.txt explains that and I just missed it.
>
"Local atomic operations only guarantee variable modification atomicity
wrt the CPU which owns the data. Therefore, care must taken to make sure
that only one CPU writes to the local_t data. This is done by using per
cpu data and making sure that we modify it from within a preemption safe
context." -> therefore, preemption must be disabled around local ops
usage. This is required to be pinned to one CPU anyway.
> DaveM explained updates "in flight" would not be visible to interrupts
> and I suspect that's the answer to my question....but then I don't "feel
> good" the local_ops are safe to update in interrupts _and_ the process
> context kernel. Maybe the relationship between local_ops, preemption,
> and interrupts could be explained more carefully in local_ops.txt.
>
Does the paragraph above explain it enough or should I add some more
explanation ?
> o OK to add a reference for local_ops.txt to atomic_ops.txt?
> They are obviously related and anyone "discovering" one of the docs
> should be made aware of the other.
> Patch+log entry appended below. Please sign-off if that's ok with you.
>
I'm perfectly ok with the idea, but suggest a small modification. See
below.
>
> thanks,
> grant
>
> Diff+Commit entry against 2.6.22.5:
>
> local_t is a variant of atomic_t and has related ops to match.
> Add reference for local_t documentation to atomic_ops.txt.
>
> Signed-off-by: Grant Grundler <[email protected]>
>
>
> --- 2.6.22.5-ORIG/Documentation/atomic_ops.txt 2007-08-27 22:50:27.000000000 -0700
> +++ 2.6.22.5-ggg/Documentation/atomic_ops.txt 2007-08-27 22:54:44.000000000 -0700
> @@ -14,6 +14,10 @@
>
> typedef struct { volatile int counter; } atomic_t;
>
> +local_t is very similar to atomic_t. If the counter is per CPU and only
> +updated by one CPU, local_t is probably more appropriate. Please see
> +Documentation/local_ops.txt for the semantics of local_t.
> +
> The first operations to implement for atomic_t's are the
> initializers and plain reads.
>
The text snippet is good, but I am not sure it belongs between the
description of atomic_t type and its initializers. What if we do
something like: (with context, I tried to explain the distinction
between atomic_t and local_t some more)
Semantics and Behavior of Atomic and
Bitmask Operations
David S. Miller
This document is intended to serve as a guide to Linux port
maintainers on how to implement atomic counter, bitops, and spinlock
interfaces properly.
atomic_t should be used to provide a type with update primitives
executed atomically from any CPU. If the counter is per CPU and only
updated by one CPU, local_t is probably more appropriate. Please see
Documentation/local_ops.txt for the semantics of local_t.
The atomic_t type should be defined as a signed integer.
Also, it should be made opaque such that any kind of cast to a normal
C integer type will fail. Something like the following should
suffice:
Mathieu
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, Aug 28, 2007 at 07:50:18AM -0400, Mathieu Desnoyers wrote:
...
> > A few questions/nits:
> > o Did you attempt quantify how many places in the kernel could use this?
> > I'm just trying to get a feel for how useful this really is vs just
> > using existing mechanisms (that people understand) to implement a
> > non-SMP-safe counter that protects updates (writes) against interrupts.
> > If you did, adding some referencs to local_ops.txt would be helpful
> > so folks could look for examples of "correct usage".
> >
>
> Good question. Since it is useful to implement fast, interrupt
> reentrant, counters of any kind without disabling interrupts, I think it
> could be vastely used in the kernel. I also use it in my LTTng kernel
> tracer implementation to provide very fast buffer management. It is used
> in LTTng, but could be used for most kind of buffering management too;
> meaning that we could manage buffers without disabling interrupts.
>
> So I don't expect to come with an "upper bound" about where it can be
> used...
Ok...so I'll try to find one in 2.6.22.5:
grundler <1855>find -name \*.c | xargs fgrep DEFINE_PER_CPU | fgrep atomic_t
./arch/s390/kernel/time.c:static DEFINE_PER_CPU(atomic_t, etr_sync_word);
grundler <1856>find -name \*.c | xargs fgrep DEFINE_PER_CPU | fgrep local_t
./arch/x86_64/kernel/nmi.c:static DEFINE_PER_CPU(local_t, alert_counter);
uhm, I was expecting more than that. Maybe there is some other systemic
problem with how PER_CPU stuff is used/declared?
In any case, some references to LTT usage would be quite helpful.
E.g. a list of file and variable names at the end of local_ops.txt file.
> > o How can a local_t counter protect updates (writes) against interrupts
> > but not preemption?
> > I always thought preemption required some sort of interrupt or trap.
> > Maybe the local_ops.txt explains that and I just missed it.
> >
>
> "Local atomic operations only guarantee variable modification atomicity
> wrt the CPU which owns the data. Therefore, care must taken to make sure
> that only one CPU writes to the local_t data. This is done by using per
> cpu data and making sure that we modify it from within a preemption safe
> context." -> therefore, preemption must be disabled around local ops
> usage. This is required to be pinned to one CPU anyway.
Sorry...the quoted text doesn't answer my question. It's a definition
of semantics, not an explanation of the "mechanics".
I want to know what happens when (if?) an interrupt occurs in the
middle of a read/modify/write sequence that isn't prefixed with LOCK
(or something similar for other arches like "store locked conditional" ops).
Stating the semantics is a good thing - but not a substitution for
describing how it works for a given architecture. Either in the code
or in local_ops.txt. Otherwise people like me won't use it because
we don't believe that (or understand how) it really works.
> > DaveM explained updates "in flight" would not be visible to interrupts
> > and I suspect that's the answer to my question....but then I don't "feel
> > good" the local_ops are safe to update in interrupts _and_ the process
> > context kernel. Maybe the relationship between local_ops, preemption,
> > and interrupts could be explained more carefully in local_ops.txt.
> >
>
> Does the paragraph above explain it enough or should I add some more
> explanation ?
Please add a bit more detail. If DaveM is correct (he normally is), then
there must be limits on how the local_t can be used in the kernel process
and interrupt contexts. I'd like those rules spelled out very clearly
since it's easy to get wrong and tracking down such a bug is quite painful.
Note: I already missed the one critical sentence about only the "owning"
CPU can write the value....there seem to be other limitations as well
with respect to interrupts.
> > o OK to add a reference for local_ops.txt to atomic_ops.txt?
> > They are obviously related and anyone "discovering" one of the docs
> > should be made aware of the other.
> > Patch+log entry appended below. Please sign-off if that's ok with you.
> >
>
> I'm perfectly ok with the idea, but suggest a small modification. See
> below.
Looks fine to me. Add your "Signed-off-by" and submit to DaveM
since he seems to be the maintainer of atomic_ops.txt.
cheers,
grant
>
> >
> > thanks,
> > grant
> >
> > Diff+Commit entry against 2.6.22.5:
> >
> > local_t is a variant of atomic_t and has related ops to match.
> > Add reference for local_t documentation to atomic_ops.txt.
> >
> > Signed-off-by: Grant Grundler <[email protected]>
> >
> >
> > --- 2.6.22.5-ORIG/Documentation/atomic_ops.txt 2007-08-27 22:50:27.000000000 -0700
> > +++ 2.6.22.5-ggg/Documentation/atomic_ops.txt 2007-08-27 22:54:44.000000000 -0700
> > @@ -14,6 +14,10 @@
> >
> > typedef struct { volatile int counter; } atomic_t;
> >
> > +local_t is very similar to atomic_t. If the counter is per CPU and only
> > +updated by one CPU, local_t is probably more appropriate. Please see
> > +Documentation/local_ops.txt for the semantics of local_t.
> > +
> > The first operations to implement for atomic_t's are the
> > initializers and plain reads.
> >
>
> The text snippet is good, but I am not sure it belongs between the
> description of atomic_t type and its initializers. What if we do
> something like: (with context, I tried to explain the distinction
> between atomic_t and local_t some more)
>
>
> Semantics and Behavior of Atomic and
> Bitmask Operations
>
> David S. Miller
>
> This document is intended to serve as a guide to Linux port
> maintainers on how to implement atomic counter, bitops, and spinlock
> interfaces properly.
>
> atomic_t should be used to provide a type with update primitives
> executed atomically from any CPU. If the counter is per CPU and only
> updated by one CPU, local_t is probably more appropriate. Please see
> Documentation/local_ops.txt for the semantics of local_t.
>
> The atomic_t type should be defined as a signed integer.
> Also, it should be made opaque such that any kind of cast to a normal
> C integer type will fail. Something like the following should
> suffice:
>
>
> Mathieu
>
> --
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
* Grant Grundler ([email protected]) wrote:
> On Tue, Aug 28, 2007 at 07:50:18AM -0400, Mathieu Desnoyers wrote:
> ...
> > > A few questions/nits:
> > > o Did you attempt quantify how many places in the kernel could use this?
> > > I'm just trying to get a feel for how useful this really is vs just
> > > using existing mechanisms (that people understand) to implement a
> > > non-SMP-safe counter that protects updates (writes) against interrupts.
> > > If you did, adding some referencs to local_ops.txt would be helpful
> > > so folks could look for examples of "correct usage".
> > >
> >
> > Good question. Since it is useful to implement fast, interrupt
> > reentrant, counters of any kind without disabling interrupts, I think it
> > could be vastely used in the kernel. I also use it in my LTTng kernel
> > tracer implementation to provide very fast buffer management. It is used
> > in LTTng, but could be used for most kind of buffering management too;
> > meaning that we could manage buffers without disabling interrupts.
> >
> > So I don't expect to come with an "upper bound" about where it can be
> > used...
>
> Ok...so I'll try to find one in 2.6.22.5:
> grundler <1855>find -name \*.c | xargs fgrep DEFINE_PER_CPU | fgrep atomic_t
> ./arch/s390/kernel/time.c:static DEFINE_PER_CPU(atomic_t, etr_sync_word);
> grundler <1856>find -name \*.c | xargs fgrep DEFINE_PER_CPU | fgrep local_t
> ./arch/x86_64/kernel/nmi.c:static DEFINE_PER_CPU(local_t, alert_counter);
>
> uhm, I was expecting more than that. Maybe there is some other systemic
> problem with how PER_CPU stuff is used/declared?
>
the local ops has just been standardized in 2.6.22 though a patchset I
did. I would not expect the code to start using them this quickly. Or
maybe is it just that I am doing a terrible marketing job ;)
> In any case, some references to LTT usage would be quite helpful.
> E.g. a list of file and variable names at the end of local_ops.txt file.
>
LTT is not mainlined (yet!) ;)
>
> > > o How can a local_t counter protect updates (writes) against interrupts
> > > but not preemption?
> > > I always thought preemption required some sort of interrupt or trap.
> > > Maybe the local_ops.txt explains that and I just missed it.
> > >
> >
> > "Local atomic operations only guarantee variable modification atomicity
> > wrt the CPU which owns the data. Therefore, care must taken to make sure
> > that only one CPU writes to the local_t data. This is done by using per
> > cpu data and making sure that we modify it from within a preemption safe
> > context." -> therefore, preemption must be disabled around local ops
> > usage. This is required to be pinned to one CPU anyway.
>
> Sorry...the quoted text doesn't answer my question. It's a definition
> of semantics, not an explanation of the "mechanics".
>
> I want to know what happens when (if?) an interrupt occurs in the
> middle of a read/modify/write sequence that isn't prefixed with LOCK
> (or something similar for other arches like "store locked conditional" ops).
>
> Stating the semantics is a good thing - but not a substitution for
> describing how it works for a given architecture. Either in the code
> or in local_ops.txt. Otherwise people like me won't use it because
> we don't believe that (or understand how) it really works.
>
Quoting Intel 64 and IA-32 Architectures Software Developer's Manual
3.2 Instructions
LOCK - Assert LOCK# Signal Prefix
Causes the processor's LOCK# signal to be asserted during execution of
the accompanying instruction (turns the instruction into an atomic
instruction). In a multiprocessor environment, the LOCK# signal insures
that the processor has exclusive use of any shared memory while the
signal is asserted.
And if we take a look at some of the atomic primitives which are used in
i386 local.h:
add (for inc/dec/add/sub)
xadd
cmpxchg
All these instructions, just like any other, can be interrupted by an
external interrupt or cause a trap, exception, or fault. Interrupt
handler are executing between instructions and traps/exceptions/faults
will either execute instead of the faulty instruction or after is has
been executed. In all these cases, each instruction can be seen as
executing atomically wrt the local CPU. This is exactly what permits
asm-i386/local.h to define out the LOCK prefix for UP kernels.
I use the same trick UP kernel are using, but I deploy it in SMP
context, but I require the CPU to be the only one to access the memory
locations written to by the local ops.
Basically, since the memory location is _not_ shared across CPUs for
writing, we can safely write to it without holding the LOCK signal.
> > > DaveM explained updates "in flight" would not be visible to interrupts
> > > and I suspect that's the answer to my question....but then I don't "feel
> > > good" the local_ops are safe to update in interrupts _and_ the process
> > > context kernel. Maybe the relationship between local_ops, preemption,
> > > and interrupts could be explained more carefully in local_ops.txt.
> > >
> >
> > Does the paragraph above explain it enough or should I add some more
> > explanation ?
>
> Please add a bit more detail. If DaveM is correct (he normally is), then
> there must be limits on how the local_t can be used in the kernel process
> and interrupt contexts. I'd like those rules spelled out very clearly
> since it's easy to get wrong and tracking down such a bug is quite painful.
>
> Note: I already missed the one critical sentence about only the "owning"
> CPU can write the value....there seem to be other limitations as well
> with respect to interrupts.
>
Ok, let's give a try at a clear statement:
- Variables touched by local ops must be per cpu variables.
- _Only_ the CPU owner of these variables must write to them.
- This CPU can use local ops from any context (process, irq, softirq, nmi, ...)
to update its local_t variables.
- Preemption (or interrupts) must be disabled when using local ops in
process context to make sure the process won't be migrated to a
different CPU between getting the per-cpu variable and doing the
actual local op.
- When using local ops in interrupt context, no special care must be
taken on a mainline kernel, since they will run on the local CPU with
preemption already disabled. I suggest, however, to explicitly
disable preemption anyway to make sure it will still work correctly on
-rt kernels.
- Reading the local cpu variable will provide the current copy of the
variable.
- Reads of these variables can be done from any CPU, because updates to
"long", aligned, variables are always atomic. Since no memory
synchronization is done by the writer CPU, an outdated copy of the
variable can be read when reading some _other_ cpu's variables.
> > > o OK to add a reference for local_ops.txt to atomic_ops.txt?
> > > They are obviously related and anyone "discovering" one of the docs
> > > should be made aware of the other.
> > > Patch+log entry appended below. Please sign-off if that's ok with you.
> > >
> >
> > I'm perfectly ok with the idea, but suggest a small modification. See
> > below.
>
> Looks fine to me. Add your "Signed-off-by" and submit to DaveM
> since he seems to be the maintainer of atomic_ops.txt.
>
Signed-off-by: Mathieu Desnoyers <[email protected]>
Thanks,
Mathieu
> cheers,
> grant
>
> >
> > >
> > > thanks,
> > > grant
> > >
> > > Diff+Commit entry against 2.6.22.5:
> > >
> > > local_t is a variant of atomic_t and has related ops to match.
> > > Add reference for local_t documentation to atomic_ops.txt.
> > >
> > > Signed-off-by: Grant Grundler <[email protected]>
> > >
> > >
> > > --- 2.6.22.5-ORIG/Documentation/atomic_ops.txt 2007-08-27 22:50:27.000000000 -0700
> > > +++ 2.6.22.5-ggg/Documentation/atomic_ops.txt 2007-08-27 22:54:44.000000000 -0700
> > > @@ -14,6 +14,10 @@
> > >
> > > typedef struct { volatile int counter; } atomic_t;
> > >
> > > +local_t is very similar to atomic_t. If the counter is per CPU and only
> > > +updated by one CPU, local_t is probably more appropriate. Please see
> > > +Documentation/local_ops.txt for the semantics of local_t.
> > > +
> > > The first operations to implement for atomic_t's are the
> > > initializers and plain reads.
> > >
> >
> > The text snippet is good, but I am not sure it belongs between the
> > description of atomic_t type and its initializers. What if we do
> > something like: (with context, I tried to explain the distinction
> > between atomic_t and local_t some more)
> >
> >
> > Semantics and Behavior of Atomic and
> > Bitmask Operations
> >
> > David S. Miller
> >
> > This document is intended to serve as a guide to Linux port
> > maintainers on how to implement atomic counter, bitops, and spinlock
> > interfaces properly.
> >
> > atomic_t should be used to provide a type with update primitives
> > executed atomically from any CPU. If the counter is per CPU and only
> > updated by one CPU, local_t is probably more appropriate. Please see
> > Documentation/local_ops.txt for the semantics of local_t.
> >
> > The atomic_t type should be defined as a signed integer.
> > Also, it should be made opaque such that any kind of cast to a normal
> > C integer type will fail. Something like the following should
> > suffice:
> >
> >
> > Mathieu
> >
> > --
> > Mathieu Desnoyers
> > Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
[davem: patch for you at the bottom to Documentation/atomic_ops.txt ]
On Tue, Aug 28, 2007 at 02:38:35PM -0400, Mathieu Desnoyers wrote:
> * Grant Grundler ([email protected]) wrote:
> > On Tue, Aug 28, 2007 at 07:50:18AM -0400, Mathieu Desnoyers wrote:
...
> > > So I don't expect to come with an "upper bound" about where it can be
> > > used...
> >
> > Ok...so I'll try to find one in 2.6.22.5:
> > grundler <1855>find -name \*.c | xargs fgrep DEFINE_PER_CPU | fgrep atomic_t
> > ./arch/s390/kernel/time.c:static DEFINE_PER_CPU(atomic_t, etr_sync_word);
> > grundler <1856>find -name \*.c | xargs fgrep DEFINE_PER_CPU | fgrep local_t
> > ./arch/x86_64/kernel/nmi.c:static DEFINE_PER_CPU(local_t, alert_counter);
> >
> > uhm, I was expecting more than that. Maybe there is some other systemic
> > problem with how PER_CPU stuff is used/declared?
>
> the local ops has just been standardized in 2.6.22 though a patchset I
> did. I would not expect the code to start using them this quickly. Or
> maybe is it just that I am doing a terrible marketing job ;)
Yeah, I didn't expect many users of local_t.
The search for atomic_t usage in DEFINE_PER_CPU was an attempt to
find other potential candidates which could be local_t.
Is there any other programmatic way we could look for code which
could (or should) be using local_t?
> > In any case, some references to LTT usage would be quite helpful.
> > E.g. a list of file and variable names at the end of local_ops.txt file.
> >
>
> LTT is not mainlined (yet!) ;)
Ok...probably not such a good example then. :)
...
> > Sorry...the quoted text doesn't answer my question. It's a definition
> > of semantics, not an explanation of the "mechanics".
> >
> > I want to know what happens when (if?) an interrupt occurs in the
> > middle of a read/modify/write sequence that isn't prefixed with LOCK
> > (or something similar for other arches like "store locked conditional" ops).
> >
> > Stating the semantics is a good thing - but not a substitution for
> > describing how it works for a given architecture. Either in the code
> > or in local_ops.txt. Otherwise people like me won't use it because
> > we don't believe that (or understand how) it really works.
> >
>
> Quoting Intel 64 and IA-32 Architectures Software Developer's Manual
>
> 3.2 Instructions
> LOCK - Assert LOCK# Signal Prefix
...
I've read this before and understand how LOCK works. This isn't helpful
since I want a description of the behavior without LOCK.
> And if we take a look at some of the atomic primitives which are used in
> i386 local.h:
>
> add (for inc/dec/add/sub)
> xadd
> cmpxchg
>
> All these instructions, just like any other, can be interrupted by an
> external interrupt or cause a trap, exception, or fault. Interrupt
> handler are executing between instructions and traps/exceptions/faults
> will either execute instead of the faulty instruction or after is has
> been executed. In all these cases, each instruction can be seen as
> executing atomically wrt the local CPU. This is exactly what permits
> asm-i386/local.h to define out the LOCK prefix for UP kernels.
I think what I'm looking for but don't know if it's true:
The cmpxchg (for example) at the kernel process context will not
clobber or be clobbered by a cmpxchg done to the same local_t
performed at the kernel interrupt context by the same CPU.
If that's not true, then it would be good to add that as another
restriction to usage.
> I use the same trick UP kernel are using, but I deploy it in SMP
> context, but I require the CPU to be the only one to access the memory
> locations written to by the local ops.
>
> Basically, since the memory location is _not_ shared across CPUs for
> writing, we can safely write to it without holding the LOCK signal.
ok.
...
> > Note: I already missed the one critical sentence about only the "owning"
> > CPU can write the value....there seem to be other limitations as well
> > with respect to interrupts.
> >
>
> Ok, let's give a try at a clear statement:
>
> - Variables touched by local ops must be per cpu variables.
> - _Only_ the CPU owner of these variables must write to them.
> - This CPU can use local ops from any context (process, irq, softirq, nmi, ...)
> to update its local_t variables.
> - Preemption (or interrupts) must be disabled when using local ops in
> process context to make sure the process won't be migrated to a
> different CPU between getting the per-cpu variable and doing the
> actual local op.
> - When using local ops in interrupt context, no special care must be
> taken on a mainline kernel, since they will run on the local CPU with
> preemption already disabled. I suggest, however, to explicitly
> disable preemption anyway to make sure it will still work correctly on
> -rt kernels.
> - Reading the local cpu variable will provide the current copy of the
> variable.
> - Reads of these variables can be done from any CPU, because updates to
> "long", aligned, variables are always atomic. Since no memory
> synchronization is done by the writer CPU, an outdated copy of the
> variable can be read when reading some _other_ cpu's variables.
Perfect! Ship it! ;)
Can you submit a patch to add the above to Documentation/local_ops.txt ?
...
> > Looks fine to me. Add your "Signed-off-by" and submit to DaveM
> > since he seems to be the maintainer of atomic_ops.txt.
>
> Signed-off-by: Mathieu Desnoyers <[email protected]>
heh...I think DaveM will want it in git or universal patch form. :)
Patch appended below.
thanks!
grant
Add document reference and simple use example of local_t.
Signed-off-by: Grant Grundler <[email protected]>
Signed-off-by: Mathieu Desnoyers <[email protected]>
--- linux-2.6.22.5-ORIG/Documentation/atomic_ops.txt 2007-08-22 16:23:54.000000000 -0700
+++ linux-2.6.22.5-ggg/Documentation/atomic_ops.txt 2007-08-28 22:57:26.000000000 -0700
@@ -7,6 +7,11 @@
maintainers on how to implement atomic counter, bitops, and spinlock
interfaces properly.
+ atomic_t should be used to provide a type with update primitives
+executed atomically from any CPU. If the counter is per CPU and only
+updated by one CPU, local_t is probably more appropriate. Please see
+Documentation/local_ops.txt for the semantics of local_t.
+
The atomic_t type should be defined as a signed integer.
Also, it should be made opaque such that any kind of cast to a normal
C integer type will fail. Something like the following should
From: Grant Grundler <[email protected]>
Date: Tue, 28 Aug 2007 23:59:55 -0600
> [davem: patch for you at the bottom to Documentation/atomic_ops.txt ]
Looks fine to me.
* Grant Grundler ([email protected]) wrote:
> [davem: patch for you at the bottom to Documentation/atomic_ops.txt ]
>
> On Tue, Aug 28, 2007 at 02:38:35PM -0400, Mathieu Desnoyers wrote:
> > * Grant Grundler ([email protected]) wrote:
> > > On Tue, Aug 28, 2007 at 07:50:18AM -0400, Mathieu Desnoyers wrote:
> ...
> > > > So I don't expect to come with an "upper bound" about where it can be
> > > > used...
> > >
> > > Ok...so I'll try to find one in 2.6.22.5:
> > > grundler <1855>find -name \*.c | xargs fgrep DEFINE_PER_CPU | fgrep atomic_t
> > > ./arch/s390/kernel/time.c:static DEFINE_PER_CPU(atomic_t, etr_sync_word);
> > > grundler <1856>find -name \*.c | xargs fgrep DEFINE_PER_CPU | fgrep local_t
> > > ./arch/x86_64/kernel/nmi.c:static DEFINE_PER_CPU(local_t, alert_counter);
> > >
> > > uhm, I was expecting more than that. Maybe there is some other systemic
> > > problem with how PER_CPU stuff is used/declared?
> >
> > the local ops has just been standardized in 2.6.22 though a patchset I
> > did. I would not expect the code to start using them this quickly. Or
> > maybe is it just that I am doing a terrible marketing job ;)
>
> Yeah, I didn't expect many users of local_t.
>
> The search for atomic_t usage in DEFINE_PER_CPU was an attempt to
> find other potential candidates which could be local_t.
> Is there any other programmatic way we could look for code which
> could (or should) be using local_t?
>
Yep.. atomic_t static variables used as counters are a perfect match:
they are updated very often and read rarely. Therefore, it is
preferable to have per cpu updates with very cheap locking and to
iterate on all of them to sum them when they should be printed. That's
just an example, but with it comes vmstat and all sorts of counters.
> > > In any case, some references to LTT usage would be quite helpful.
> > > E.g. a list of file and variable names at the end of local_ops.txt file.
> > >
> >
> > LTT is not mainlined (yet!) ;)
>
> Ok...probably not such a good example then. :)
>
> ...
> > > Sorry...the quoted text doesn't answer my question. It's a definition
> > > of semantics, not an explanation of the "mechanics".
> > >
> > > I want to know what happens when (if?) an interrupt occurs in the
> > > middle of a read/modify/write sequence that isn't prefixed with LOCK
> > > (or something similar for other arches like "store locked conditional" ops).
> > >
> > > Stating the semantics is a good thing - but not a substitution for
> > > describing how it works for a given architecture. Either in the code
> > > or in local_ops.txt. Otherwise people like me won't use it because
> > > we don't believe that (or understand how) it really works.
> > >
> >
> > Quoting Intel 64 and IA-32 Architectures Software Developer's Manual
> >
> > 3.2 Instructions
> > LOCK - Assert LOCK# Signal Prefix
> ...
>
> I've read this before and understand how LOCK works. This isn't helpful
> since I want a description of the behavior without LOCK.
>
> > And if we take a look at some of the atomic primitives which are used in
> > i386 local.h:
> >
> > add (for inc/dec/add/sub)
> > xadd
> > cmpxchg
> >
> > All these instructions, just like any other, can be interrupted by an
> > external interrupt or cause a trap, exception, or fault. Interrupt
> > handler are executing between instructions and traps/exceptions/faults
> > will either execute instead of the faulty instruction or after is has
> > been executed. In all these cases, each instruction can be seen as
> > executing atomically wrt the local CPU. This is exactly what permits
> > asm-i386/local.h to define out the LOCK prefix for UP kernels.
>
> I think what I'm looking for but don't know if it's true:
> The cmpxchg (for example) at the kernel process context will not
> clobber or be clobbered by a cmpxchg done to the same local_t
> performed at the kernel interrupt context by the same CPU.
>
> If that's not true, then it would be good to add that as another
> restriction to usage.
>
Each of these cmpxchg will execute atomically wrt the local CPU, so
there is no limitation there.
> > I use the same trick UP kernel are using, but I deploy it in SMP
> > context, but I require the CPU to be the only one to access the memory
> > locations written to by the local ops.
> >
> > Basically, since the memory location is _not_ shared across CPUs for
> > writing, we can safely write to it without holding the LOCK signal.
>
> ok.
>
> ...
> > > Note: I already missed the one critical sentence about only the "owning"
> > > CPU can write the value....there seem to be other limitations as well
> > > with respect to interrupts.
> > >
> >
> > Ok, let's give a try at a clear statement:
> >
> > - Variables touched by local ops must be per cpu variables.
> > - _Only_ the CPU owner of these variables must write to them.
> > - This CPU can use local ops from any context (process, irq, softirq, nmi, ...)
> > to update its local_t variables.
> > - Preemption (or interrupts) must be disabled when using local ops in
> > process context to make sure the process won't be migrated to a
> > different CPU between getting the per-cpu variable and doing the
> > actual local op.
> > - When using local ops in interrupt context, no special care must be
> > taken on a mainline kernel, since they will run on the local CPU with
> > preemption already disabled. I suggest, however, to explicitly
> > disable preemption anyway to make sure it will still work correctly on
> > -rt kernels.
> > - Reading the local cpu variable will provide the current copy of the
> > variable.
> > - Reads of these variables can be done from any CPU, because updates to
> > "long", aligned, variables are always atomic. Since no memory
> > synchronization is done by the writer CPU, an outdated copy of the
> > variable can be read when reading some _other_ cpu's variables.
>
> Perfect! Ship it! ;)
> Can you submit a patch to add the above to Documentation/local_ops.txt ?
>
Hehe sure :) I'll prepare one.
> ...
> > > Looks fine to me. Add your "Signed-off-by" and submit to DaveM
> > > since he seems to be the maintainer of atomic_ops.txt.
> >
> > Signed-off-by: Mathieu Desnoyers <[email protected]>
>
> heh...I think DaveM will want it in git or universal patch form. :)
> Patch appended below.
>
> thanks!
> grant
>
> Add document reference and simple use example of local_t.
>
> Signed-off-by: Grant Grundler <[email protected]>
> Signed-off-by: Mathieu Desnoyers <[email protected]>
>
> --- linux-2.6.22.5-ORIG/Documentation/atomic_ops.txt 2007-08-22 16:23:54.000000000 -0700
> +++ linux-2.6.22.5-ggg/Documentation/atomic_ops.txt 2007-08-28 22:57:26.000000000 -0700
> @@ -7,6 +7,11 @@
> maintainers on how to implement atomic counter, bitops, and spinlock
> interfaces properly.
>
> + atomic_t should be used to provide a type with update primitives
> +executed atomically from any CPU. If the counter is per CPU and only
> +updated by one CPU, local_t is probably more appropriate. Please see
> +Documentation/local_ops.txt for the semantics of local_t.
> +
> The atomic_t type should be defined as a signed integer.
> Also, it should be made opaque such that any kind of cast to a normal
> C integer type will fail. Something like the following should
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68