Skip to contentSkip to conversation

arm64: percpu: Make this_cpu accessors pre-empt safe

PATCH New Steve Capper 3 participants 11 messages Mar 19, 2015 – Mar 23, 2015

Subsystems (2)
arm64 Per-CPU memory allocator
lore
Share & tools
lore

Steve Capper

author
#0 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

this_cpu operations were implemented for arm64 in:
5284e1b arm64: xchg: Implement cmpxchg_double f97fc81 arm64: percpu: Implement this_cpu operations

Unfortunately, it is possible for pre-emption to take place between address generation and data access. This can lead to cases where data is being manipulated by this_cpu for a different CPU than it was called on. Which effectively breaks the spec.

This patch disables pre-emption for the this_cpu operations guaranteeing that address generation and data manipulation.

Fixes 5284e1b: arm64: xchg: Implement cmpxchg_double
Fixes f97fc81: arm64: percpu: Implement this_cpu operations
Reported-by Mark Rutland <[email protected]>
Signed-off-by Steve Capper <[email protected]>
---
 arch/arm64/include/asm/cmpxchg.h | 32 ++++++++++++++++------
 arch/arm64/include/asm/percpu.h  | 57 ++++++++++++++++++++++++++++++++--------
 2 files changed, 70 insertions(+), 19 deletions(-)
arch/arm64/include/asm/cmpxchg.h +24 -8
index cb95930..0ff749b 100644
@@ -246,14 +246,30 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old,
 	__ret; \
 })
 
-#define this_cpu_cmpxchg_1(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_2(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_4(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_8(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-
-#define this_cpu_cmpxchg_double_8(ptr1, ptr2, o1, o2, n1, n2) \
-	cmpxchg_double_local(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \
-				o1, o2, n1, n2)
+#define _protect_cmpxchg_local(pcp, o, n)			\
+({								\
+	typeof(*raw_cpu_ptr(&(pcp))) __ret;			\
+	__pcp_preempt_disable();				\
+	__ret = cmpxchg_local(raw_cpu_ptr(&(pcp)), o, n);	\
+	__pcp_preempt_enable();					\
+	__ret;							\
+})
+
+#define this_cpu_cmpxchg_1(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_2(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_4(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_8(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+
+#define this_cpu_cmpxchg_double_8(ptr1, ptr2, o1, o2, n1, n2)		\
+({									\
+	int __ret;							\
+	__pcp_preempt_disable();					\
+	__ret = cmpxchg_double_local(	raw_cpu_ptr(&(ptr1)),		\
+					raw_cpu_ptr(&(ptr2)),		\
+					o1, o2, n1, n2);		\
+	__pcp_preempt_enable();						\
+	__ret;								\
+})
 
 #define cmpxchg64(ptr,o,n)		cmpxchg((ptr),(o),(n))
 #define cmpxchg64_local(ptr,o,n)	cmpxchg_local((ptr),(o),(n))
arch/arm64/include/asm/percpu.h +46 -11

71 lines not on this page · raw message

Signature · 1 line
2.1.0
reviewer
#1 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

Hi Steve,

Thanks for putting this together!

Quoted 11 lines · Steve Capper

this_cpu operations were implemented for arm64 in:
5284e1b arm64: xchg: Implement cmpxchg_double f97fc81 arm64: percpu: Implement this_cpu operations

Unfortunately, it is possible for pre-emption to take place between address generation and data access. This can lead to cases where data is being manipulated by this_cpu for a different CPU than it was called on. Which effectively breaks the spec.

This patch disables pre-emption for the this_cpu operations guaranteeing that address generation and data manipulation.

Shouldn't that last sentence end with "occur on the same CPU", or something like that?

[...]

arch/arm64/include/asm/percpu.h static inline unsigned long __percpu_xchg(void *ptr, unsigned long val, · line 216

Quoted 10 lines
arch/arm64/include/asm/percpu.h +10 -0
+/*
+ * Modules aren't allowed to use preempt_enable_no_resched, and it is
+ * undef'ed. If we are unable to use preempt_enable_no_resched, then
+ * fallback to the standard preempt_enable.
+ */
+#ifdef preempt_enable_no_resched
+#define __pcp_preempt_enable()	preempt_enable_no_resched()
+#else
+#define __pcp_preempt_enable()	preempt_enable()
+#endif /* preempt_enable_no_resched */

I think it would be worth mentioning in the comment why we want to use preempt_enable_no_resched where possible (e.g. read-modify-cmpxchg sequences where we want to have as few retries as possible).

Other than those points, the patch looks good to me, feel free to add:

Reviewed-by Mark Rutland <[email protected]>

It's a shame there don't seem to be any this_cpu_* self-tests; I've booted a kernel with this applied, but I didn't have anything that exploded without this, so I'd feel uneasy giving a Tested-by.

Mark.

author
#2 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

Quoted 2 lines · Mark Rutland

Hi Steve,

Hey Mark,

Quoted 12 lines

Thanks for putting this together!

Quoted 9 lines · Steve Capper

this_cpu operations were implemented for arm64 in:
5284e1b arm64: xchg: Implement cmpxchg_double f97fc81 arm64: percpu: Implement this_cpu operations

Unfortunately, it is possible for pre-emption to take place between address generation and data access. This can lead to cases where data is being manipulated by this_cpu for a different CPU than it was called on. Which effectively breaks the spec.

Quoted 5 lines
Quoted 2 lines

This patch disables pre-emption for the this_cpu operations guaranteeing that address generation and data manipulation.

Shouldn't that last sentence end with "occur on the same CPU", or something like that?

Gah! Yes it should, thank you.

Quoted 17 lines

[...]

arch/arm64/include/asm/percpu.h static inline unsigned long __percpu_xchg(void *ptr, unsigned long val, · line 216

Quoted 10 lines
arch/arm64/include/asm/percpu.h +10 -0
+/*
+ * Modules aren't allowed to use preempt_enable_no_resched, and it is
+ * undef'ed. If we are unable to use preempt_enable_no_resched, then
+ * fallback to the standard preempt_enable.
+ */
+#ifdef preempt_enable_no_resched
+#define __pcp_preempt_enable()       preempt_enable_no_resched()
+#else
+#define __pcp_preempt_enable()       preempt_enable()
+#endif /* preempt_enable_no_resched */

I think it would be worth mentioning in the comment why we want to use preempt_enable_no_resched where possible (e.g. read-modify-cmpxchg sequences where we want to have as few retries as possible).

Good point, I'll add something.

Quoted 4 lines

Other than those points, the patch looks good to me, feel free to add:

Reviewed-by Mark Rutland <[email protected]>

Thanks!

Quoted 4 lines

It's a shame there don't seem to be any this_cpu_* self-tests; I've booted a kernel with this applied, but I didn't have anything that exploded without this, so I'd feel uneasy giving a Tested-by.

There is the percpu_test module, I ran this and it appeared to pass. Also, I ran the traditional hackbench tests.

Thanks Mark, I'll send out a V2 shortly with the amended commit log and expanded comment.

Cheers,

Signature · 1 line
Steve
Quoted 2 lines

Mark.

reviewer
#3 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

Quoted 6 lines
Quoted 3 lines

It's a shame there don't seem to be any this_cpu_* self-tests; I've booted a kernel with this applied, but I didn't have anything that exploded without this, so I'd feel uneasy giving a Tested-by.

There is the percpu_test module, I ran this and it appeared to pass. Also, I ran the traditional hackbench tests.

I haan't spotted the percpu_test module, but looking at it, it seems to be effectively useless. The body of the test is:

static int __init percpu_test_init(void)

{
	pr_info("percpu test start\n");
	preempt_disable();

	...
	/*
	 * test each this_cpu operation in turn
	 */
	...

	preempt_enable();
	pr_info("percpu test done\n");

	return -EAGAIN;
}

Which doesn't stress the this_cpu operations at all, unless you consider the hard part to be the maths ;)

To test this patch thoroughly we need something that has a few threads which perform some common use patterns (while getting migrated), checking that they end up with consistent and the system doesn't lockup or explode.

Mark.

#4 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

In reply to Mark Rutland
Quoted 18 lines · Mark Rutland
Quoted 11 lines · Steve Capper

this_cpu operations were implemented for arm64 in:
5284e1b arm64: xchg: Implement cmpxchg_double f97fc81 arm64: percpu: Implement this_cpu operations

Unfortunately, it is possible for pre-emption to take place between address generation and data access. This can lead to cases where data is being manipulated by this_cpu for a different CPU than it was called on. Which effectively breaks the spec.

This patch disables pre-emption for the this_cpu operations guaranteeing that address generation and data manipulation.

Shouldn't that last sentence end with "occur on the same CPU", or something like that?

[...]

Quoted 14 lines

arch/arm64/include/asm/percpu.h static inline unsigned long __percpu_xchg(void *ptr, unsigned long val, · line 216

Quoted 10 lines
arch/arm64/include/asm/percpu.h +10 -0
+/*
+ * Modules aren't allowed to use preempt_enable_no_resched, and it is
+ * undef'ed. If we are unable to use preempt_enable_no_resched, then
+ * fallback to the standard preempt_enable.
+ */
+#ifdef preempt_enable_no_resched
+#define __pcp_preempt_enable()	preempt_enable_no_resched()
+#else
+#define __pcp_preempt_enable()	preempt_enable()
+#endif /* preempt_enable_no_resched */

I think it would be worth mentioning in the comment why we want to use preempt_enable_no_resched where possible (e.g. read-modify-cmpxchg sequences where we want to have as few retries as possible).

Hmm, I'm not sure I agree with that. In the interest of throughput, I can understand that you want to minimise the retries but since preempt kernels are all about minimising latency then actually scheduling when a cmpxchg loop fail sounds pretty ideal to me.

Why can't we just use preempt_enable?

Will

reviewer
#5 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

Quoted 34 lines · Will Deacon
Quoted 18 lines · Mark Rutland
Quoted 11 lines · Steve Capper

this_cpu operations were implemented for arm64 in:
5284e1b arm64: xchg: Implement cmpxchg_double f97fc81 arm64: percpu: Implement this_cpu operations

Unfortunately, it is possible for pre-emption to take place between address generation and data access. This can lead to cases where data is being manipulated by this_cpu for a different CPU than it was called on. Which effectively breaks the spec.

This patch disables pre-emption for the this_cpu operations guaranteeing that address generation and data manipulation.

Shouldn't that last sentence end with "occur on the same CPU", or something like that?

[...]

Quoted 14 lines

arch/arm64/include/asm/percpu.h static inline unsigned long __percpu_xchg(void *ptr, unsigned long val, · line 216

Quoted 10 lines
arch/arm64/include/asm/percpu.h +10 -0
+/*
+ * Modules aren't allowed to use preempt_enable_no_resched, and it is
+ * undef'ed. If we are unable to use preempt_enable_no_resched, then
+ * fallback to the standard preempt_enable.
+ */
+#ifdef preempt_enable_no_resched
+#define __pcp_preempt_enable()	preempt_enable_no_resched()
+#else
+#define __pcp_preempt_enable()	preempt_enable()
+#endif /* preempt_enable_no_resched */

I think it would be worth mentioning in the comment why we want to use preempt_enable_no_resched where possible (e.g. read-modify-cmpxchg sequences where we want to have as few retries as possible).

Quoted 4 lines

Hmm, I'm not sure I agree with that. In the interest of throughput, I can understand that you want to minimise the retries but since preempt kernels are all about minimising latency then actually scheduling when a cmpxchg loop fail sounds pretty ideal to me.

I'm on about scheduling at the end of the read, before the cmpxchg. It's basically asking for another thread to make the read stale (and hence the cmpxchg is very likely to fail).

Scheduling after the cmpxchg is fine.

Mark.

#6 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

Quoted 18 lines · Mark Rutland
Quoted 17 lines · Will Deacon
Quoted 15 lines · Mark Rutland

arch/arm64/include/asm/percpu.h static inline unsigned long __percpu_xchg(void *ptr, unsigned long val, · line 216

Quoted 10 lines · Steve Capper
arch/arm64/include/asm/percpu.h +10 -0
+/*
+ * Modules aren't allowed to use preempt_enable_no_resched, and it is
+ * undef'ed. If we are unable to use preempt_enable_no_resched, then
+ * fallback to the standard preempt_enable.
+ */
+#ifdef preempt_enable_no_resched
+#define __pcp_preempt_enable()	preempt_enable_no_resched()
+#else
+#define __pcp_preempt_enable()	preempt_enable()
+#endif /* preempt_enable_no_resched */

I think it would be worth mentioning in the comment why we want to use preempt_enable_no_resched where possible (e.g. read-modify-cmpxchg sequences where we want to have as few retries as possible).

Quoted 8 lines
Quoted 4 lines

Hmm, I'm not sure I agree with that. In the interest of throughput, I can understand that you want to minimise the retries but since preempt kernels are all about minimising latency then actually scheduling when a cmpxchg loop fail sounds pretty ideal to me.

I'm on about scheduling at the end of the read, before the cmpxchg. It's basically asking for another thread to make the read stale (and hence the cmpxchg is very likely to fail).

/me gets introduced to SLUB's slab_alloc_node.

Quoted 1 line

Scheduling after the cmpxchg is fine.

I still don't think the slub code warrants using preempt_enable_no_resched, for a number of reasons:

(1) s390 uses preempt_enable, so it doesn't appear to be the end of the world

(2) The slub code is well aware of what it's doing, but doesn't consider it an issue:

* [...] We may switch back and forth between cpus while
* reading from one cpu area. That does not matter as long
* as we end up on the original cpu again when doing
* the cmpxchg.

(3) Preemption isn't actually an issue here -- CPU migration is. I'd expect that to be a lot rarer.

(4) Having different preempt behaviour depending on whether or not something is built as a module is bloody horrible

If we wanted to change anything, SLUB is probably a better candidate than the pcpu accessors!

Will

reviewer
#7 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

Quoted 38 lines · Will Deacon
Quoted 18 lines · Mark Rutland
Quoted 17 lines · Will Deacon
Quoted 15 lines · Mark Rutland

arch/arm64/include/asm/percpu.h static inline unsigned long __percpu_xchg(void *ptr, unsigned long val, · line 216

Quoted 10 lines · Steve Capper
arch/arm64/include/asm/percpu.h +10 -0
+/*
+ * Modules aren't allowed to use preempt_enable_no_resched, and it is
+ * undef'ed. If we are unable to use preempt_enable_no_resched, then
+ * fallback to the standard preempt_enable.
+ */
+#ifdef preempt_enable_no_resched
+#define __pcp_preempt_enable()	preempt_enable_no_resched()
+#else
+#define __pcp_preempt_enable()	preempt_enable()
+#endif /* preempt_enable_no_resched */

I think it would be worth mentioning in the comment why we want to use preempt_enable_no_resched where possible (e.g. read-modify-cmpxchg sequences where we want to have as few retries as possible).

Quoted 8 lines
Quoted 4 lines

Hmm, I'm not sure I agree with that. In the interest of throughput, I can understand that you want to minimise the retries but since preempt kernels are all about minimising latency then actually scheduling when a cmpxchg loop fail sounds pretty ideal to me.

I'm on about scheduling at the end of the read, before the cmpxchg. It's basically asking for another thread to make the read stale (and hence the cmpxchg is very likely to fail).

/me gets introduced to SLUB's slab_alloc_node.

Quoted 1 line

Scheduling after the cmpxchg is fine.

I still don't think the slub code warrants using preempt_enable_no_resched, for a number of reasons:

(1) s390 uses preempt_enable, so it doesn't appear to be the end of the world

Quoted 7 lines

(2) The slub code is well aware of what it's doing, but doesn't consider it an issue:

* [...] We may switch back and forth between cpus while
* reading from one cpu area. That does not matter as long
* as we end up on the original cpu again when doing
* the cmpxchg.

Sure, the code is correct regardless, on both of these points.

Quoted 2 lines

(3) Preemption isn't actually an issue here -- CPU migration is. I'd expect that to be a lot rarer.

That's not true: in a read-modify-cmpxchg loop (even on a UP system) if you get preempted at the end of the read and another thread modifies your datastructure, then your subsequent cmpxchg will fail. So any preemption could cause you to have to retry, so you want to miminise potential preemptions.

Note that this is a performance argument rather than a correctness argument.

Quoted 2 lines

(4) Having different preempt behaviour depending on whether or not something is built as a module is bloody horrible

Agreed.

Mark.

#8 lore raw

PATCH 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

Quoted 4 lines · Mark Rutland
Quoted 2 lines · Will Deacon

(3) Preemption isn't actually an issue here -- CPU migration is. I'd expect that to be a lot rarer.

Quoted 8 lines

That's not true: in a read-modify-cmpxchg loop (even on a UP system) if you get preempted at the end of the read and another thread modifies your datastructure, then your subsequent cmpxchg will fail. So any preemption could cause you to have to retry, so you want to miminise potential preemptions.

Note that this is a performance argument rather than a correctness argument.

My argument is more that a migration will always break the cmpxchg, whereas preemption doesn't necessarily.

Steve -- can you respin this using preempt_enable everywhere please?

Will

author
#9 lore raw

PATCH v2 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

this_cpu operations were implemented for arm64 in:
5284e1b arm64: xchg: Implement cmpxchg_double f97fc81 arm64: percpu: Implement this_cpu operations

Unfortunately, it is possible for pre-emption to take place between address generation and data access. This can lead to cases where data is being manipulated by this_cpu for a different CPU than it was called on. Which effectively breaks the spec.

This patch disables pre-emption for the this_cpu operations guaranteeing that address generation and data manipulation take place without a pre-emption in-between.

Fixes 5284e1b4bc8a ("arm64: xchg: Implement cmpxchg_double")
Fixes f97fc810798c ("arm64: percpu: Implement this_cpu operations")
Reported-by Mark Rutland <[email protected]>
Signed-off-by Steve Capper <[email protected]>

---
Changed in V2, moved over to preempt_enable() completely. Corrected the "Fixes" tag.

Mark, I've dropped your Reviewed-by as I've changed some logic, please let me know if I should re-add it.

Cheers,

Signature · 5 lines
Steve
---
 arch/arm64/include/asm/cmpxchg.h | 32 +++++++++++++++++++++--------
 arch/arm64/include/asm/percpu.h  | 44 ++++++++++++++++++++++++++++++----------
 2 files changed, 57 insertions(+), 19 deletions(-)
arch/arm64/include/asm/cmpxchg.h +24 -8
index cb95930..d8c25b7 100644
@@ -246,14 +246,30 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old,
 	__ret; \
 })
 
-#define this_cpu_cmpxchg_1(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_2(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_4(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_8(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-
-#define this_cpu_cmpxchg_double_8(ptr1, ptr2, o1, o2, n1, n2) \
-	cmpxchg_double_local(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \
-				o1, o2, n1, n2)
+#define _protect_cmpxchg_local(pcp, o, n)			\
+({								\
+	typeof(*raw_cpu_ptr(&(pcp))) __ret;			\
+	preempt_disable();					\
+	__ret = cmpxchg_local(raw_cpu_ptr(&(pcp)), o, n);	\
+	preempt_enable();					\
+	__ret;							\
+})
+
+#define this_cpu_cmpxchg_1(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_2(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_4(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_8(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+
+#define this_cpu_cmpxchg_double_8(ptr1, ptr2, o1, o2, n1, n2)		\
+({									\
+	int __ret;							\
+	preempt_disable();						\
+	__ret = cmpxchg_double_local(	raw_cpu_ptr(&(ptr1)),		\
+					raw_cpu_ptr(&(ptr2)),		\
+					o1, o2, n1, n2);		\
+	preempt_enable();						\
+	__ret;								\
+})
 
 #define cmpxchg64(ptr,o,n)		cmpxchg((ptr),(o),(n))
 #define cmpxchg64_local(ptr,o,n)	cmpxchg_local((ptr),(o),(n))
arch/arm64/include/asm/percpu.h +33 -11

58 lines not on this page · raw message

Signature · 1 line
2.1.0
#10 lore raw

PATCH v2 1/1 arm64: percpu: Make this_cpu accessors pre-empt safe

Quoted 57 lines · Steve Capper

this_cpu operations were implemented for arm64 in:
5284e1b arm64: xchg: Implement cmpxchg_double f97fc81 arm64: percpu: Implement this_cpu operations

Unfortunately, it is possible for pre-emption to take place between address generation and data access. This can lead to cases where data is being manipulated by this_cpu for a different CPU than it was called on. Which effectively breaks the spec.

This patch disables pre-emption for the this_cpu operations guaranteeing that address generation and data manipulation take place without a pre-emption in-between.

Fixes 5284e1b4bc8a ("arm64: xchg: Implement cmpxchg_double")
Fixes f97fc810798c ("arm64: percpu: Implement this_cpu operations")
Reported-by Mark Rutland <[email protected]>
Signed-off-by Steve Capper <[email protected]>

---
Changed in V2, moved over to preempt_enable() completely. Corrected the "Fixes" tag.

Mark, I've dropped your Reviewed-by as I've changed some logic, please let me know if I should re-add it.

Cheers,

Signature · 5 lines
Steve
---
 arch/arm64/include/asm/cmpxchg.h | 32 +++++++++++++++++++++--------
 arch/arm64/include/asm/percpu.h  | 44 ++++++++++++++++++++++++++++++----------
 2 files changed, 57 insertions(+), 19 deletions(-)
arch/arm64/include/asm/cmpxchg.h +9 -8
index cb95930..d8c25b7 100644
@@ -246,14 +246,30 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old,
 	__ret; \
 })
 
-#define this_cpu_cmpxchg_1(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_2(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_4(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-#define this_cpu_cmpxchg_8(ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)
-
-#define this_cpu_cmpxchg_double_8(ptr1, ptr2, o1, o2, n1, n2) \
-	cmpxchg_double_local(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \
-				o1, o2, n1, n2)
+#define _protect_cmpxchg_local(pcp, o, n)			\
+({								\
+	typeof(*raw_cpu_ptr(&(pcp))) __ret;			\
+	preempt_disable();					\
+	__ret = cmpxchg_local(raw_cpu_ptr(&(pcp)), o, n);	\
+	preempt_enable();					\
+	__ret;							\
+})
+

arch/arm64/include/asm/cmpxchg.h static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, · line 267

Quoted 10 lines
arch/arm64/include/asm/cmpxchg.h +10 -0
+#define this_cpu_cmpxchg_1(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_2(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_4(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+#define this_cpu_cmpxchg_8(ptr, o, n) _protect_cmpxchg_local(ptr, o, n)
+
+#define this_cpu_cmpxchg_double_8(ptr1, ptr2, o1, o2, n1, n2)		\
+({									\
+	int __ret;							\
+	preempt_disable();						\
+	__ret = cmpxchg_double_local(	raw_cpu_ptr(&(ptr1)),		\

Weird whitespace here, but the patch looks fine to me:

Acked-by Will Deacon <[email protected]>

Thanks, Steve.

Will