We already have cmpxchg_local that can work on an arbitrary type. The cmpxchg
is only safe from concurrent access on the local cpu.
Add another operation that in the similar fasion allow "local" safe adds to
a variable.
Signed-off-by: Christoph Lameter <[email protected]>
---
arch/alpha/include/asm/add-local.h | 2 +
arch/arm/include/asm/add-local.h | 2 +
arch/avr32/include/asm/add-local.h | 2 +
arch/blackfin/include/asm/add-local.h | 2 +
arch/cris/include/asm/add-local.h | 2 +
arch/frv/include/asm/add-local.h | 2 +
arch/h8300/include/asm/add-local.h | 2 +
arch/ia64/include/asm/add-local.h | 2 +
arch/m32r/include/asm/add-local.h | 2 +
arch/m68k/include/asm/add-local.h | 2 +
arch/microblaze/include/asm/add-local.h | 2 +
arch/mips/include/asm/add-local.h | 2 +
arch/mn10300/include/asm/add-local.h | 2 +
arch/parisc/include/asm/add-local.h | 2 +
arch/powerpc/include/asm/add-local.h | 2 +
arch/s390/include/asm/add-local.h | 2 +
arch/score/include/asm/add-local.h | 2 +
arch/sh/include/asm/add-local.h | 2 +
arch/sparc/include/asm/add-local.h | 2 +
arch/um/include/asm/add-local.h | 2 +
arch/x86/include/asm/add-local.h | 2 +
arch/xtensa/include/asm/add-local.h | 2 +
include/asm-generic/add-local-generic.h | 40 ++++++++++++++++++++++++++++++++
include/asm-generic/add-local.h | 13 ++++++++++
24 files changed, 97 insertions(+)
Index: linux-2.6/include/asm-generic/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/asm-generic/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,13 @@
+#ifndef __ASM_GENERIC_ADD_LOCAL_H
+#define __ASM_GENERIC_ADD_LOCAL_H
+
+#include <asm-generic/add-local-generic.h>
+
+#define add_return_local(ptr, v) \
+ ((__typeof__(*(ptr)))__add_return_local_generic((ptr), \
+ (unsigned long)(v), sizeof(*(ptr))))
+
+#define add_local(ptr, v) (void)__add_return_local_generic((ptr), \
+ (unsigned long)(v), sizeof(*(ptr)))
+
+#endif
Index: linux-2.6/arch/alpha/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/alpha/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/arm/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/arm/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/avr32/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/avr32/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/blackfin/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/blackfin/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/cris/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/cris/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/frv/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/frv/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/h8300/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/h8300/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/ia64/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/ia64/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/m32r/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/m32r/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/m68k/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/m68k/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/microblaze/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/microblaze/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/mips/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/mips/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/mn10300/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/mn10300/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/parisc/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/parisc/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/powerpc/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/powerpc/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/s390/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/s390/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/score/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/score/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/sh/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/sh/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/sparc/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/sparc/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/um/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/um/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/x86/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/x86/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/arch/xtensa/include/asm/add-local.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/arch/xtensa/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,2 @@
+#include <asm-generic/add-local.h>
+
Index: linux-2.6/include/asm-generic/add-local-generic.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/asm-generic/add-local-generic.h 2010-01-05 15:36:02.000000000 -0600
@@ -0,0 +1,40 @@
+#ifndef __ASM_GENERIC_ADD_LOCAL_GENERIC_H
+#define __ASM_GENERIC_ADD_LOCAL_GENERIC_H
+
+#include <linux/types.h>
+
+extern unsigned long wrong_size_add_local(volatile void *ptr);
+
+/*
+ * Generic version of __add_return_local (disables interrupts). Takes an
+ * unsigned long parameter, supporting various types of architectures.
+ */
+static inline unsigned long __add_return_local_generic(volatile void *ptr,
+ unsigned long value, int size)
+{
+ unsigned long flags, r;
+
+ /*
+ * Sanity checking, compile-time.
+ */
+ if (size == 8 && sizeof(unsigned long) != 8)
+ wrong_size_add_local(ptr);
+
+ local_irq_save(flags);
+ switch (size) {
+ case 1: r = (*((u8 *)ptr) += value);
+ break;
+ case 2: r = (*((u16 *)ptr) += value);
+ break;
+ case 4: r = (*((u32 *)ptr) += value);
+ break;
+ case 8: r = (*((u64 *)ptr) += value);
+ break;
+ default:
+ wrong_size_add_local(ptr);
+ }
+ local_irq_restore(flags);
+ return r;
+}
+
+#endif
--
On Tue, Jan 5, 2010 at 17:04, Christoph Lameter wrote:
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/include/asm-generic/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,13 @@
> +#ifndef __ASM_GENERIC_ADD_LOCAL_H
> +#define __ASM_GENERIC_ADD_LOCAL_H
needs comment header (blurb/copyright/license)
> +#define add_return_local(ptr, v) \
> + ((__typeof__(*(ptr)))__add_return_local_generic((ptr), \
> + (unsigned long)(v), sizeof(*(ptr))))
> +
> +#define add_local(ptr, v) (void)__add_return_local_generic((ptr), \
> + (unsigned long)(v), sizeof(*(ptr)))
the only difference here is the cast ... perhaps define an
_add_local() macro to avoid the duplication
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/alpha/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
these arch stubs all have an extra new line
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/include/asm-generic/add-local-generic.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,40 @@
> +#ifndef __ASM_GENERIC_ADD_LOCAL_GENERIC_H
> +#define __ASM_GENERIC_ADD_LOCAL_GENERIC_H
needs comment header (blurb/copyright/license)
> +/*
> + * Generic version of __add_return_local (disables interrupts). Takes an
> + * unsigned long parameter, supporting various types of architectures.
> + */
> +static inline unsigned long __add_return_local_generic(volatile void *ptr,
> + unsigned long value, int size)
size_t for size ?
> +extern unsigned long wrong_size_add_local(volatile void *ptr);
> ...
> + /*
> + * Sanity checking, compile-time.
> + */
> + if (size == 8 && sizeof(unsigned long) != 8)
> + wrong_size_add_local(ptr);
> ...
> + default:
> + wrong_size_add_local(ptr);
> + }
should be BUILD_BUG_ON() ?
-mike
On Tue, 5 Jan 2010, Mike Frysinger wrote:
> On Tue, Jan 5, 2010 at 17:04, Christoph Lameter wrote:
> > --- /dev/null ? 1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6/include/asm-generic/add-local.h ? 2010-01-05 15:36:02.000000000 -0600
> > @@ -0,0 +1,13 @@
> > +#ifndef __ASM_GENERIC_ADD_LOCAL_H
> > +#define __ASM_GENERIC_ADD_LOCAL_H
>
> needs comment header (blurb/copyright/license)
A simple small include file? Really?
> > +#define add_return_local(ptr, v) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> > + ? ? ? ((__typeof__(*(ptr)))__add_return_local_generic((ptr), ? ? ? ? ?\
> > + ? ? ? ? ? ? ? ? ? ? ? (unsigned long)(v), sizeof(*(ptr))))
> > +
> > +#define add_local(ptr, v) (void)__add_return_local_generic((ptr), ? ? ?\
> > + ? ? ? ? ? ? ? ? ? ? ? (unsigned long)(v), sizeof(*(ptr)))
>
> the only difference here is the cast ... perhaps define an
> _add_local() macro to avoid the duplication
Right. This is a shortcut to define the add_local() operation without too
much effort. We should be using add / inc /dec there at some point.
> > --- /dev/null ? 1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6/arch/alpha/include/asm/add-local.h ? ? ? ?2010-01-05 15:36:02.000000000 -0600
> > @@ -0,0 +1,2 @@
> > +#include <asm-generic/add-local.h>
> > +
>
> these arch stubs all have an extra new line
Thats bad?
> > +/*
> > + * Generic version of __add_return_local (disables interrupts). Takes an
> > + * unsigned long parameter, supporting various types of architectures.
> > + */
> > +static inline unsigned long __add_return_local_generic(volatile void *ptr,
> > + ? ? ? ? ? ? ? unsigned long value, int size)
>
> size_t for size ?
No. It can be anything.
> > +extern unsigned long wrong_size_add_local(volatile void *ptr);
> > ...
> > + ? ? ? /*
> > + ? ? ? ?* Sanity checking, compile-time.
> > + ? ? ? ?*/
> > + ? ? ? if (size == 8 && sizeof(unsigned long) != 8)
> > + ? ? ? ? ? ? ? wrong_size_add_local(ptr);
> > ...
> > + ? ? ? default:
> > + ? ? ? ? ? ? ? wrong_size_add_local(ptr);
> > + ? ? ? }
>
> should be BUILD_BUG_ON() ?
Does not work there.
* Christoph Lameter ([email protected]) wrote:
> We already have cmpxchg_local that can work on an arbitrary type. The cmpxchg
> is only safe from concurrent access on the local cpu.
>
> Add another operation that in the similar fasion allow "local" safe adds to
> a variable.
>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> ---
> arch/alpha/include/asm/add-local.h | 2 +
> arch/arm/include/asm/add-local.h | 2 +
> arch/avr32/include/asm/add-local.h | 2 +
> arch/blackfin/include/asm/add-local.h | 2 +
> arch/cris/include/asm/add-local.h | 2 +
> arch/frv/include/asm/add-local.h | 2 +
> arch/h8300/include/asm/add-local.h | 2 +
> arch/ia64/include/asm/add-local.h | 2 +
> arch/m32r/include/asm/add-local.h | 2 +
> arch/m68k/include/asm/add-local.h | 2 +
> arch/microblaze/include/asm/add-local.h | 2 +
> arch/mips/include/asm/add-local.h | 2 +
> arch/mn10300/include/asm/add-local.h | 2 +
> arch/parisc/include/asm/add-local.h | 2 +
> arch/powerpc/include/asm/add-local.h | 2 +
> arch/s390/include/asm/add-local.h | 2 +
> arch/score/include/asm/add-local.h | 2 +
> arch/sh/include/asm/add-local.h | 2 +
> arch/sparc/include/asm/add-local.h | 2 +
> arch/um/include/asm/add-local.h | 2 +
> arch/x86/include/asm/add-local.h | 2 +
> arch/xtensa/include/asm/add-local.h | 2 +
> include/asm-generic/add-local-generic.h | 40 ++++++++++++++++++++++++++++++++
> include/asm-generic/add-local.h | 13 ++++++++++
> 24 files changed, 97 insertions(+)
The problem I see here is that with ~5-6 operations, we will end up
having 20*5 = 100 headers only for this. Can we combine these in a
single header file instead ? local.h wasn't bad in this respect.
Also, separating all these in sub-files will make it a bit difficult to
pinpoint errors that would arise from using a bad combination of, e.g.
generic add with a non-protected read or set.
Thanks,
Mathieu
>
> Index: linux-2.6/include/asm-generic/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/include/asm-generic/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,13 @@
> +#ifndef __ASM_GENERIC_ADD_LOCAL_H
> +#define __ASM_GENERIC_ADD_LOCAL_H
> +
> +#include <asm-generic/add-local-generic.h>
> +
> +#define add_return_local(ptr, v) \
> + ((__typeof__(*(ptr)))__add_return_local_generic((ptr), \
> + (unsigned long)(v), sizeof(*(ptr))))
> +
> +#define add_local(ptr, v) (void)__add_return_local_generic((ptr), \
> + (unsigned long)(v), sizeof(*(ptr)))
> +
> +#endif
> Index: linux-2.6/arch/alpha/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/alpha/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/arm/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/arm/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/avr32/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/avr32/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/blackfin/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/blackfin/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/cris/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/cris/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/frv/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/frv/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/h8300/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/h8300/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/ia64/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/ia64/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/m32r/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/m32r/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/m68k/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/m68k/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/microblaze/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/microblaze/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/mips/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/mips/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/mn10300/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/mn10300/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/parisc/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/parisc/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/powerpc/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/powerpc/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/s390/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/s390/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/score/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/score/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/sh/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/sh/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/sparc/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/sparc/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/um/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/um/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/x86/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/x86/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/arch/xtensa/include/asm/add-local.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/arch/xtensa/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,2 @@
> +#include <asm-generic/add-local.h>
> +
> Index: linux-2.6/include/asm-generic/add-local-generic.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/include/asm-generic/add-local-generic.h 2010-01-05 15:36:02.000000000 -0600
> @@ -0,0 +1,40 @@
> +#ifndef __ASM_GENERIC_ADD_LOCAL_GENERIC_H
> +#define __ASM_GENERIC_ADD_LOCAL_GENERIC_H
> +
> +#include <linux/types.h>
> +
> +extern unsigned long wrong_size_add_local(volatile void *ptr);
> +
> +/*
> + * Generic version of __add_return_local (disables interrupts). Takes an
> + * unsigned long parameter, supporting various types of architectures.
> + */
> +static inline unsigned long __add_return_local_generic(volatile void *ptr,
> + unsigned long value, int size)
> +{
> + unsigned long flags, r;
> +
> + /*
> + * Sanity checking, compile-time.
> + */
> + if (size == 8 && sizeof(unsigned long) != 8)
> + wrong_size_add_local(ptr);
> +
> + local_irq_save(flags);
> + switch (size) {
> + case 1: r = (*((u8 *)ptr) += value);
> + break;
> + case 2: r = (*((u16 *)ptr) += value);
> + break;
> + case 4: r = (*((u32 *)ptr) += value);
> + break;
> + case 8: r = (*((u64 *)ptr) += value);
> + break;
> + default:
> + wrong_size_add_local(ptr);
> + }
> + local_irq_restore(flags);
> + return r;
> +}
> +
> +#endif
>
> --
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, Jan 5, 2010 at 17:29, Christoph Lameter wrote:
> On Tue, 5 Jan 2010, Mike Frysinger wrote:
>> On Tue, Jan 5, 2010 at 17:04, Christoph Lameter wrote:
>> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
>> > +++ linux-2.6/include/asm-generic/add-local.h 2010-01-05 15:36:02.000000000 -0600
>> > @@ -0,0 +1,13 @@
>> > +#ifndef __ASM_GENERIC_ADD_LOCAL_H
>> > +#define __ASM_GENERIC_ADD_LOCAL_H
>>
>> needs comment header (blurb/copyright/license)
>
> A simple small include file? Really?
if there's enough content to warrant protection against multiple
inclusion, then generally yes
>> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
>> > +++ linux-2.6/arch/alpha/include/asm/add-local.h 2010-01-05 15:36:02.000000000 -0600
>> > @@ -0,0 +1,2 @@
>> > +#include <asm-generic/add-local.h>
>> > +
>>
>> these arch stubs all have an extra new line
>
> Thats bad?
files shouldnt have trailing new lines
>> > +/*
>> > + * Generic version of __add_return_local (disables interrupts). Takes an
>> > + * unsigned long parameter, supporting various types of architectures.
>> > + */
>> > +static inline unsigned long __add_return_local_generic(volatile void *ptr,
>> > + unsigned long value, int size)
>>
>> size_t for size ?
>
> No. It can be anything.
you're passing it sizeof() which returns a size_t
>> > +extern unsigned long wrong_size_add_local(volatile void *ptr);
>> > ...
>> > + /*
>> > + * Sanity checking, compile-time.
>> > + */
>> > + if (size == 8 && sizeof(unsigned long) != 8)
>> > + wrong_size_add_local(ptr);
>> > ...
>> > + default:
>> > + wrong_size_add_local(ptr);
>> > + }
>>
>> should be BUILD_BUG_ON() ?
>
> Does not work there.
why not ? BUILD_BUG_ON() should work on any compile-time constant
which sizeof() is ... unless you plan on letting people call this
function with arbitrary sizes ?
-mike
On Tuesday 05 January 2010, Mathieu Desnoyers wrote:
>
> The problem I see here is that with ~5-6 operations, we will end up
> having 20*5 = 100 headers only for this. Can we combine these in a
> single header file instead ? local.h wasn't bad in this respect.
I have an old patch that I was planning to dig out for 2.6.34,
which autogenerates arch/*/include/foo.h files that only contain
"#include <asm-generic/foo.h>".
I guess this would be sufficient to avoid the overload with all
these header files.
> Also, separating all these in sub-files will make it a bit difficult to
> pinpoint errors that would arise from using a bad combination of, e.g.
> generic add with a non-protected read or set.
Yes.
> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6/include/asm-generic/add-local-generic.h 2010-01-05 15:36:02.000000000 -0600
> > @@ -0,0 +1,40 @@
> > +#ifndef __ASM_GENERIC_ADD_LOCAL_GENERIC_H
> > +#define __ASM_GENERIC_ADD_LOCAL_GENERIC_H
Why split the file between asm-generic/add-local.h and add-local-generic.h?
I don't see how any architecture could use one but not the other.
> > +#include <linux/types.h>
> > +
> > +extern unsigned long wrong_size_add_local(volatile void *ptr);
> > +
> > +/*
> > + * Generic version of __add_return_local (disables interrupts). Takes an
> > + * unsigned long parameter, supporting various types of architectures.
> > + */
> > +static inline unsigned long __add_return_local_generic(volatile void *ptr,
> > + unsigned long value, int size)
You could probably lose the 'volatile' here, if you want to discourage
marking data as volatile in the code.
> > +{
> > + unsigned long flags, r;
> > +
> > + /*
> > + * Sanity checking, compile-time.
> > + */
> > + if (size == 8 && sizeof(unsigned long) != 8)
> > + wrong_size_add_local(ptr);
It can be BUILD_BUG_ON if you move it to the outer macro.
> > + local_irq_save(flags);
> > + switch (size) {
> > + case 1: r = (*((u8 *)ptr) += value);
> > + break;
> > + case 2: r = (*((u16 *)ptr) += value);
> > + break;
> > + case 4: r = (*((u32 *)ptr) += value);
> > + break;
> > + case 8: r = (*((u64 *)ptr) += value);
> > + break;
But I think here you actually need to add the volatile in order
to make these atomic assignments.
Arnd
* Arnd Bergmann ([email protected]) wrote:
> On Tuesday 05 January 2010, Mathieu Desnoyers wrote:
> >
> > The problem I see here is that with ~5-6 operations, we will end up
> > having 20*5 = 100 headers only for this. Can we combine these in a
> > single header file instead ? local.h wasn't bad in this respect.
>
> I have an old patch that I was planning to dig out for 2.6.34,
> which autogenerates arch/*/include/foo.h files that only contain
> "#include <asm-generic/foo.h>".
>
> I guess this would be sufficient to avoid the overload with all
> these header files.
Well, given we already have local.h, I am not completely sure that this
whole exercise is giving us.
[...]
> > > +#include <linux/types.h>
> > > +
> > > +extern unsigned long wrong_size_add_local(volatile void *ptr);
> > > +
> > > +/*
> > > + * Generic version of __add_return_local (disables interrupts). Takes an
> > > + * unsigned long parameter, supporting various types of architectures.
> > > + */
> > > +static inline unsigned long __add_return_local_generic(volatile void *ptr,
> > > + unsigned long value, int size)
>
> You could probably lose the 'volatile' here, if you want to discourage
> marking data as volatile in the code.
>
> > > +{
> > > + unsigned long flags, r;
> > > +
> > > + /*
> > > + * Sanity checking, compile-time.
> > > + */
> > > + if (size == 8 && sizeof(unsigned long) != 8)
> > > + wrong_size_add_local(ptr);
>
> It can be BUILD_BUG_ON if you move it to the outer macro.
>
> > > + local_irq_save(flags);
> > > + switch (size) {
> > > + case 1: r = (*((u8 *)ptr) += value);
> > > + break;
> > > + case 2: r = (*((u16 *)ptr) += value);
> > > + break;
> > > + case 4: r = (*((u32 *)ptr) += value);
> > > + break;
> > > + case 8: r = (*((u64 *)ptr) += value);
> > > + break;
>
> But I think here you actually need to add the volatile in order
> to make these atomic assignments.
Yes, you are right. If we ever try to access these variables from a
remote CPU with a load (but not with any concurrent store operation, as
this would be semantically invalid), then the volatile is important.
Mathieu
>
> Arnd
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Thursday 07 January 2010, Mathieu Desnoyers wrote:
> * Arnd Bergmann ([email protected]) wrote:
> > > > + local_irq_save(flags);
> > > > + switch (size) {
> > > > + case 1: r = (*((u8 *)ptr) += value);
> > > > + break;
> > > > + case 2: r = (*((u16 *)ptr) += value);
> > > > + break;
> > > > + case 4: r = (*((u32 *)ptr) += value);
> > > > + break;
> > > > + case 8: r = (*((u64 *)ptr) += value);
> > > > + break;
> >
> > But I think here you actually need to add the volatile in order
> > to make these atomic assignments.
>
> Yes, you are right. If we ever try to access these variables from a
> remote CPU with a load (but not with any concurrent store operation, as
> this would be semantically invalid), then the volatile is important.
Just to make sure everyone has the same understanding: We need the volatile
in the cast in these lines, not the one in the function prototype which
only serves to avoid warnings but has no impact on the object code when
we cast the pointer to a non-volatile type for the assignment.
Arnd
On Wed, 6 Jan 2010, Mike Frysinger wrote:
> >> > +static inline unsigned long __add_return_local_generic(volatile void *ptr,
> >> > + unsigned long value, int size)
> >>
> >> size_t for size ?
> >
> > No. It can be anything.
>
> you're passing it sizeof() which returns a size_t
Ahh not value but the size parameter... Ok.
On Tue, 5 Jan 2010, Mathieu Desnoyers wrote:
> The problem I see here is that with ~5-6 operations, we will end up
> having 20*5 = 100 headers only for this. Can we combine these in a
> single header file instead ? local.h wasn't bad in this respect.
We could actually keep local.h and just thin it out a bit. Get rid of the
local_t type and use long instead? Then make the local_inc/local_add work
on an arbitrary scalar like cmpxchg_local?
> Also, separating all these in sub-files will make it a bit difficult to
> pinpoint errors that would arise from using a bad combination of, e.g.
> generic add with a non-protected read or set.
Yes surely I dont want many files. I thought about adding
#define inc_local(x) add_local(x, 1)
etc to add-local.h