Provide support for this_cpu_cmpxchg.
Signed-off-by: Christoph Lameter <[email protected]>
---
include/linux/percpu.h | 99 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
Index: linux-2.6/include/linux/percpu.h
===================================================================
--- linux-2.6.orig/include/linux/percpu.h 2009-12-18 15:17:05.000000000 -0600
+++ linux-2.6/include/linux/percpu.h 2009-12-18 15:33:12.000000000 -0600
@@ -443,6 +443,62 @@ do { \
# define this_cpu_xor(pcp, val) __pcpu_size_call(this_cpu_or_, (pcp), (val))
#endif
+static inline unsigned long __this_cpu_cmpxchg_generic(volatile void *pptr,
+ unsigned long old, unsigned long new, int size)
+{
+ unsigned long prev;
+ volatile void * ptr = __this_cpu_ptr(pptr);
+
+ switch (size) {
+ case 1: prev = *(u8 *)ptr;
+ if (prev == old)
+ *(u8 *)ptr = (u8)new;
+ break;
+ case 2: prev = *(u16 *)ptr;
+ if (prev == old)
+ *(u16 *)ptr = (u16)new;
+ break;
+ case 4: prev = *(u32 *)ptr;
+ if (prev == old)
+ *(u32 *)ptr = (u32)new;
+ break;
+ case 8: prev = *(u64 *)ptr;
+ if (prev == old)
+ *(u64 *)ptr = (u64)new;
+ break;
+ default:
+ __bad_size_call_parameter();
+ }
+ return prev;
+}
+
+static inline unsigned long this_cpu_cmpxchg_generic(volatile void *ptr,
+ unsigned long old, unsigned long new, int size)
+{
+ unsigned long prev;
+
+ preempt_disable();
+ prev = __this_cpu_cmpxchg_generic(ptr, old, new, size);
+ preempt_enable();
+ return prev;
+}
+
+#ifndef this_cpu_cmpxchg
+# ifndef this_cpu_cmpxchg_1
+# define this_cpu_cmpxchg_1(pcp, old, new) this_cpu_cmpxchg_generic((pcp), (old), (new), 1)
+# endif
+# ifndef this_cpu_cmpxchg_2
+# define this_cpu_cmpxchg_2(pcp, old, new) this_cpu_cmpxchg_generic((pcp), (old), (new), 2)
+# endif
+# ifndef this_cpu_cmpxchg_4
+# define this_cpu_cmpxchg_4(pcp, old, new) this_cpu_cmpxchg_generic((pcp), (old), (new), 4)
+# endif
+# ifndef this_cpu_cmpxchg_8
+# define this_cpu_cmpxchg_8(pcp, old, new) this_cpu_cmpxchg_generic((pcp), (old), (new), 8)
+# endif
+# define this_cpu_cmpxchg(pcp, old, new) __pcpu_size_call_return(__this_cpu_cmpxchg_, (old), (new))
+#endif
+
/*
* Generic percpu operations that do not require preemption handling.
* Either we do not care about races or the caller has the
@@ -594,6 +650,22 @@ do { \
# define __this_cpu_xor(pcp, val) __pcpu_size_call(__this_cpu_xor_, (pcp), (val))
#endif
+#ifndef __this_cpu_cmpxchg
+# ifndef __this_cpu_cmpxchg_1
+# define __this_cpu_cmpxchg_1(pcp, old, new) __this_cpu_cmpxchg_generic((pcp), (old), (new), 1)
+# endif
+# ifndef __this_cpu_cmpxchg_2
+# define __this_cpu_cmpxchg_2(pcp, old, new) __this_cpu_cmpxchg_generic((pcp), (old), (new), 2)
+# endif
+# ifndef __this_cpu_cmpxchg_4
+# define __this_cpu_cmpxchg_4(pcp, old, new) __this_cpu_cmpxchg_generic((pcp), (old), (new), 4)
+# endif
+# ifndef __this_cpu_cmpxchg_8
+# define __this_cpu_cmpxchg_8(pcp, old, new) __this_cpu_cmpxchg_generic((pcp), (old), (new), 8)
+# endif
+# define __this_cpu_cmpxchg(pcp, old, new) __pcpu_size_call_return(__this_cpu_cmpxchg_, (old), (new))
+#endif
+
/*
* IRQ safe versions of the per cpu RMW operations. Note that these operations
* are *not* safe against modification of the same variable from another
@@ -709,4 +781,31 @@ do { \
# define irqsafe_cpu_xor(pcp, val) __pcpu_size_call(irqsafe_cpu_xor_, (val))
#endif
+static inline unsigned long irqsafe_cpu_cmpxchg_generic(volatile void *ptr,
+ unsigned long old, unsigned long new, int size)
+{
+ unsigned long flags, prev;
+
+ local_irq_save(flags);
+ prev = __this_cpu_cmpxchg_generic(ptr, old, new, size);
+ local_irq_restore(flags);
+ return prev;
+}
+
+#ifndef irqsafe_cpu_cmpxchg
+# ifndef irqsafe_cpu_cmpxchg_1
+# define irqsafe_cpu_cmpxchg_1(pcp, old, new) irqsafe_cpu_cmpxchg_generic(((pcp), (old), (new), 1)
+# endif
+# ifndef irqsafe_cpu_cmpxchg_2
+# define irqsafe_cpu_cmpxchg_2(pcp, old, new) irqsafe_cpu_cmpxchg_generic(((pcp), (old), (new), 2)
+# endif
+# ifndef irqsafe_cpu_cmpxchg_4
+# define irqsafe_cpu_cmpxchg_4(pcp, old, new) irqsafe_cpu_cmpxchg_generic(((pcp), (old), (new), 4)
+# endif
+# ifndef irqsafe_cpu_cmpxchg_8
+# define irqsafe_cpu_cmpxchg_8(pcp, old, new) irqsafe_cpu_cmpxchg_generic(((pcp), (old), (new), 8)
+# endif
+# define irqsafe_cpu_cmpxchg(pcp, old, new) __pcpu_size_call_return(irqsafe_cpu_cmpxchg_, (old), (new))
+#endif
+
#endif /* __LINUX_PERCPU_H */
--
* Christoph Lameter ([email protected]) wrote:
> Provide support for this_cpu_cmpxchg.
>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> ---
> include/linux/percpu.h | 99 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> Index: linux-2.6/include/linux/percpu.h
> ===================================================================
> --- linux-2.6.orig/include/linux/percpu.h 2009-12-18 15:17:05.000000000 -0600
> +++ linux-2.6/include/linux/percpu.h 2009-12-18 15:33:12.000000000 -0600
> @@ -443,6 +443,62 @@ do { \
> # define this_cpu_xor(pcp, val) __pcpu_size_call(this_cpu_or_, (pcp), (val))
> #endif
>
> +static inline unsigned long __this_cpu_cmpxchg_generic(volatile void *pptr,
> + unsigned long old, unsigned long new, int size)
> +{
> + unsigned long prev;
> + volatile void * ptr = __this_cpu_ptr(pptr);
> +
> + switch (size) {
> + case 1: prev = *(u8 *)ptr;
> + if (prev == old)
> + *(u8 *)ptr = (u8)new;
> + break;
> + case 2: prev = *(u16 *)ptr;
> + if (prev == old)
> + *(u16 *)ptr = (u16)new;
> + break;
> + case 4: prev = *(u32 *)ptr;
> + if (prev == old)
> + *(u32 *)ptr = (u32)new;
> + break;
> + case 8: prev = *(u64 *)ptr;
> + if (prev == old)
> + *(u64 *)ptr = (u64)new;
> + break;
> + default:
> + __bad_size_call_parameter();
> + }
> + return prev;
> +}
Hi Christoph,
I am a bit concerned about the "generic" version of this_cpu_cmpxchg.
Given that what LTTng needs is basically an atomic, nmi-safe version of
the primitive (on all architectures that have something close to a NMI),
this means that it could not switch over to your primitives until we add
the equivalent support we currently have with local_t to all
architectures. The transition would be faster if we create an
atomic_cpu_*() variant which would map to local_t operations in the
initial version.
Or maybe have I missed something in your patchset that address this ?
Thanks,
Mathieu
> +
> +static inline unsigned long this_cpu_cmpxchg_generic(volatile void *ptr,
> + unsigned long old, unsigned long new, int size)
> +{
> + unsigned long prev;
> +
> + preempt_disable();
> + prev = __this_cpu_cmpxchg_generic(ptr, old, new, size);
> + preempt_enable();
> + return prev;
> +}
> +
> +#ifndef this_cpu_cmpxchg
> +# ifndef this_cpu_cmpxchg_1
> +# define this_cpu_cmpxchg_1(pcp, old, new) this_cpu_cmpxchg_generic((pcp), (old), (new), 1)
> +# endif
> +# ifndef this_cpu_cmpxchg_2
> +# define this_cpu_cmpxchg_2(pcp, old, new) this_cpu_cmpxchg_generic((pcp), (old), (new), 2)
> +# endif
> +# ifndef this_cpu_cmpxchg_4
> +# define this_cpu_cmpxchg_4(pcp, old, new) this_cpu_cmpxchg_generic((pcp), (old), (new), 4)
> +# endif
> +# ifndef this_cpu_cmpxchg_8
> +# define this_cpu_cmpxchg_8(pcp, old, new) this_cpu_cmpxchg_generic((pcp), (old), (new), 8)
> +# endif
> +# define this_cpu_cmpxchg(pcp, old, new) __pcpu_size_call_return(__this_cpu_cmpxchg_, (old), (new))
> +#endif
> +
> /*
> * Generic percpu operations that do not require preemption handling.
> * Either we do not care about races or the caller has the
> @@ -594,6 +650,22 @@ do { \
> # define __this_cpu_xor(pcp, val) __pcpu_size_call(__this_cpu_xor_, (pcp), (val))
> #endif
>
> +#ifndef __this_cpu_cmpxchg
> +# ifndef __this_cpu_cmpxchg_1
> +# define __this_cpu_cmpxchg_1(pcp, old, new) __this_cpu_cmpxchg_generic((pcp), (old), (new), 1)
> +# endif
> +# ifndef __this_cpu_cmpxchg_2
> +# define __this_cpu_cmpxchg_2(pcp, old, new) __this_cpu_cmpxchg_generic((pcp), (old), (new), 2)
> +# endif
> +# ifndef __this_cpu_cmpxchg_4
> +# define __this_cpu_cmpxchg_4(pcp, old, new) __this_cpu_cmpxchg_generic((pcp), (old), (new), 4)
> +# endif
> +# ifndef __this_cpu_cmpxchg_8
> +# define __this_cpu_cmpxchg_8(pcp, old, new) __this_cpu_cmpxchg_generic((pcp), (old), (new), 8)
> +# endif
> +# define __this_cpu_cmpxchg(pcp, old, new) __pcpu_size_call_return(__this_cpu_cmpxchg_, (old), (new))
> +#endif
> +
> /*
> * IRQ safe versions of the per cpu RMW operations. Note that these operations
> * are *not* safe against modification of the same variable from another
> @@ -709,4 +781,31 @@ do { \
> # define irqsafe_cpu_xor(pcp, val) __pcpu_size_call(irqsafe_cpu_xor_, (val))
> #endif
>
> +static inline unsigned long irqsafe_cpu_cmpxchg_generic(volatile void *ptr,
> + unsigned long old, unsigned long new, int size)
> +{
> + unsigned long flags, prev;
> +
> + local_irq_save(flags);
> + prev = __this_cpu_cmpxchg_generic(ptr, old, new, size);
> + local_irq_restore(flags);
> + return prev;
> +}
> +
> +#ifndef irqsafe_cpu_cmpxchg
> +# ifndef irqsafe_cpu_cmpxchg_1
> +# define irqsafe_cpu_cmpxchg_1(pcp, old, new) irqsafe_cpu_cmpxchg_generic(((pcp), (old), (new), 1)
> +# endif
> +# ifndef irqsafe_cpu_cmpxchg_2
> +# define irqsafe_cpu_cmpxchg_2(pcp, old, new) irqsafe_cpu_cmpxchg_generic(((pcp), (old), (new), 2)
> +# endif
> +# ifndef irqsafe_cpu_cmpxchg_4
> +# define irqsafe_cpu_cmpxchg_4(pcp, old, new) irqsafe_cpu_cmpxchg_generic(((pcp), (old), (new), 4)
> +# endif
> +# ifndef irqsafe_cpu_cmpxchg_8
> +# define irqsafe_cpu_cmpxchg_8(pcp, old, new) irqsafe_cpu_cmpxchg_generic(((pcp), (old), (new), 8)
> +# endif
> +# define irqsafe_cpu_cmpxchg(pcp, old, new) __pcpu_size_call_return(irqsafe_cpu_cmpxchg_, (old), (new))
> +#endif
> +
> #endif /* __LINUX_PERCPU_H */
>
> --
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Sat, 19 Dec 2009, Mathieu Desnoyers wrote:
> I am a bit concerned about the "generic" version of this_cpu_cmpxchg.
> Given that what LTTng needs is basically an atomic, nmi-safe version of
> the primitive (on all architectures that have something close to a NMI),
> this means that it could not switch over to your primitives until we add
> the equivalent support we currently have with local_t to all
> architectures. The transition would be faster if we create an
> atomic_cpu_*() variant which would map to local_t operations in the
> initial version.
>
> Or maybe have I missed something in your patchset that address this ?
NMI safeness is not covered by this_cpu operations.
We could add nmi_safe_.... ops?
The atomic_cpu reference make me think that you want full (LOCK)
semantics? Then use the regular atomic ops?
* Christoph Lameter ([email protected]) wrote:
> On Sat, 19 Dec 2009, Mathieu Desnoyers wrote:
>
> > I am a bit concerned about the "generic" version of this_cpu_cmpxchg.
> > Given that what LTTng needs is basically an atomic, nmi-safe version of
> > the primitive (on all architectures that have something close to a NMI),
> > this means that it could not switch over to your primitives until we add
> > the equivalent support we currently have with local_t to all
> > architectures. The transition would be faster if we create an
> > atomic_cpu_*() variant which would map to local_t operations in the
> > initial version.
> >
> > Or maybe have I missed something in your patchset that address this ?
>
> NMI safeness is not covered by this_cpu operations.
>
> We could add nmi_safe_.... ops?
>
> The atomic_cpu reference make me think that you want full (LOCK)
> semantics? Then use the regular atomic ops?
nmi_safe would probably make sense here.
But given that we have to disable preemption to add precision in terms
of trace clock timestamp, I wonder if we would really gain something
considerable performance-wise.
I also thought about the design change this requires for the per-cpu
buffer commit count pointer which would have to become a per-cpu pointer
independent of the buffer structure, and I foresee a problem with
Steven's irq off tracing which need to perform buffer exchanges while
tracing is active. Basically, having only one top-level pointer for the
buffer makes it possible to exchange it atomically, but if we have to
have two separate pointers (one for per-cpu buffer, one for per-cpu
commit count array), then we are stucked.
So given that per-cpu ops limits us in terms of data structure layout, I
am less and less sure it's the best fit for ring buffers, especially if
we don't gain much performance-wise.
Thanks,
Mathieu
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, 22 Dec 2009, Mathieu Desnoyers wrote:
> > > I am a bit concerned about the "generic" version of this_cpu_cmpxchg.
> > > Given that what LTTng needs is basically an atomic, nmi-safe version of
> > > the primitive (on all architectures that have something close to a NMI),
> > > this means that it could not switch over to your primitives until we add
> > > the equivalent support we currently have with local_t to all
> > > architectures. The transition would be faster if we create an
> > > atomic_cpu_*() variant which would map to local_t operations in the
> > > initial version.
> > >
> > > Or maybe have I missed something in your patchset that address this ?
> >
> > NMI safeness is not covered by this_cpu operations.
> >
> > We could add nmi_safe_.... ops?
> >
> > The atomic_cpu reference make me think that you want full (LOCK)
> > semantics? Then use the regular atomic ops?
>
> nmi_safe would probably make sense here.
I am not sure how to implement fallback for nmi_safe operations though
since there is no way of disabling NMIs.
> But given that we have to disable preemption to add precision in terms
> of trace clock timestamp, I wonder if we would really gain something
> considerable performance-wise.
Not sure what exactly you attempt to do there.
> I also thought about the design change this requires for the per-cpu
> buffer commit count pointer which would have to become a per-cpu pointer
> independent of the buffer structure, and I foresee a problem with
> Steven's irq off tracing which need to perform buffer exchanges while
> tracing is active. Basically, having only one top-level pointer for the
> buffer makes it possible to exchange it atomically, but if we have to
> have two separate pointers (one for per-cpu buffer, one for per-cpu
> commit count array), then we are stucked.
You just need to keep percpu pointers that are offsets into the percpu
area. They can be relocated as needed to the processor specific addresses
using the cpu ops.
> So given that per-cpu ops limits us in terms of data structure layout, I
> am less and less sure it's the best fit for ring buffers, especially if
> we don't gain much performance-wise.
I dont understand how exactly the ring buffer logic works and what you are
trying to do here.
The ringbuffers are per cpu structures right and you do not change cpus
while performing operations on them? If not then the per cpu ops are not
useful to you.
If you dont: How can you safely use the local_t operations for the
ringbuffer logic?
* Christoph Lameter ([email protected]) wrote:
> On Tue, 22 Dec 2009, Mathieu Desnoyers wrote:
>
> > > > I am a bit concerned about the "generic" version of this_cpu_cmpxchg.
> > > > Given that what LTTng needs is basically an atomic, nmi-safe version of
> > > > the primitive (on all architectures that have something close to a NMI),
> > > > this means that it could not switch over to your primitives until we add
> > > > the equivalent support we currently have with local_t to all
> > > > architectures. The transition would be faster if we create an
> > > > atomic_cpu_*() variant which would map to local_t operations in the
> > > > initial version.
> > > >
> > > > Or maybe have I missed something in your patchset that address this ?
> > >
> > > NMI safeness is not covered by this_cpu operations.
> > >
> > > We could add nmi_safe_.... ops?
> > >
> > > The atomic_cpu reference make me think that you want full (LOCK)
> > > semantics? Then use the regular atomic ops?
> >
> > nmi_safe would probably make sense here.
>
> I am not sure how to implement fallback for nmi_safe operations though
> since there is no way of disabling NMIs.
>
> > But given that we have to disable preemption to add precision in terms
> > of trace clock timestamp, I wonder if we would really gain something
> > considerable performance-wise.
>
> Not sure what exactly you attempt to do there.
>
> > I also thought about the design change this requires for the per-cpu
> > buffer commit count pointer which would have to become a per-cpu pointer
> > independent of the buffer structure, and I foresee a problem with
> > Steven's irq off tracing which need to perform buffer exchanges while
> > tracing is active. Basically, having only one top-level pointer for the
> > buffer makes it possible to exchange it atomically, but if we have to
> > have two separate pointers (one for per-cpu buffer, one for per-cpu
> > commit count array), then we are stucked.
>
> You just need to keep percpu pointers that are offsets into the percpu
> area. They can be relocated as needed to the processor specific addresses
> using the cpu ops.
>
> > So given that per-cpu ops limits us in terms of data structure layout, I
> > am less and less sure it's the best fit for ring buffers, especially if
> > we don't gain much performance-wise.
>
> I dont understand how exactly the ring buffer logic works and what you are
> trying to do here.
>
> The ringbuffers are per cpu structures right and you do not change cpus
> while performing operations on them? If not then the per cpu ops are not
> useful to you.
Trying to make a long story short:
This scheme where Steven moves buffers from one CPU to another, he only
performs this operation when some other exclusion mechanism ensures that
the buffer is not used for writing by the CPU when this move operation
is done.
It is therefore correct, and needs local_t type to deal with the data
structure hierarchy vs atomic exchange as I pointed in my email.
Mathieu
>
> If you dont: How can you safely use the local_t operations for the
> ringbuffer logic?
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, 5 Jan 2010, Mathieu Desnoyers wrote:
> This scheme where Steven moves buffers from one CPU to another, he only
> performs this operation when some other exclusion mechanism ensures that
> the buffer is not used for writing by the CPU when this move operation
> is done.
>
> It is therefore correct, and needs local_t type to deal with the data
> structure hierarchy vs atomic exchange as I pointed in my email.
Yes this_cpu_xx does not seem to work for your scheme. Please review the
patchset that I sent you instead.