2022-12-22 12:14:37

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 00/19] Introduce __xchg, non-atomic xchg

Hi all,

I hope there will be place for such tiny helper in kernel.
Quick cocci analyze shows there is probably few thousands places
where it could be useful.
I am not sure who is good person to review/ack such patches,
so I've used my intuition to construct to/cc lists, sorry for mistakes.
This is the 2nd approach of the same idea, with comments addressed[0].

The helper is tiny and there are advices we can leave without it, so
I want to present few arguments why it would be good to have it:

1. Code readability/simplification/number of lines:

Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
- previous_min_rate = evport->qos.min_rate;
- evport->qos.min_rate = min_rate;
+ previous_min_rate = __xchg(evport->qos.min_rate, min_rate);

For sure the code is more compact, and IMHO more readable.

2. Presence of similar helpers in other somehow related languages/libs:

a) Rust[1]: 'replace' from std::mem module, there is also 'take'
helper (__xchg(&x, 0)), which is the same as private helper in
i915 - fetch_and_zero, see latest patch.
b) C++ [2]: 'exchange' from utility header.

If the idea is OK there are still 2 qestions to answer:

1. Name of the helper, __xchg follows kernel conventions,
but for me Rust names are also OK.
2. Where to put the helper:
a) as in this patchset include/linux/non-atomic/xchg.h,
proposed by Andy Shevchenko,
b) include/linux/utils.h ? any better name? Some kind
of container for simple helpers.

Structure of the patchset:
17 patches releasing __xchg name from arch files
1 patch adding __xchg
1 patch adding users of __xchg

Arnd thanks for convienient set of cross compilers, it was very helpful.

So many words for so small helper :)

[0]: https://lore.kernel.org/lkml/[email protected]/T/
[1]: https://doc.rust-lang.org/std/mem/index.html
[2]: https://en.cppreference.com/w/cpp/header/utility

Regards
Andrzej

Andrzej Hajda (19):
arch/alpha: rename internal name __xchg to __arch_xchg
arch/arc: rename internal name __xchg to __arch_xchg
arch/arm: rename internal name __xchg to __arch_xchg
arch/arm64: rename internal name __xchg to __arch_xchg
arch/hexagon: rename internal name __xchg to __arch_xchg
arch/ia64: rename internal name __xchg to __arch_xchg
arch/loongarch: rename internal name __xchg to __arch_xchg
arch/m68k: rename internal name __xchg to __arch_xchg
arch/mips: rename internal name __xchg to __arch_xchg
arch/openrisc: rename internal name __xchg to __arch_xchg
arch/parisc: rename internal name __xchg to __arch_xchg
arch/powerpc: correct logged function names in xchg helpers
arch/riscv: rename internal name __xchg to __arch_xchg
arch/s390: rename internal name __xchg to __arch_xchg
arch/sh: rename internal name __xchg to __arch_xchg
arch/sparc: rename internal name __xchg to __arch_xchg
arch/xtensa: rename internal name __xchg to __arch_xchg
linux/include: add non-atomic version of xchg
drm/i915/gt: use __xchg instead of internal helper

arch/alpha/include/asm/cmpxchg.h | 6 +++---
arch/arc/include/asm/cmpxchg.h | 4 ++--
arch/arm/include/asm/cmpxchg.h | 4 ++--
arch/arm64/include/asm/cmpxchg.h | 4 ++--
arch/hexagon/include/asm/cmpxchg.h | 6 +++---
arch/ia64/include/asm/cmpxchg.h | 2 +-
arch/ia64/include/uapi/asm/cmpxchg.h | 4 ++--
arch/loongarch/include/asm/cmpxchg.h | 4 ++--
arch/m68k/include/asm/cmpxchg.h | 6 +++---
arch/mips/include/asm/cmpxchg.h | 4 ++--
arch/openrisc/include/asm/cmpxchg.h | 4 ++--
arch/parisc/include/asm/cmpxchg.h | 4 ++--
arch/powerpc/include/asm/cmpxchg.h | 4 ++--
arch/riscv/include/asm/atomic.h | 2 +-
arch/riscv/include/asm/cmpxchg.h | 4 ++--
arch/s390/include/asm/cmpxchg.h | 4 ++--
arch/sh/include/asm/cmpxchg.h | 4 ++--
arch/sparc/include/asm/cmpxchg_32.h | 4 ++--
arch/sparc/include/asm/cmpxchg_64.h | 4 ++--
arch/xtensa/include/asm/cmpxchg.h | 4 ++--
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
.../gpu/drm/i915/gt/intel_engine_heartbeat.c | 4 ++--
.../drm/i915/gt/intel_execlists_submission.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_ggtt.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gsc.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 2 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 6 +++---
drivers/gpu/drm/i915/gt/intel_migrate.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rps.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_context.c | 2 +-
.../drm/i915/gt/selftest_ring_submission.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_timeline.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 2 +-
drivers/gpu/drm/i915/i915_utils.h | 1 +
include/linux/non-atomic/xchg.h | 19 +++++++++++++++++++
39 files changed, 84 insertions(+), 64 deletions(-)
create mode 100644 include/linux/non-atomic/xchg.h

--
2.34.1


2022-12-22 12:15:22

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 16/19] arch/sparc: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/sparc/include/asm/cmpxchg_32.h | 4 ++--
arch/sparc/include/asm/cmpxchg_64.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/include/asm/cmpxchg_32.h b/arch/sparc/include/asm/cmpxchg_32.h
index 27a57a3a7597eb..7a1339533d1d7e 100644
--- a/arch/sparc/include/asm/cmpxchg_32.h
+++ b/arch/sparc/include/asm/cmpxchg_32.h
@@ -15,7 +15,7 @@
unsigned long __xchg_u32(volatile u32 *m, u32 new);
void __xchg_called_with_bad_pointer(void);

-static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, __volatile__ void * ptr, int size)
{
switch (size) {
case 4:
@@ -25,7 +25,7 @@ static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr, int
return x;
}

-#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})
+#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})

/* Emulate cmpxchg() the same way we emulate atomics,
* by hashing the object address and indexing into an array
diff --git a/arch/sparc/include/asm/cmpxchg_64.h b/arch/sparc/include/asm/cmpxchg_64.h
index 12d00a42c0a3ed..4c22fd9110c945 100644
--- a/arch/sparc/include/asm/cmpxchg_64.h
+++ b/arch/sparc/include/asm/cmpxchg_64.h
@@ -55,7 +55,7 @@ static inline unsigned long xchg64(__volatile__ unsigned long *m, unsigned long
#define arch_xchg(ptr,x) \
({ __typeof__(*(ptr)) __ret; \
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)(x), (ptr), sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)(x), (ptr), sizeof(*(ptr))); \
__ret; \
})

@@ -87,7 +87,7 @@ xchg16(__volatile__ unsigned short *m, unsigned short val)
return (load32 & mask) >> bit_shift;
}

-static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr,
+static inline unsigned long __arch_xchg(unsigned long x, __volatile__ void * ptr,
int size)
{
switch (size) {
--
2.34.1

2022-12-22 12:15:29

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 18/19] linux/include: add non-atomic version of xchg

The pattern of setting variable with new value and returning old
one is very common in kernel. Usually atomicity of the operation
is not required, so xchg seems to be suboptimal and confusing in
such cases.

Signed-off-by: Andrzej Hajda <[email protected]>
---
include/linux/non-atomic/xchg.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 include/linux/non-atomic/xchg.h

diff --git a/include/linux/non-atomic/xchg.h b/include/linux/non-atomic/xchg.h
new file mode 100644
index 00000000000000..f7fa5dd746f37d
--- /dev/null
+++ b/include/linux/non-atomic/xchg.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_NON_ATOMIC_XCHG_H
+#define _LINUX_NON_ATOMIC_XCHG_H
+
+/**
+ * __xchg - set variable pointed by @ptr to @val, return old value
+ * @ptr: pointer to affected variable
+ * @val: value to be written
+ *
+ * This is non-atomic variant of xchg.
+ */
+#define __xchg(ptr, val) ({ \
+ __auto_type __ptr = ptr; \
+ __auto_type __t = *__ptr; \
+ *__ptr = (val); \
+ __t; \
+})
+
+#endif
--
2.34.1

2022-12-22 12:15:41

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 04/19] arch/arm64: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/arm64/include/asm/cmpxchg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index 497acf134d9923..3a36ba58e8c2ef 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -62,7 +62,7 @@ __XCHG_CASE( , , mb_, 64, dmb ish, nop, , a, l, "memory")
#undef __XCHG_CASE

#define __XCHG_GEN(sfx) \
-static __always_inline unsigned long __xchg##sfx(unsigned long x, \
+static __always_inline unsigned long __arch_xchg##sfx(unsigned long x, \
volatile void *ptr, \
int size) \
{ \
@@ -93,7 +93,7 @@ __XCHG_GEN(_mb)
({ \
__typeof__(*(ptr)) __ret; \
__ret = (__typeof__(*(ptr))) \
- __xchg##sfx((unsigned long)(x), (ptr), sizeof(*(ptr))); \
+ __arch_xchg##sfx((unsigned long)(x), (ptr), sizeof(*(ptr))); \
__ret; \
})

--
2.34.1

2022-12-22 12:16:30

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 15/19] arch/sh: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/sh/include/asm/cmpxchg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sh/include/asm/cmpxchg.h b/arch/sh/include/asm/cmpxchg.h
index 0ed9b3f4a57796..288f6f38d98fb4 100644
--- a/arch/sh/include/asm/cmpxchg.h
+++ b/arch/sh/include/asm/cmpxchg.h
@@ -22,7 +22,7 @@

extern void __xchg_called_with_bad_pointer(void);

-#define __xchg(ptr, x, size) \
+#define __arch_xchg(ptr, x, size) \
({ \
unsigned long __xchg__res; \
volatile void *__xchg_ptr = (ptr); \
@@ -46,7 +46,7 @@ extern void __xchg_called_with_bad_pointer(void);
})

#define arch_xchg(ptr,x) \
- ((__typeof__(*(ptr)))__xchg((ptr),(unsigned long)(x), sizeof(*(ptr))))
+ ((__typeof__(*(ptr)))__arch_xchg((ptr),(unsigned long)(x), sizeof(*(ptr))))

/* This function doesn't exist, so you'll get a linker error
* if something tries to do an invalid cmpxchg(). */
--
2.34.1

2022-12-22 12:18:14

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 07/19] arch/loongarch: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/loongarch/include/asm/cmpxchg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/loongarch/include/asm/cmpxchg.h b/arch/loongarch/include/asm/cmpxchg.h
index ecfa6cf79806e6..979fde61bba8a4 100644
--- a/arch/loongarch/include/asm/cmpxchg.h
+++ b/arch/loongarch/include/asm/cmpxchg.h
@@ -62,7 +62,7 @@ static inline unsigned int __xchg_small(volatile void *ptr, unsigned int val,
}

static __always_inline unsigned long
-__xchg(volatile void *ptr, unsigned long x, int size)
+__arch_xchg(volatile void *ptr, unsigned long x, int size)
{
switch (size) {
case 1:
@@ -87,7 +87,7 @@ __xchg(volatile void *ptr, unsigned long x, int size)
__typeof__(*(ptr)) __res; \
\
__res = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
\
__res; \
})
--
2.34.1

2022-12-22 12:18:50

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 09/19] arch/mips: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/mips/include/asm/cmpxchg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
index 7ec9493b28614f..feed343ad483a9 100644
--- a/arch/mips/include/asm/cmpxchg.h
+++ b/arch/mips/include/asm/cmpxchg.h
@@ -68,7 +68,7 @@ extern unsigned long __xchg_small(volatile void *ptr, unsigned long val,
unsigned int size);

static __always_inline
-unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
+unsigned long __arch_xchg(volatile void *ptr, unsigned long x, int size)
{
switch (size) {
case 1:
@@ -102,7 +102,7 @@ unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
smp_mb__before_llsc(); \
\
__res = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
\
smp_llsc_mb(); \
\
--
2.34.1

2022-12-22 12:20:24

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 19/19] drm/i915/gt: use __xchg instead of internal helper

Prefer core helper if available.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_ggtt.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gsc.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 2 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 6 +++---
drivers/gpu/drm/i915/gt/intel_migrate.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rps.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_context.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_ring_submission.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_timeline.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 2 +-
drivers/gpu/drm/i915/i915_utils.h | 1 +
18 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 99c4b866adddfa..aa548bd0b83687 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1042,7 +1042,7 @@ static void cleanup_status_page(struct intel_engine_cs *engine)
/* Prevent writes into HWSP after returning the page to the system */
intel_engine_set_hwsp_writemask(engine, ~0u);

- vma = fetch_and_zero(&engine->status_page.vma);
+ vma = __xchg(&engine->status_page.vma, 0);
if (!vma)
return;

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 9a527e1f5be655..09befcc6a84fa1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -229,7 +229,7 @@ static void heartbeat(struct work_struct *wrk)
mutex_unlock(&ce->timeline->mutex);
out:
if (!engine->i915->params.enable_hangcheck || !next_heartbeat(engine))
- i915_request_put(fetch_and_zero(&engine->heartbeat.systole));
+ i915_request_put(__xchg(&engine->heartbeat.systole, 0));
intel_engine_pm_put(engine);
}

@@ -244,7 +244,7 @@ void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine)
void intel_engine_park_heartbeat(struct intel_engine_cs *engine)
{
if (cancel_delayed_work(&engine->heartbeat.work))
- i915_request_put(fetch_and_zero(&engine->heartbeat.systole));
+ i915_request_put(__xchg(&engine->heartbeat.systole, 0));
}

void intel_gt_unpark_heartbeats(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 49a8f10d76c77b..692f18e5a10ef3 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -3197,7 +3197,7 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine)
RB_CLEAR_NODE(rb);

spin_lock(&ve->base.sched_engine->lock);
- rq = fetch_and_zero(&ve->request);
+ rq = __xchg(&ve->request, NULL);
if (rq) {
if (i915_request_mark_eio(rq)) {
rq->engine = engine;
@@ -3602,7 +3602,7 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk)

spin_lock_irq(&ve->base.sched_engine->lock);

- old = fetch_and_zero(&ve->request);
+ old = __xchg(&ve->request, NULL);
if (old) {
GEM_BUG_ON(!__i915_request_is_complete(old));
__i915_request_submit(old);
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 0c7fe360f87331..81317187d0c5c6 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -684,7 +684,7 @@ static void fini_aliasing_ppgtt(struct i915_ggtt *ggtt)
{
struct i915_ppgtt *ppgtt;

- ppgtt = fetch_and_zero(&ggtt->alias);
+ ppgtt = __xchg(&ggtt->alias, NULL);
if (!ppgtt)
return;

@@ -1238,7 +1238,7 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
was_bound);

if (obj) { /* only used during resume => exclusive access */
- write_domain_objs |= fetch_and_zero(&obj->write_domain);
+ write_domain_objs |= __xchg(&obj->write_domain, 0);
obj->read_domains |= I915_GEM_DOMAIN_GTT;
}
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_gsc.c
index bcc3605158dbde..38fbea757ba741 100644
--- a/drivers/gpu/drm/i915/gt/intel_gsc.c
+++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
@@ -70,7 +70,7 @@ gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t size

static void gsc_ext_om_destroy(struct intel_gsc_intf *intf)
{
- struct drm_i915_gem_object *obj = fetch_and_zero(&intf->gem_obj);
+ struct drm_i915_gem_object *obj = __xchg(&intf->gem_obj, 0);

if (!obj)
return;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 7eeee5a7cb33cb..03fe262de7bda5 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -754,7 +754,7 @@ int intel_gt_init(struct intel_gt *gt)
intel_uc_fini(&gt->uc);
err_engines:
intel_engines_release(gt);
- i915_vm_put(fetch_and_zero(&gt->vm));
+ i915_vm_put(__xchg(&gt->vm, 0));
err_pm:
intel_gt_pm_fini(gt);
intel_gt_fini_scratch(gt);
@@ -801,7 +801,7 @@ void intel_gt_driver_release(struct intel_gt *gt)
{
struct i915_address_space *vm;

- vm = fetch_and_zero(&gt->vm);
+ vm = __xchg(&gt->vm, 0);
if (vm) /* FIXME being called twice on error paths :( */
i915_vm_put(vm);

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index c065950d0badec..219dfecf5f48db 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -123,7 +123,7 @@ static int __gt_unpark(struct intel_wakeref *wf)
static int __gt_park(struct intel_wakeref *wf)
{
struct intel_gt *gt = container_of(wf, typeof(*gt), wakeref);
- intel_wakeref_t wakeref = fetch_and_zero(&gt->awake);
+ intel_wakeref_t wakeref = __xchg(&gt->awake, 0);
struct drm_i915_private *i915 = gt->i915;

GT_TRACE(gt, "\n");
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 7771a19008c604..9dfa3c10ddc85f 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1144,7 +1144,7 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine)
static struct intel_timeline *
pinned_timeline(struct intel_context *ce, struct intel_engine_cs *engine)
{
- struct intel_timeline *tl = fetch_and_zero(&ce->timeline);
+ struct intel_timeline *tl = __xchg(&ce->timeline, 0);

return intel_timeline_create_from_engine(engine, page_unmask_bits(tl));
}
@@ -1261,8 +1261,8 @@ void lrc_fini(struct intel_context *ce)
if (!ce->state)
return;

- intel_ring_put(fetch_and_zero(&ce->ring));
- i915_vma_put(fetch_and_zero(&ce->state));
+ intel_ring_put(__xchg(&ce->ring, 0));
+ i915_vma_put(__xchg(&ce->state, 0));
}

void lrc_destroy(struct kref *kref)
diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c
index 3f638f19879685..3eab1867a4abee 100644
--- a/drivers/gpu/drm/i915/gt/intel_migrate.c
+++ b/drivers/gpu/drm/i915/gt/intel_migrate.c
@@ -1147,7 +1147,7 @@ void intel_migrate_fini(struct intel_migrate *m)
{
struct intel_context *ce;

- ce = fetch_and_zero(&m->context);
+ ce = __xchg(&m->context, 0);
if (!ce)
return;

diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
index 2ee4051e4d9613..dde40fe05709cc 100644
--- a/drivers/gpu/drm/i915/gt/intel_rc6.c
+++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
@@ -702,7 +702,7 @@ void intel_rc6_fini(struct intel_rc6 *rc6)

intel_rc6_disable(rc6);

- pctx = fetch_and_zero(&rc6->pctx);
+ pctx = __xchg(&rc6->pctx, 0);
if (pctx)
i915_gem_object_put(pctx);

diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 9ad3bc7201cbaa..e34ca33b09d2e7 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1831,7 +1831,7 @@ static void rps_work(struct work_struct *work)
u32 pm_iir = 0;

spin_lock_irq(gt->irq_lock);
- pm_iir = fetch_and_zero(&rps->pm_iir) & rps->pm_events;
+ pm_iir = __xchg(&rps->pm_iir, 0) & rps->pm_events;
client_boost = atomic_read(&rps->num_waiters);
spin_unlock_irq(gt->irq_lock);

diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
index 76fbae358072df..3f49ca1debc6ce 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -171,7 +171,7 @@ static int live_context_size(void *arg)
* active state is sufficient, we are only checking that we
* don't use more than we planned.
*/
- saved = fetch_and_zero(&engine->default_state);
+ saved = __xchg(&engine->default_state, 0);

/* Overlaps with the execlists redzone */
engine->context_size += I915_GTT_PAGE_SIZE;
diff --git a/drivers/gpu/drm/i915/gt/selftest_ring_submission.c b/drivers/gpu/drm/i915/gt/selftest_ring_submission.c
index 87ceb0f374b673..a01aaca4fbf5ff 100644
--- a/drivers/gpu/drm/i915/gt/selftest_ring_submission.c
+++ b/drivers/gpu/drm/i915/gt/selftest_ring_submission.c
@@ -269,7 +269,7 @@ static int live_ctx_switch_wa(void *arg)
if (IS_GRAPHICS_VER(gt->i915, 4, 5))
continue; /* MI_STORE_DWORD is privileged! */

- saved_wa = fetch_and_zero(&engine->wa_ctx.vma);
+ saved_wa = __xchg(&engine->wa_ctx.vma, 0);

intel_engine_pm_get(engine);
err = __live_ctx_switch_wa(engine);
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index 522d0190509ccc..d14d5159024ec7 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -892,7 +892,7 @@ static int create_watcher(struct hwsp_watcher *w,
static int check_watcher(struct hwsp_watcher *w, const char *name,
bool (*op)(u32 hwsp, u32 seqno))
{
- struct i915_request *rq = fetch_and_zero(&w->rq);
+ struct i915_request *rq = __xchg(&w->rq, NULL);
u32 offset, end;
int err;

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
index fd21dbd2663bec..3f85d3f6fc6e92 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
@@ -110,7 +110,7 @@ void intel_gsc_uc_fini(struct intel_gsc_uc *gsc)
flush_work(&gsc->work);

if (gsc->ce)
- intel_engine_destroy_pinned_context(fetch_and_zero(&gsc->ce));
+ intel_engine_destroy_pinned_context(__xchg(&gsc->ce, NULL));

i915_vma_unpin_and_release(&gsc->local, 0);

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 9a8a1abf71d7fe..0292212cffbcb9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -169,7 +169,7 @@ static void __uc_capture_load_err_log(struct intel_uc *uc)

static void __uc_free_load_err_log(struct intel_uc *uc)
{
- struct drm_i915_gem_object *log = fetch_and_zero(&uc->load_err_log);
+ struct drm_i915_gem_object *log = __xchg(&uc->load_err_log, NULL);

if (log)
i915_gem_object_put(log);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index d6ff6c584c1e1d..21324c017aea49 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -1105,7 +1105,7 @@ void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw)
if (!intel_uc_fw_is_available(uc_fw))
return;

- i915_gem_object_put(fetch_and_zero(&uc_fw->obj));
+ i915_gem_object_put(__xchg(&uc_fw->obj, NULL));

intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_SELECTED);
}
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index b64192d9c7daa7..2394a4e9e53d4c 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -26,6 +26,7 @@
#define __I915_UTILS_H

#include <linux/list.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/overflow.h>
#include <linux/sched.h>
#include <linux/string_helpers.h>
--
2.34.1

2022-12-22 12:20:28

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 03/19] arch/arm: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/arm/include/asm/cmpxchg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 4dfe538dfc689b..6953fc05a97886 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -25,7 +25,7 @@
#define swp_is_buggy
#endif

-static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void *ptr, int size)
{
extern void __bad_xchg(volatile void *, int);
unsigned long ret;
@@ -115,7 +115,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
}

#define arch_xchg_relaxed(ptr, x) ({ \
- (__typeof__(*(ptr)))__xchg((unsigned long)(x), (ptr), \
+ (__typeof__(*(ptr)))__arch_xchg((unsigned long)(x), (ptr), \
sizeof(*(ptr))); \
})

--
2.34.1

2022-12-22 12:20:51

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 13/19] arch/riscv: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/riscv/include/asm/atomic.h | 2 +-
arch/riscv/include/asm/cmpxchg.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 0dfe9d857a762b..bba472928b5393 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -261,7 +261,7 @@ c_t arch_atomic##prefix##_xchg_release(atomic##prefix##_t *v, c_t n) \
static __always_inline \
c_t arch_atomic##prefix##_xchg(atomic##prefix##_t *v, c_t n) \
{ \
- return __xchg(&(v->counter), n, size); \
+ return __arch_xchg(&(v->counter), n, size); \
} \
static __always_inline \
c_t arch_atomic##prefix##_cmpxchg_relaxed(atomic##prefix##_t *v, \
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 12debce235e52d..2f4726d3cfcc25 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -114,7 +114,7 @@
_x_, sizeof(*(ptr))); \
})

-#define __xchg(ptr, new, size) \
+#define __arch_xchg(ptr, new, size) \
({ \
__typeof__(ptr) __ptr = (ptr); \
__typeof__(new) __new = (new); \
@@ -143,7 +143,7 @@
#define arch_xchg(ptr, x) \
({ \
__typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg((ptr), _x_, sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __arch_xchg((ptr), _x_, sizeof(*(ptr))); \
})

#define xchg32(ptr, x) \
--
2.34.1

2022-12-22 12:21:03

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 06/19] arch/ia64: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/ia64/include/asm/cmpxchg.h | 2 +-
arch/ia64/include/uapi/asm/cmpxchg.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/include/asm/cmpxchg.h b/arch/ia64/include/asm/cmpxchg.h
index 94ef844298431a..8b2e644ef6a14e 100644
--- a/arch/ia64/include/asm/cmpxchg.h
+++ b/arch/ia64/include/asm/cmpxchg.h
@@ -5,7 +5,7 @@
#include <uapi/asm/cmpxchg.h>

#define arch_xchg(ptr, x) \
-({(__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
+({(__typeof__(*(ptr))) __arch_xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})

#define arch_cmpxchg(ptr, o, n) cmpxchg_acq((ptr), (o), (n))
#define arch_cmpxchg64(ptr, o, n) cmpxchg_acq((ptr), (o), (n))
diff --git a/arch/ia64/include/uapi/asm/cmpxchg.h b/arch/ia64/include/uapi/asm/cmpxchg.h
index ca2e0268534384..3fec9b037051bb 100644
--- a/arch/ia64/include/uapi/asm/cmpxchg.h
+++ b/arch/ia64/include/uapi/asm/cmpxchg.h
@@ -27,7 +27,7 @@
*/
extern void ia64_xchg_called_with_bad_pointer(void);

-#define __xchg(x, ptr, size) \
+#define __arch_xchg(x, ptr, size) \
({ \
unsigned long __xchg_result; \
\
@@ -55,7 +55,7 @@ extern void ia64_xchg_called_with_bad_pointer(void);

#ifndef __KERNEL__
#define xchg(ptr, x) \
-({(__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
+({(__typeof__(*(ptr))) __arch_xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
#endif

/*
--
2.34.1

2022-12-22 12:21:07

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 10/19] arch/openrisc: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/openrisc/include/asm/cmpxchg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/openrisc/include/asm/cmpxchg.h b/arch/openrisc/include/asm/cmpxchg.h
index 79fd16162ccb6d..5725e22e10683b 100644
--- a/arch/openrisc/include/asm/cmpxchg.h
+++ b/arch/openrisc/include/asm/cmpxchg.h
@@ -147,7 +147,7 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
extern unsigned long __xchg_called_with_bad_pointer(void)
__compiletime_error("Bad argument size for xchg");

-static inline unsigned long __xchg(volatile void *ptr, unsigned long with,
+static inline unsigned long __arch_xchg(volatile void *ptr, unsigned long with,
int size)
{
switch (size) {
@@ -163,7 +163,7 @@ static inline unsigned long __xchg(volatile void *ptr, unsigned long with,

#define arch_xchg(ptr, with) \
({ \
- (__typeof__(*(ptr))) __xchg((ptr), \
+ (__typeof__(*(ptr))) __arch_xchg((ptr), \
(unsigned long)(with), \
sizeof(*(ptr))); \
})
--
2.34.1

2022-12-22 12:21:32

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 01/19] arch/alpha: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/alpha/include/asm/cmpxchg.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
index 6e0a850aa9d38c..40e8159ef6e794 100644
--- a/arch/alpha/include/asm/cmpxchg.h
+++ b/arch/alpha/include/asm/cmpxchg.h
@@ -6,7 +6,7 @@
* Atomic exchange routines.
*/

-#define ____xchg(type, args...) __xchg ## type ## _local(args)
+#define ____xchg(type, args...) __arch_xchg ## type ## _local(args)
#define ____cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
#include <asm/xchg.h>

@@ -34,7 +34,7 @@

#undef ____xchg
#undef ____cmpxchg
-#define ____xchg(type, args...) __xchg ##type(args)
+#define ____xchg(type, args...) __arch_xchg ##type(args)
#define ____cmpxchg(type, args...) __cmpxchg ##type(args)
#include <asm/xchg.h>

@@ -48,7 +48,7 @@
__typeof__(*(ptr)) _x_ = (x); \
smp_mb(); \
__ret = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
smp_mb(); \
__ret; \
})
--
2.34.1

2022-12-22 13:11:55

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 02/19] arch/arc: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/arc/include/asm/cmpxchg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
index c5b544a5fe8106..e138fde067dea5 100644
--- a/arch/arc/include/asm/cmpxchg.h
+++ b/arch/arc/include/asm/cmpxchg.h
@@ -85,7 +85,7 @@
*/
#ifdef CONFIG_ARC_HAS_LLSC

-#define __xchg(ptr, val) \
+#define __arch_xchg(ptr, val) \
({ \
__asm__ __volatile__( \
" ex %0, [%1] \n" /* set new value */ \
@@ -102,7 +102,7 @@
\
switch(sizeof(*(_p_))) { \
case 4: \
- _val_ = __xchg(_p_, _val_); \
+ _val_ = __arch_xchg(_p_, _val_); \
break; \
default: \
BUILD_BUG(); \
--
2.34.1

2022-12-22 13:12:18

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 11/19] arch/parisc: rename internal name __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/parisc/include/asm/cmpxchg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/parisc/include/asm/cmpxchg.h b/arch/parisc/include/asm/cmpxchg.h
index 5f274be105671e..c1d776bb16b4ed 100644
--- a/arch/parisc/include/asm/cmpxchg.h
+++ b/arch/parisc/include/asm/cmpxchg.h
@@ -22,7 +22,7 @@ extern unsigned long __xchg64(unsigned long, volatile unsigned long *);

/* optimizer better get rid of switch since size is a constant */
static inline unsigned long
-__xchg(unsigned long x, volatile void *ptr, int size)
+__arch_xchg(unsigned long x, volatile void *ptr, int size)
{
switch (size) {
#ifdef CONFIG_64BIT
@@ -49,7 +49,7 @@ __xchg(unsigned long x, volatile void *ptr, int size)
__typeof__(*(ptr)) __ret; \
__typeof__(*(ptr)) _x_ = (x); \
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)_x_, (ptr), sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)_x_, (ptr), sizeof(*(ptr))); \
__ret; \
})

--
2.34.1

2022-12-22 13:19:24

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 18/19] linux/include: add non-atomic version of xchg

On Thu, Dec 22, 2022 at 12:46:34PM +0100, Andrzej Hajda wrote:
> The pattern of setting variable with new value and returning old
> one is very common in kernel. Usually atomicity of the operation
> is not required, so xchg seems to be suboptimal and confusing in
> such cases.

FWIW,
Reviewed-by: Andy Shevchenko <[email protected]>

> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> include/linux/non-atomic/xchg.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 include/linux/non-atomic/xchg.h
>
> diff --git a/include/linux/non-atomic/xchg.h b/include/linux/non-atomic/xchg.h
> new file mode 100644
> index 00000000000000..f7fa5dd746f37d
> --- /dev/null
> +++ b/include/linux/non-atomic/xchg.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_NON_ATOMIC_XCHG_H
> +#define _LINUX_NON_ATOMIC_XCHG_H
> +
> +/**
> + * __xchg - set variable pointed by @ptr to @val, return old value
> + * @ptr: pointer to affected variable
> + * @val: value to be written
> + *
> + * This is non-atomic variant of xchg.
> + */
> +#define __xchg(ptr, val) ({ \
> + __auto_type __ptr = ptr; \
> + __auto_type __t = *__ptr; \
> + *__ptr = (val); \
> + __t; \
> +})
> +
> +#endif
> --
> 2.34.1
>

--
With Best Regards,
Andy Shevchenko


2022-12-22 14:55:58

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg



On 22.12.2022 15:12, Geert Uytterhoeven wrote:
> Hi Andrzej,
>
> Thanks for your series!
>
> On Thu, Dec 22, 2022 at 12:49 PM Andrzej Hajda <[email protected]> wrote:
>> I hope there will be place for such tiny helper in kernel.
>> Quick cocci analyze shows there is probably few thousands places
>> where it could be useful.
>> I am not sure who is good person to review/ack such patches,
>> so I've used my intuition to construct to/cc lists, sorry for mistakes.
>> This is the 2nd approach of the same idea, with comments addressed[0].
>>
>> The helper is tiny and there are advices we can leave without it, so
>> I want to present few arguments why it would be good to have it:
>>
>> 1. Code readability/simplification/number of lines:
>>
>> Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
>> - previous_min_rate = evport->qos.min_rate;
>> - evport->qos.min_rate = min_rate;
>> + previous_min_rate = __xchg(evport->qos.min_rate, min_rate);
> Upon closer look, shouldn't that be
>
> previous_min_rate = __xchg(&evport->qos.min_rate, min_rate);
>
> ?

Yes, you are right, the first argument is a pointer.

Regards
Andrzej

>
>> For sure the code is more compact, and IMHO more readable.
>>
>> 2. Presence of similar helpers in other somehow related languages/libs:
>>
>> a) Rust[1]: 'replace' from std::mem module, there is also 'take'
>> helper (__xchg(&x, 0)), which is the same as private helper in
>> i915 - fetch_and_zero, see latest patch.
>> b) C++ [2]: 'exchange' from utility header.
>>
>> If the idea is OK there are still 2 qestions to answer:
>>
>> 1. Name of the helper, __xchg follows kernel conventions,
>> but for me Rust names are also OK.
> Before I realized the missing "&", I wondered how this is different
> from swap(), so naming is important.
> https://elixir.bootlin.com/linux/latest/source/include/linux/minmax.h#L139
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds

2022-12-22 15:44:27

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg

Hi Andrzej,

Thanks for your series!

On Thu, Dec 22, 2022 at 12:49 PM Andrzej Hajda <[email protected]> wrote:
> I hope there will be place for such tiny helper in kernel.
> Quick cocci analyze shows there is probably few thousands places
> where it could be useful.
> I am not sure who is good person to review/ack such patches,
> so I've used my intuition to construct to/cc lists, sorry for mistakes.
> This is the 2nd approach of the same idea, with comments addressed[0].
>
> The helper is tiny and there are advices we can leave without it, so
> I want to present few arguments why it would be good to have it:
>
> 1. Code readability/simplification/number of lines:
>
> Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
> - previous_min_rate = evport->qos.min_rate;
> - evport->qos.min_rate = min_rate;
> + previous_min_rate = __xchg(evport->qos.min_rate, min_rate);

Upon closer look, shouldn't that be

previous_min_rate = __xchg(&evport->qos.min_rate, min_rate);

?

> For sure the code is more compact, and IMHO more readable.
>
> 2. Presence of similar helpers in other somehow related languages/libs:
>
> a) Rust[1]: 'replace' from std::mem module, there is also 'take'
> helper (__xchg(&x, 0)), which is the same as private helper in
> i915 - fetch_and_zero, see latest patch.
> b) C++ [2]: 'exchange' from utility header.
>
> If the idea is OK there are still 2 qestions to answer:
>
> 1. Name of the helper, __xchg follows kernel conventions,
> but for me Rust names are also OK.

Before I realized the missing "&", I wondered how this is different
from swap(), so naming is important.
https://elixir.bootlin.com/linux/latest/source/include/linux/minmax.h#L139

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-12-22 17:24:27

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg

On Thu, 22 Dec 2022 12:46:16 +0100 Andrzej Hajda <[email protected]> wrote:

> Hi all,
>
> I hope there will be place for such tiny helper in kernel.
> Quick cocci analyze shows there is probably few thousands places
> where it could be useful.

So to clarify, the intent here is a simple readability cleanup for
existing open-coded exchange operations. The intent is *not* to
identify existing xchg() sites which are unnecessarily atomic and to
optimize them by using the non-atomic version.

Have you considered the latter?

> I am not sure who is good person to review/ack such patches,

I can take 'em.

> so I've used my intuition to construct to/cc lists, sorry for mistakes.
> This is the 2nd approach of the same idea, with comments addressed[0].
>
> The helper is tiny and there are advices we can leave without it, so
> I want to present few arguments why it would be good to have it:
>
> 1. Code readability/simplification/number of lines:
>
> Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
> - previous_min_rate = evport->qos.min_rate;
> - evport->qos.min_rate = min_rate;
> + previous_min_rate = __xchg(evport->qos.min_rate, min_rate);
>
> For sure the code is more compact, and IMHO more readable.
>
> 2. Presence of similar helpers in other somehow related languages/libs:
>
> a) Rust[1]: 'replace' from std::mem module, there is also 'take'
> helper (__xchg(&x, 0)), which is the same as private helper in
> i915 - fetch_and_zero, see latest patch.
> b) C++ [2]: 'exchange' from utility header.
>
> If the idea is OK there are still 2 qestions to answer:
>
> 1. Name of the helper, __xchg follows kernel conventions,
> but for me Rust names are also OK.

I like replace(), or, shockingly, exchange().

But... Can we simply make swap() return the previous value?

previous_min_rate = swap(&evport->qos.min_rate, min_rate);


2022-12-23 14:29:33

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg

From: Andrew Morton <[email protected]>
Date: Thu, 22 Dec 2022 09:21:47 -0800

> On Thu, 22 Dec 2022 12:46:16 +0100 Andrzej Hajda <[email protected]> wrote:
>
> > Hi all,
> >
> > I hope there will be place for such tiny helper in kernel.
> > Quick cocci analyze shows there is probably few thousands places
> > where it could be useful.
>
> So to clarify, the intent here is a simple readability cleanup for
> existing open-coded exchange operations. The intent is *not* to
> identify existing xchg() sites which are unnecessarily atomic and to
> optimize them by using the non-atomic version.
>
> Have you considered the latter?
>
> > I am not sure who is good person to review/ack such patches,
>
> I can take 'em.
>
> > so I've used my intuition to construct to/cc lists, sorry for mistakes.
> > This is the 2nd approach of the same idea, with comments addressed[0].
> >
> > The helper is tiny and there are advices we can leave without it, so
> > I want to present few arguments why it would be good to have it:
> >
> > 1. Code readability/simplification/number of lines:
> >
> > Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
> > - previous_min_rate = evport->qos.min_rate;
> > - evport->qos.min_rate = min_rate;
> > + previous_min_rate = __xchg(evport->qos.min_rate, min_rate);
> >
> > For sure the code is more compact, and IMHO more readable.
> >
> > 2. Presence of similar helpers in other somehow related languages/libs:
> >
> > a) Rust[1]: 'replace' from std::mem module, there is also 'take'
> > helper (__xchg(&x, 0)), which is the same as private helper in
> > i915 - fetch_and_zero, see latest patch.
> > b) C++ [2]: 'exchange' from utility header.
> >
> > If the idea is OK there are still 2 qestions to answer:
> >
> > 1. Name of the helper, __xchg follows kernel conventions,
> > but for me Rust names are also OK.
>
> I like replace(), or, shockingly, exchange().
>
> But... Can we simply make swap() return the previous value?
>
> previous_min_rate = swap(&evport->qos.min_rate, min_rate);

Unforunately, swap()'s arguments get passed by names, not as
pointers, so you can't do

swap(some_ptr, NULL);

-- pretty common pattern for xchg.

Thanks,
Olek

2022-12-29 09:51:46

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 02/19] arch/arc: rename internal name __xchg to __arch_xchg

On Thu, Dec 22, 2022, at 12:46, Andrzej Hajda wrote:
> __xchg will be used for non-atomic xchg macro.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> arch/arc/include/asm/cmpxchg.h | 4 ++--

Reviewed-by: Arnd Bergmann <[email protected]>

for all the arch/*/include/asm/cmpxchg.h changes.

Since these patches are all the same, and they have identical
subject and description texts, I would suggest combining them
into a single patch to keep the series more compact.

Having them separate would allow merging the patches through
the individual architecture maintainer trees, but that in turn
would mean waiting longer to get it all merged, but in this
case it seems way easier to go through the asm-generic
tree.

Arnd

2022-12-29 10:04:43

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg

Forgive me late response - Holidays,

On 22.12.2022 18:21, Andrew Morton wrote:
> On Thu, 22 Dec 2022 12:46:16 +0100 Andrzej Hajda <[email protected]> wrote:
>
>> Hi all,
>>
>> I hope there will be place for such tiny helper in kernel.
>> Quick cocci analyze shows there is probably few thousands places
>> where it could be useful.
> So to clarify, the intent here is a simple readability cleanup for
> existing open-coded exchange operations.

And replace private helpers with common one, see the last patch - the
ultimate goal
would be to replace all occurrences of fetch_and_zero with __xchg.

> The intent is *not* to
> identify existing xchg() sites which are unnecessarily atomic and to
> optimize them by using the non-atomic version.
>
> Have you considered the latter?

If you mean some way of (semi-)automatic detection of such cases, then
no. Anyway this could be quite interesting challenge.

>
>> I am not sure who is good person to review/ack such patches,
> I can take 'em.
>
>> so I've used my intuition to construct to/cc lists, sorry for mistakes.
>> This is the 2nd approach of the same idea, with comments addressed[0].
>>
>> The helper is tiny and there are advices we can leave without it, so
>> I want to present few arguments why it would be good to have it:
>>
>> 1. Code readability/simplification/number of lines:
>>
>> Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
>> - previous_min_rate = evport->qos.min_rate;
>> - evport->qos.min_rate = min_rate;
>> + previous_min_rate = __xchg(evport->qos.min_rate, min_rate);
>>
>> For sure the code is more compact, and IMHO more readable.
>>
>> 2. Presence of similar helpers in other somehow related languages/libs:
>>
>> a) Rust[1]: 'replace' from std::mem module, there is also 'take'
>> helper (__xchg(&x, 0)), which is the same as private helper in
>> i915 - fetch_and_zero, see latest patch.
>> b) C++ [2]: 'exchange' from utility header.
>>
>> If the idea is OK there are still 2 qestions to answer:
>>
>> 1. Name of the helper, __xchg follows kernel conventions,
>> but for me Rust names are also OK.
> I like replace(), or, shockingly, exchange().
>
> But... Can we simply make swap() return the previous value?
>
> previous_min_rate = swap(&evport->qos.min_rate, min_rate);

As Alexander already pointed out, swap requires 'references' to two
variables,
in contrast to xchg which requires reference to variable and value.
So we cannot use swap for cases:
    old_value = __xchg(&x, new_value);

Regards
Andrzej

2022-12-29 11:39:36

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH v2] arch: rename all internal names __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
---
arch/alpha/include/asm/cmpxchg.h | 6 +++---
arch/arc/include/asm/cmpxchg.h | 4 ++--
arch/arm/include/asm/cmpxchg.h | 4 ++--
arch/arm64/include/asm/cmpxchg.h | 4 ++--
arch/hexagon/include/asm/cmpxchg.h | 6 +++---
arch/ia64/include/asm/cmpxchg.h | 2 +-
arch/ia64/include/uapi/asm/cmpxchg.h | 4 ++--
arch/loongarch/include/asm/cmpxchg.h | 4 ++--
arch/m68k/include/asm/cmpxchg.h | 6 +++---
arch/mips/include/asm/cmpxchg.h | 4 ++--
arch/openrisc/include/asm/cmpxchg.h | 4 ++--
arch/parisc/include/asm/cmpxchg.h | 4 ++--
arch/powerpc/include/asm/cmpxchg.h | 4 ++--
arch/riscv/include/asm/atomic.h | 2 +-
arch/riscv/include/asm/cmpxchg.h | 4 ++--
arch/s390/include/asm/cmpxchg.h | 4 ++--
arch/sh/include/asm/cmpxchg.h | 4 ++--
arch/sparc/include/asm/cmpxchg_32.h | 4 ++--
arch/sparc/include/asm/cmpxchg_64.h | 4 ++--
arch/xtensa/include/asm/cmpxchg.h | 4 ++--
20 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
index 6e0a850aa9d38c..40e8159ef6e794 100644
--- a/arch/alpha/include/asm/cmpxchg.h
+++ b/arch/alpha/include/asm/cmpxchg.h
@@ -6,7 +6,7 @@
* Atomic exchange routines.
*/

-#define ____xchg(type, args...) __xchg ## type ## _local(args)
+#define ____xchg(type, args...) __arch_xchg ## type ## _local(args)
#define ____cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
#include <asm/xchg.h>

@@ -34,7 +34,7 @@

#undef ____xchg
#undef ____cmpxchg
-#define ____xchg(type, args...) __xchg ##type(args)
+#define ____xchg(type, args...) __arch_xchg ##type(args)
#define ____cmpxchg(type, args...) __cmpxchg ##type(args)
#include <asm/xchg.h>

@@ -48,7 +48,7 @@
__typeof__(*(ptr)) _x_ = (x); \
smp_mb(); \
__ret = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
smp_mb(); \
__ret; \
})
diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
index c5b544a5fe8106..e138fde067dea5 100644
--- a/arch/arc/include/asm/cmpxchg.h
+++ b/arch/arc/include/asm/cmpxchg.h
@@ -85,7 +85,7 @@
*/
#ifdef CONFIG_ARC_HAS_LLSC

-#define __xchg(ptr, val) \
+#define __arch_xchg(ptr, val) \
({ \
__asm__ __volatile__( \
" ex %0, [%1] \n" /* set new value */ \
@@ -102,7 +102,7 @@
\
switch(sizeof(*(_p_))) { \
case 4: \
- _val_ = __xchg(_p_, _val_); \
+ _val_ = __arch_xchg(_p_, _val_); \
break; \
default: \
BUILD_BUG(); \
diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 4dfe538dfc689b..6953fc05a97886 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -25,7 +25,7 @@
#define swp_is_buggy
#endif

-static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void *ptr, int size)
{
extern void __bad_xchg(volatile void *, int);
unsigned long ret;
@@ -115,7 +115,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
}

#define arch_xchg_relaxed(ptr, x) ({ \
- (__typeof__(*(ptr)))__xchg((unsigned long)(x), (ptr), \
+ (__typeof__(*(ptr)))__arch_xchg((unsigned long)(x), (ptr), \
sizeof(*(ptr))); \
})

diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index 497acf134d9923..3a36ba58e8c2ef 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -62,7 +62,7 @@ __XCHG_CASE( , , mb_, 64, dmb ish, nop, , a, l, "memory")
#undef __XCHG_CASE

#define __XCHG_GEN(sfx) \
-static __always_inline unsigned long __xchg##sfx(unsigned long x, \
+static __always_inline unsigned long __arch_xchg##sfx(unsigned long x, \
volatile void *ptr, \
int size) \
{ \
@@ -93,7 +93,7 @@ __XCHG_GEN(_mb)
({ \
__typeof__(*(ptr)) __ret; \
__ret = (__typeof__(*(ptr))) \
- __xchg##sfx((unsigned long)(x), (ptr), sizeof(*(ptr))); \
+ __arch_xchg##sfx((unsigned long)(x), (ptr), sizeof(*(ptr))); \
__ret; \
})

diff --git a/arch/hexagon/include/asm/cmpxchg.h b/arch/hexagon/include/asm/cmpxchg.h
index cdb705e1496af8..92dc5e5f836f3b 100644
--- a/arch/hexagon/include/asm/cmpxchg.h
+++ b/arch/hexagon/include/asm/cmpxchg.h
@@ -9,7 +9,7 @@
#define _ASM_CMPXCHG_H

/*
- * __xchg - atomically exchange a register and a memory location
+ * __arch_xchg - atomically exchange a register and a memory location
* @x: value to swap
* @ptr: pointer to memory
* @size: size of the value
@@ -19,7 +19,7 @@
* Note: there was an errata for V2 about .new's and memw_locked.
*
*/
-static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
+static inline unsigned long __arch_xchg(unsigned long x, volatile void *ptr,
int size)
{
unsigned long retval;
@@ -42,7 +42,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
* Atomically swap the contents of a register with memory. Should be atomic
* between multiple CPU's and within interrupts on the same CPU.
*/
-#define arch_xchg(ptr, v) ((__typeof__(*(ptr)))__xchg((unsigned long)(v), (ptr), \
+#define arch_xchg(ptr, v) ((__typeof__(*(ptr)))__arch_xchg((unsigned long)(v), (ptr), \
sizeof(*(ptr))))

/*
diff --git a/arch/ia64/include/asm/cmpxchg.h b/arch/ia64/include/asm/cmpxchg.h
index 94ef844298431a..8b2e644ef6a14e 100644
--- a/arch/ia64/include/asm/cmpxchg.h
+++ b/arch/ia64/include/asm/cmpxchg.h
@@ -5,7 +5,7 @@
#include <uapi/asm/cmpxchg.h>

#define arch_xchg(ptr, x) \
-({(__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
+({(__typeof__(*(ptr))) __arch_xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})

#define arch_cmpxchg(ptr, o, n) cmpxchg_acq((ptr), (o), (n))
#define arch_cmpxchg64(ptr, o, n) cmpxchg_acq((ptr), (o), (n))
diff --git a/arch/ia64/include/uapi/asm/cmpxchg.h b/arch/ia64/include/uapi/asm/cmpxchg.h
index ca2e0268534384..3fec9b037051bb 100644
--- a/arch/ia64/include/uapi/asm/cmpxchg.h
+++ b/arch/ia64/include/uapi/asm/cmpxchg.h
@@ -27,7 +27,7 @@
*/
extern void ia64_xchg_called_with_bad_pointer(void);

-#define __xchg(x, ptr, size) \
+#define __arch_xchg(x, ptr, size) \
({ \
unsigned long __xchg_result; \
\
@@ -55,7 +55,7 @@ extern void ia64_xchg_called_with_bad_pointer(void);

#ifndef __KERNEL__
#define xchg(ptr, x) \
-({(__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
+({(__typeof__(*(ptr))) __arch_xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
#endif

/*
diff --git a/arch/loongarch/include/asm/cmpxchg.h b/arch/loongarch/include/asm/cmpxchg.h
index ecfa6cf79806e6..979fde61bba8a4 100644
--- a/arch/loongarch/include/asm/cmpxchg.h
+++ b/arch/loongarch/include/asm/cmpxchg.h
@@ -62,7 +62,7 @@ static inline unsigned int __xchg_small(volatile void *ptr, unsigned int val,
}

static __always_inline unsigned long
-__xchg(volatile void *ptr, unsigned long x, int size)
+__arch_xchg(volatile void *ptr, unsigned long x, int size)
{
switch (size) {
case 1:
@@ -87,7 +87,7 @@ __xchg(volatile void *ptr, unsigned long x, int size)
__typeof__(*(ptr)) __res; \
\
__res = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
\
__res; \
})
diff --git a/arch/m68k/include/asm/cmpxchg.h b/arch/m68k/include/asm/cmpxchg.h
index 6cf464cdab067e..d7f3de9c5d6f79 100644
--- a/arch/m68k/include/asm/cmpxchg.h
+++ b/arch/m68k/include/asm/cmpxchg.h
@@ -9,7 +9,7 @@
extern unsigned long __invalid_xchg_size(unsigned long, volatile void *, int);

#ifndef CONFIG_RMW_INSNS
-static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void * ptr, int size)
{
unsigned long flags, tmp;

@@ -40,7 +40,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int siz
return x;
}
#else
-static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void * ptr, int size)
{
switch (size) {
case 1:
@@ -75,7 +75,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int siz
}
#endif

-#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})
+#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})

#include <asm-generic/cmpxchg-local.h>

diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
index 7ec9493b28614f..feed343ad483a9 100644
--- a/arch/mips/include/asm/cmpxchg.h
+++ b/arch/mips/include/asm/cmpxchg.h
@@ -68,7 +68,7 @@ extern unsigned long __xchg_small(volatile void *ptr, unsigned long val,
unsigned int size);

static __always_inline
-unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
+unsigned long __arch_xchg(volatile void *ptr, unsigned long x, int size)
{
switch (size) {
case 1:
@@ -102,7 +102,7 @@ unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
smp_mb__before_llsc(); \
\
__res = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
\
smp_llsc_mb(); \
\
diff --git a/arch/openrisc/include/asm/cmpxchg.h b/arch/openrisc/include/asm/cmpxchg.h
index 79fd16162ccb6d..5725e22e10683b 100644
--- a/arch/openrisc/include/asm/cmpxchg.h
+++ b/arch/openrisc/include/asm/cmpxchg.h
@@ -147,7 +147,7 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
extern unsigned long __xchg_called_with_bad_pointer(void)
__compiletime_error("Bad argument size for xchg");

-static inline unsigned long __xchg(volatile void *ptr, unsigned long with,
+static inline unsigned long __arch_xchg(volatile void *ptr, unsigned long with,
int size)
{
switch (size) {
@@ -163,7 +163,7 @@ static inline unsigned long __xchg(volatile void *ptr, unsigned long with,

#define arch_xchg(ptr, with) \
({ \
- (__typeof__(*(ptr))) __xchg((ptr), \
+ (__typeof__(*(ptr))) __arch_xchg((ptr), \
(unsigned long)(with), \
sizeof(*(ptr))); \
})
diff --git a/arch/parisc/include/asm/cmpxchg.h b/arch/parisc/include/asm/cmpxchg.h
index 5f274be105671e..c1d776bb16b4ed 100644
--- a/arch/parisc/include/asm/cmpxchg.h
+++ b/arch/parisc/include/asm/cmpxchg.h
@@ -22,7 +22,7 @@ extern unsigned long __xchg64(unsigned long, volatile unsigned long *);

/* optimizer better get rid of switch since size is a constant */
static inline unsigned long
-__xchg(unsigned long x, volatile void *ptr, int size)
+__arch_xchg(unsigned long x, volatile void *ptr, int size)
{
switch (size) {
#ifdef CONFIG_64BIT
@@ -49,7 +49,7 @@ __xchg(unsigned long x, volatile void *ptr, int size)
__typeof__(*(ptr)) __ret; \
__typeof__(*(ptr)) _x_ = (x); \
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)_x_, (ptr), sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)_x_, (ptr), sizeof(*(ptr))); \
__ret; \
})

diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h
index d0ea0571e79ab5..dbb50c06f0bf4d 100644
--- a/arch/powerpc/include/asm/cmpxchg.h
+++ b/arch/powerpc/include/asm/cmpxchg.h
@@ -229,7 +229,7 @@ __xchg_local(void *ptr, unsigned long x, unsigned int size)
return __xchg_u64_local(ptr, x);
#endif
}
- BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg");
+ BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
return x;
}

@@ -248,7 +248,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size)
return __xchg_u64_relaxed(ptr, x);
#endif
}
- BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
+ BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_relaxed");
return x;
}
#define arch_xchg_local(ptr,x) \
diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 0dfe9d857a762b..bba472928b5393 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -261,7 +261,7 @@ c_t arch_atomic##prefix##_xchg_release(atomic##prefix##_t *v, c_t n) \
static __always_inline \
c_t arch_atomic##prefix##_xchg(atomic##prefix##_t *v, c_t n) \
{ \
- return __xchg(&(v->counter), n, size); \
+ return __arch_xchg(&(v->counter), n, size); \
} \
static __always_inline \
c_t arch_atomic##prefix##_cmpxchg_relaxed(atomic##prefix##_t *v, \
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 12debce235e52d..2f4726d3cfcc25 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -114,7 +114,7 @@
_x_, sizeof(*(ptr))); \
})

-#define __xchg(ptr, new, size) \
+#define __arch_xchg(ptr, new, size) \
({ \
__typeof__(ptr) __ptr = (ptr); \
__typeof__(new) __new = (new); \
@@ -143,7 +143,7 @@
#define arch_xchg(ptr, x) \
({ \
__typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg((ptr), _x_, sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __arch_xchg((ptr), _x_, sizeof(*(ptr))); \
})

#define xchg32(ptr, x) \
diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h
index 84c3f0d576c5b1..efc16f4aac8643 100644
--- a/arch/s390/include/asm/cmpxchg.h
+++ b/arch/s390/include/asm/cmpxchg.h
@@ -14,7 +14,7 @@

void __xchg_called_with_bad_pointer(void);

-static __always_inline unsigned long __xchg(unsigned long x,
+static __always_inline unsigned long __arch_xchg(unsigned long x,
unsigned long address, int size)
{
unsigned long old;
@@ -77,7 +77,7 @@ static __always_inline unsigned long __xchg(unsigned long x,
__typeof__(*(ptr)) __ret; \
\
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)(x), (unsigned long)(ptr), \
+ __arch_xchg((unsigned long)(x), (unsigned long)(ptr), \
sizeof(*(ptr))); \
__ret; \
})
diff --git a/arch/sh/include/asm/cmpxchg.h b/arch/sh/include/asm/cmpxchg.h
index 0ed9b3f4a57796..288f6f38d98fb4 100644
--- a/arch/sh/include/asm/cmpxchg.h
+++ b/arch/sh/include/asm/cmpxchg.h
@@ -22,7 +22,7 @@

extern void __xchg_called_with_bad_pointer(void);

-#define __xchg(ptr, x, size) \
+#define __arch_xchg(ptr, x, size) \
({ \
unsigned long __xchg__res; \
volatile void *__xchg_ptr = (ptr); \
@@ -46,7 +46,7 @@ extern void __xchg_called_with_bad_pointer(void);
})

#define arch_xchg(ptr,x) \
- ((__typeof__(*(ptr)))__xchg((ptr),(unsigned long)(x), sizeof(*(ptr))))
+ ((__typeof__(*(ptr)))__arch_xchg((ptr),(unsigned long)(x), sizeof(*(ptr))))

/* This function doesn't exist, so you'll get a linker error
* if something tries to do an invalid cmpxchg(). */
diff --git a/arch/sparc/include/asm/cmpxchg_32.h b/arch/sparc/include/asm/cmpxchg_32.h
index 27a57a3a7597eb..7a1339533d1d7e 100644
--- a/arch/sparc/include/asm/cmpxchg_32.h
+++ b/arch/sparc/include/asm/cmpxchg_32.h
@@ -15,7 +15,7 @@
unsigned long __xchg_u32(volatile u32 *m, u32 new);
void __xchg_called_with_bad_pointer(void);

-static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, __volatile__ void * ptr, int size)
{
switch (size) {
case 4:
@@ -25,7 +25,7 @@ static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr, int
return x;
}

-#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})
+#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})

/* Emulate cmpxchg() the same way we emulate atomics,
* by hashing the object address and indexing into an array
diff --git a/arch/sparc/include/asm/cmpxchg_64.h b/arch/sparc/include/asm/cmpxchg_64.h
index 12d00a42c0a3ed..4c22fd9110c945 100644
--- a/arch/sparc/include/asm/cmpxchg_64.h
+++ b/arch/sparc/include/asm/cmpxchg_64.h
@@ -55,7 +55,7 @@ static inline unsigned long xchg64(__volatile__ unsigned long *m, unsigned long
#define arch_xchg(ptr,x) \
({ __typeof__(*(ptr)) __ret; \
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)(x), (ptr), sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)(x), (ptr), sizeof(*(ptr))); \
__ret; \
})

@@ -87,7 +87,7 @@ xchg16(__volatile__ unsigned short *m, unsigned short val)
return (load32 & mask) >> bit_shift;
}

-static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr,
+static inline unsigned long __arch_xchg(unsigned long x, __volatile__ void * ptr,
int size)
{
switch (size) {
diff --git a/arch/xtensa/include/asm/cmpxchg.h b/arch/xtensa/include/asm/cmpxchg.h
index eb87810357ad88..675a11ea8de76b 100644
--- a/arch/xtensa/include/asm/cmpxchg.h
+++ b/arch/xtensa/include/asm/cmpxchg.h
@@ -170,7 +170,7 @@ static inline unsigned long xchg_u32(volatile int * m, unsigned long val)
}

#define arch_xchg(ptr,x) \
- ((__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr))))
+ ((__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr))))

static inline u32 xchg_small(volatile void *ptr, u32 x, int size)
{
@@ -203,7 +203,7 @@ static inline u32 xchg_small(volatile void *ptr, u32 x, int size)
extern void __xchg_called_with_bad_pointer(void);

static __inline__ unsigned long
-__xchg(unsigned long x, volatile void * ptr, int size)
+__arch_xchg(unsigned long x, volatile void * ptr, int size)
{
switch (size) {
case 1:
--
2.34.1

2022-12-29 16:26:38

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 13/19] arch/riscv: rename internal name __xchg to __arch_xchg

On Thu, 22 Dec 2022 03:46:29 PST (-0800), [email protected] wrote:
> __xchg will be used for non-atomic xchg macro.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> arch/riscv/include/asm/atomic.h | 2 +-
> arch/riscv/include/asm/cmpxchg.h | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
> index 0dfe9d857a762b..bba472928b5393 100644
> --- a/arch/riscv/include/asm/atomic.h
> +++ b/arch/riscv/include/asm/atomic.h
> @@ -261,7 +261,7 @@ c_t arch_atomic##prefix##_xchg_release(atomic##prefix##_t *v, c_t n) \
> static __always_inline \
> c_t arch_atomic##prefix##_xchg(atomic##prefix##_t *v, c_t n) \
> { \
> - return __xchg(&(v->counter), n, size); \
> + return __arch_xchg(&(v->counter), n, size); \
> } \
> static __always_inline \
> c_t arch_atomic##prefix##_cmpxchg_relaxed(atomic##prefix##_t *v, \
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 12debce235e52d..2f4726d3cfcc25 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -114,7 +114,7 @@
> _x_, sizeof(*(ptr))); \
> })
>
> -#define __xchg(ptr, new, size) \
> +#define __arch_xchg(ptr, new, size) \
> ({ \
> __typeof__(ptr) __ptr = (ptr); \
> __typeof__(new) __new = (new); \
> @@ -143,7 +143,7 @@
> #define arch_xchg(ptr, x) \
> ({ \
> __typeof__(*(ptr)) _x_ = (x); \
> - (__typeof__(*(ptr))) __xchg((ptr), _x_, sizeof(*(ptr))); \
> + (__typeof__(*(ptr))) __arch_xchg((ptr), _x_, sizeof(*(ptr))); \
> })
>
> #define xchg32(ptr, x) \

Acked-by: Palmer Dabbelt <[email protected]>

Thanks!

2022-12-29 23:20:45

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2] arch: rename all internal names __xchg to __arch_xchg

Hi Andrzej,

I love your patch! Yet something to improve:

[auto build test ERROR on vgupta-arc/for-curr]
[also build test ERROR on arm64/for-next/core geert-m68k/for-next geert-m68k/for-linus deller-parisc/for-next powerpc/next powerpc/fixes s390/features linus/master v6.2-rc1 next-20221226]
[cannot apply to jcmvbkbc-xtensa/xtensa-for-next davem-sparc/master vgupta-arc/for-next openrisc/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Andrzej-Hajda/arch-rename-all-internal-names-__xchg-to-__arch_xchg/20221229-193545
base: https://git.kernel.org/pub/scm/linux/kernel/git/vgupta/arc.git for-curr
patch link: https://lore.kernel.org/r/20221229113338.2436892-1-andrzej.hajda%40intel.com
patch subject: [PATCH v2] arch: rename all internal names __xchg to __arch_xchg
config: alpha-allyesconfig
compiler: alpha-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/977d2ba67f6de80971b7c2ea73430108e3db2177
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Andrzej-Hajda/arch-rename-all-internal-names-__xchg-to-__arch_xchg/20221229-193545
git checkout 977d2ba67f6de80971b7c2ea73430108e3db2177
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from arch/alpha/include/asm/atomic.h:7,
from include/linux/atomic.h:7,
from include/linux/cpumask.h:13,
from include/linux/smp.h:13,
from include/linux/lockdep.h:14,
from include/linux/spinlock.h:63,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from kernel/events/core.c:11:
kernel/events/core.c: In function '__perf_event_sync_stat':
>> arch/alpha/include/asm/cmpxchg.h:16:30: error: implicit declaration of function '__xchg_local'; did you mean 'xchg_local'? [-Werror=implicit-function-declaration]
16 | (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_, \
| ^~~~~~~~~~~~
arch/alpha/include/asm/local.h:57:27: note: in expansion of macro 'xchg_local'
57 | #define local_xchg(l, n) (xchg_local(&((l)->a.counter), (n)))
| ^~~~~~~~~~
include/asm-generic/local64.h:46:33: note: in expansion of macro 'local_xchg'
46 | #define local64_xchg(l, n) local_xchg((&(l)->a), (n))
| ^~~~~~~~~~
kernel/events/core.c:3363:17: note: in expansion of macro 'local64_xchg'
3363 | value = local64_xchg(&event->count, value);
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from arch/alpha/include/asm/atomic.h:7,
from include/linux/atomic.h:7,
from include/linux/rcupdate.h:25,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/ptrace.h:6,
from include/uapi/asm-generic/bpf_perf_event.h:4,
from ./arch/alpha/include/generated/uapi/asm/bpf_perf_event.h:1,
from include/uapi/linux/bpf_perf_event.h:11,
from include/linux/perf_event.h:18,
from kernel/events/ring_buffer.c:11:
kernel/events/ring_buffer.c: In function '__perf_output_begin':
>> arch/alpha/include/asm/cmpxchg.h:16:30: error: implicit declaration of function '__xchg_local'; did you mean 'xchg_local'? [-Werror=implicit-function-declaration]
16 | (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_, \
| ^~~~~~~~~~~~
arch/alpha/include/asm/local.h:57:27: note: in expansion of macro 'xchg_local'
57 | #define local_xchg(l, n) (xchg_local(&((l)->a.counter), (n)))
| ^~~~~~~~~~
kernel/events/ring_buffer.c:247:42: note: in expansion of macro 'local_xchg'
247 | lost_event.lost = local_xchg(&rb->lost, 0);
| ^~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from arch/alpha/include/asm/atomic.h:7,
from include/linux/atomic.h:7,
from include/linux/rcupdate.h:25,
from include/linux/rbtree.h:24,
from include/linux/hrtimer.h:16,
from drivers/perf/arm-ccn.c:8:
drivers/perf/arm-ccn.c: In function 'arm_ccn_pmu_event_update':
>> arch/alpha/include/asm/cmpxchg.h:16:30: error: implicit declaration of function '__xchg_local'; did you mean 'xchg_local'? [-Werror=implicit-function-declaration]
16 | (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_, \
| ^~~~~~~~~~~~
arch/alpha/include/asm/local.h:57:27: note: in expansion of macro 'xchg_local'
57 | #define local_xchg(l, n) (xchg_local(&((l)->a.counter), (n)))
| ^~~~~~~~~~
include/asm-generic/local64.h:46:33: note: in expansion of macro 'local_xchg'
46 | #define local64_xchg(l, n) local_xchg((&(l)->a), (n))
| ^~~~~~~~~~
drivers/perf/arm-ccn.c:877:18: note: in expansion of macro 'local64_xchg'
877 | } while (local64_xchg(&hw->prev_count, new_count) != prev_count);
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from arch/alpha/include/asm/atomic.h:7,
from include/linux/atomic.h:7,
from include/linux/cpumask.h:13,
from include/linux/smp.h:13,
from include/linux/lockdep.h:14,
from include/linux/mutex.h:17,
from include/linux/kernfs.h:11,
from include/linux/sysfs.h:16,
from include/linux/kobject.h:20,
from include/linux/of.h:17,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from drivers/perf/arm-cmn.c:5:
drivers/perf/arm-cmn.c: In function 'arm_cmn_event_read':
>> arch/alpha/include/asm/cmpxchg.h:16:30: error: implicit declaration of function '__xchg_local'; did you mean 'xchg_local'? [-Werror=implicit-function-declaration]
16 | (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_, \
| ^~~~~~~~~~~~
arch/alpha/include/asm/local.h:57:27: note: in expansion of macro 'xchg_local'
57 | #define local_xchg(l, n) (xchg_local(&((l)->a.counter), (n)))
| ^~~~~~~~~~
include/asm-generic/local64.h:46:33: note: in expansion of macro 'local_xchg'
46 | #define local64_xchg(l, n) local_xchg((&(l)->a), (n))
| ^~~~~~~~~~
drivers/perf/arm-cmn.c:1299:16: note: in expansion of macro 'local64_xchg'
1299 | prev = local64_xchg(&event->hw.prev_count, new);
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from arch/alpha/include/asm/atomic.h:7,
from include/linux/atomic.h:7,
from include/linux/cpumask.h:13,
from include/linux/smp.h:13,
from include/linux/lockdep.h:14,
from include/linux/spinlock.h:63,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:7,
from include/linux/mm.h:7,
from arch/alpha/include/asm/io.h:8,
from include/linux/io.h:13,
from drivers/perf/marvell_cn10k_ddr_pmu.c:8:
drivers/perf/marvell_cn10k_ddr_pmu.c: In function 'cn10k_ddr_perf_event_update':
>> arch/alpha/include/asm/cmpxchg.h:16:30: error: implicit declaration of function '__xchg_local'; did you mean 'xchg_local'? [-Werror=implicit-function-declaration]
16 | (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_, \
| ^~~~~~~~~~~~
arch/alpha/include/asm/local.h:57:27: note: in expansion of macro 'xchg_local'
57 | #define local_xchg(l, n) (xchg_local(&((l)->a.counter), (n)))
| ^~~~~~~~~~
include/asm-generic/local64.h:46:33: note: in expansion of macro 'local_xchg'
46 | #define local64_xchg(l, n) local_xchg((&(l)->a), (n))
| ^~~~~~~~~~
drivers/perf/marvell_cn10k_ddr_pmu.c:415:18: note: in expansion of macro 'local64_xchg'
415 | } while (local64_xchg(&hwc->prev_count, new_count) != prev_count);
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors


vim +16 arch/alpha/include/asm/cmpxchg.h

5ba840f9da1ff9 Paul Gortmaker 2012-04-02 12
5ba840f9da1ff9 Paul Gortmaker 2012-04-02 13 #define xchg_local(ptr, x) \
5ba840f9da1ff9 Paul Gortmaker 2012-04-02 14 ({ \
5ba840f9da1ff9 Paul Gortmaker 2012-04-02 15 __typeof__(*(ptr)) _x_ = (x); \
5ba840f9da1ff9 Paul Gortmaker 2012-04-02 @16 (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_, \
5ba840f9da1ff9 Paul Gortmaker 2012-04-02 17 sizeof(*(ptr))); \
5ba840f9da1ff9 Paul Gortmaker 2012-04-02 18 })
5ba840f9da1ff9 Paul Gortmaker 2012-04-02 19

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (10.44 kB)
config (320.58 kB)
Download all attachments

2022-12-30 14:40:54

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH v3] arch: rename all internal names __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
---
v2: squashed all arch patches into one
v3: fixed alpha/xchg_local, thx to [email protected]
---
arch/alpha/include/asm/cmpxchg.h | 8 ++++----
arch/arc/include/asm/cmpxchg.h | 4 ++--
arch/arm/include/asm/cmpxchg.h | 4 ++--
arch/arm64/include/asm/cmpxchg.h | 4 ++--
arch/hexagon/include/asm/cmpxchg.h | 6 +++---
arch/ia64/include/asm/cmpxchg.h | 2 +-
arch/ia64/include/uapi/asm/cmpxchg.h | 4 ++--
arch/loongarch/include/asm/cmpxchg.h | 4 ++--
arch/m68k/include/asm/cmpxchg.h | 6 +++---
arch/mips/include/asm/cmpxchg.h | 4 ++--
arch/openrisc/include/asm/cmpxchg.h | 4 ++--
arch/parisc/include/asm/cmpxchg.h | 4 ++--
arch/powerpc/include/asm/cmpxchg.h | 4 ++--
arch/riscv/include/asm/atomic.h | 2 +-
arch/riscv/include/asm/cmpxchg.h | 4 ++--
arch/s390/include/asm/cmpxchg.h | 4 ++--
arch/sh/include/asm/cmpxchg.h | 4 ++--
arch/sparc/include/asm/cmpxchg_32.h | 4 ++--
arch/sparc/include/asm/cmpxchg_64.h | 4 ++--
arch/xtensa/include/asm/cmpxchg.h | 4 ++--
20 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
index 6e0a850aa9d38c..e0cb5b5c1c54b9 100644
--- a/arch/alpha/include/asm/cmpxchg.h
+++ b/arch/alpha/include/asm/cmpxchg.h
@@ -6,14 +6,14 @@
* Atomic exchange routines.
*/

-#define ____xchg(type, args...) __xchg ## type ## _local(args)
+#define ____xchg(type, args...) __arch_xchg ## type ## _local(args)
#define ____cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
#include <asm/xchg.h>

#define xchg_local(ptr, x) \
({ \
__typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_, \
+ (__typeof__(*(ptr))) __arch_xchg_local((ptr), (unsigned long)_x_,\
sizeof(*(ptr))); \
})

@@ -34,7 +34,7 @@

#undef ____xchg
#undef ____cmpxchg
-#define ____xchg(type, args...) __xchg ##type(args)
+#define ____xchg(type, args...) __arch_xchg ##type(args)
#define ____cmpxchg(type, args...) __cmpxchg ##type(args)
#include <asm/xchg.h>

@@ -48,7 +48,7 @@
__typeof__(*(ptr)) _x_ = (x); \
smp_mb(); \
__ret = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
smp_mb(); \
__ret; \
})
diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
index c5b544a5fe8106..e138fde067dea5 100644
--- a/arch/arc/include/asm/cmpxchg.h
+++ b/arch/arc/include/asm/cmpxchg.h
@@ -85,7 +85,7 @@
*/
#ifdef CONFIG_ARC_HAS_LLSC

-#define __xchg(ptr, val) \
+#define __arch_xchg(ptr, val) \
({ \
__asm__ __volatile__( \
" ex %0, [%1] \n" /* set new value */ \
@@ -102,7 +102,7 @@
\
switch(sizeof(*(_p_))) { \
case 4: \
- _val_ = __xchg(_p_, _val_); \
+ _val_ = __arch_xchg(_p_, _val_); \
break; \
default: \
BUILD_BUG(); \
diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 4dfe538dfc689b..6953fc05a97886 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -25,7 +25,7 @@
#define swp_is_buggy
#endif

-static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void *ptr, int size)
{
extern void __bad_xchg(volatile void *, int);
unsigned long ret;
@@ -115,7 +115,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
}

#define arch_xchg_relaxed(ptr, x) ({ \
- (__typeof__(*(ptr)))__xchg((unsigned long)(x), (ptr), \
+ (__typeof__(*(ptr)))__arch_xchg((unsigned long)(x), (ptr), \
sizeof(*(ptr))); \
})

diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index 497acf134d9923..3a36ba58e8c2ef 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -62,7 +62,7 @@ __XCHG_CASE( , , mb_, 64, dmb ish, nop, , a, l, "memory")
#undef __XCHG_CASE

#define __XCHG_GEN(sfx) \
-static __always_inline unsigned long __xchg##sfx(unsigned long x, \
+static __always_inline unsigned long __arch_xchg##sfx(unsigned long x, \
volatile void *ptr, \
int size) \
{ \
@@ -93,7 +93,7 @@ __XCHG_GEN(_mb)
({ \
__typeof__(*(ptr)) __ret; \
__ret = (__typeof__(*(ptr))) \
- __xchg##sfx((unsigned long)(x), (ptr), sizeof(*(ptr))); \
+ __arch_xchg##sfx((unsigned long)(x), (ptr), sizeof(*(ptr))); \
__ret; \
})

diff --git a/arch/hexagon/include/asm/cmpxchg.h b/arch/hexagon/include/asm/cmpxchg.h
index cdb705e1496af8..92dc5e5f836f3b 100644
--- a/arch/hexagon/include/asm/cmpxchg.h
+++ b/arch/hexagon/include/asm/cmpxchg.h
@@ -9,7 +9,7 @@
#define _ASM_CMPXCHG_H

/*
- * __xchg - atomically exchange a register and a memory location
+ * __arch_xchg - atomically exchange a register and a memory location
* @x: value to swap
* @ptr: pointer to memory
* @size: size of the value
@@ -19,7 +19,7 @@
* Note: there was an errata for V2 about .new's and memw_locked.
*
*/
-static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
+static inline unsigned long __arch_xchg(unsigned long x, volatile void *ptr,
int size)
{
unsigned long retval;
@@ -42,7 +42,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
* Atomically swap the contents of a register with memory. Should be atomic
* between multiple CPU's and within interrupts on the same CPU.
*/
-#define arch_xchg(ptr, v) ((__typeof__(*(ptr)))__xchg((unsigned long)(v), (ptr), \
+#define arch_xchg(ptr, v) ((__typeof__(*(ptr)))__arch_xchg((unsigned long)(v), (ptr), \
sizeof(*(ptr))))

/*
diff --git a/arch/ia64/include/asm/cmpxchg.h b/arch/ia64/include/asm/cmpxchg.h
index 94ef844298431a..8b2e644ef6a14e 100644
--- a/arch/ia64/include/asm/cmpxchg.h
+++ b/arch/ia64/include/asm/cmpxchg.h
@@ -5,7 +5,7 @@
#include <uapi/asm/cmpxchg.h>

#define arch_xchg(ptr, x) \
-({(__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
+({(__typeof__(*(ptr))) __arch_xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})

#define arch_cmpxchg(ptr, o, n) cmpxchg_acq((ptr), (o), (n))
#define arch_cmpxchg64(ptr, o, n) cmpxchg_acq((ptr), (o), (n))
diff --git a/arch/ia64/include/uapi/asm/cmpxchg.h b/arch/ia64/include/uapi/asm/cmpxchg.h
index ca2e0268534384..3fec9b037051bb 100644
--- a/arch/ia64/include/uapi/asm/cmpxchg.h
+++ b/arch/ia64/include/uapi/asm/cmpxchg.h
@@ -27,7 +27,7 @@
*/
extern void ia64_xchg_called_with_bad_pointer(void);

-#define __xchg(x, ptr, size) \
+#define __arch_xchg(x, ptr, size) \
({ \
unsigned long __xchg_result; \
\
@@ -55,7 +55,7 @@ extern void ia64_xchg_called_with_bad_pointer(void);

#ifndef __KERNEL__
#define xchg(ptr, x) \
-({(__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
+({(__typeof__(*(ptr))) __arch_xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
#endif

/*
diff --git a/arch/loongarch/include/asm/cmpxchg.h b/arch/loongarch/include/asm/cmpxchg.h
index ecfa6cf79806e6..979fde61bba8a4 100644
--- a/arch/loongarch/include/asm/cmpxchg.h
+++ b/arch/loongarch/include/asm/cmpxchg.h
@@ -62,7 +62,7 @@ static inline unsigned int __xchg_small(volatile void *ptr, unsigned int val,
}

static __always_inline unsigned long
-__xchg(volatile void *ptr, unsigned long x, int size)
+__arch_xchg(volatile void *ptr, unsigned long x, int size)
{
switch (size) {
case 1:
@@ -87,7 +87,7 @@ __xchg(volatile void *ptr, unsigned long x, int size)
__typeof__(*(ptr)) __res; \
\
__res = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
\
__res; \
})
diff --git a/arch/m68k/include/asm/cmpxchg.h b/arch/m68k/include/asm/cmpxchg.h
index 6cf464cdab067e..d7f3de9c5d6f79 100644
--- a/arch/m68k/include/asm/cmpxchg.h
+++ b/arch/m68k/include/asm/cmpxchg.h
@@ -9,7 +9,7 @@
extern unsigned long __invalid_xchg_size(unsigned long, volatile void *, int);

#ifndef CONFIG_RMW_INSNS
-static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void * ptr, int size)
{
unsigned long flags, tmp;

@@ -40,7 +40,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int siz
return x;
}
#else
-static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void * ptr, int size)
{
switch (size) {
case 1:
@@ -75,7 +75,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int siz
}
#endif

-#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})
+#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})

#include <asm-generic/cmpxchg-local.h>

diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
index 7ec9493b28614f..feed343ad483a9 100644
--- a/arch/mips/include/asm/cmpxchg.h
+++ b/arch/mips/include/asm/cmpxchg.h
@@ -68,7 +68,7 @@ extern unsigned long __xchg_small(volatile void *ptr, unsigned long val,
unsigned int size);

static __always_inline
-unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
+unsigned long __arch_xchg(volatile void *ptr, unsigned long x, int size)
{
switch (size) {
case 1:
@@ -102,7 +102,7 @@ unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
smp_mb__before_llsc(); \
\
__res = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
\
smp_llsc_mb(); \
\
diff --git a/arch/openrisc/include/asm/cmpxchg.h b/arch/openrisc/include/asm/cmpxchg.h
index 79fd16162ccb6d..5725e22e10683b 100644
--- a/arch/openrisc/include/asm/cmpxchg.h
+++ b/arch/openrisc/include/asm/cmpxchg.h
@@ -147,7 +147,7 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
extern unsigned long __xchg_called_with_bad_pointer(void)
__compiletime_error("Bad argument size for xchg");

-static inline unsigned long __xchg(volatile void *ptr, unsigned long with,
+static inline unsigned long __arch_xchg(volatile void *ptr, unsigned long with,
int size)
{
switch (size) {
@@ -163,7 +163,7 @@ static inline unsigned long __xchg(volatile void *ptr, unsigned long with,

#define arch_xchg(ptr, with) \
({ \
- (__typeof__(*(ptr))) __xchg((ptr), \
+ (__typeof__(*(ptr))) __arch_xchg((ptr), \
(unsigned long)(with), \
sizeof(*(ptr))); \
})
diff --git a/arch/parisc/include/asm/cmpxchg.h b/arch/parisc/include/asm/cmpxchg.h
index 5f274be105671e..c1d776bb16b4ed 100644
--- a/arch/parisc/include/asm/cmpxchg.h
+++ b/arch/parisc/include/asm/cmpxchg.h
@@ -22,7 +22,7 @@ extern unsigned long __xchg64(unsigned long, volatile unsigned long *);

/* optimizer better get rid of switch since size is a constant */
static inline unsigned long
-__xchg(unsigned long x, volatile void *ptr, int size)
+__arch_xchg(unsigned long x, volatile void *ptr, int size)
{
switch (size) {
#ifdef CONFIG_64BIT
@@ -49,7 +49,7 @@ __xchg(unsigned long x, volatile void *ptr, int size)
__typeof__(*(ptr)) __ret; \
__typeof__(*(ptr)) _x_ = (x); \
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)_x_, (ptr), sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)_x_, (ptr), sizeof(*(ptr))); \
__ret; \
})

diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h
index d0ea0571e79ab5..dbb50c06f0bf4d 100644
--- a/arch/powerpc/include/asm/cmpxchg.h
+++ b/arch/powerpc/include/asm/cmpxchg.h
@@ -229,7 +229,7 @@ __xchg_local(void *ptr, unsigned long x, unsigned int size)
return __xchg_u64_local(ptr, x);
#endif
}
- BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg");
+ BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
return x;
}

@@ -248,7 +248,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size)
return __xchg_u64_relaxed(ptr, x);
#endif
}
- BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
+ BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_relaxed");
return x;
}
#define arch_xchg_local(ptr,x) \
diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 0dfe9d857a762b..bba472928b5393 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -261,7 +261,7 @@ c_t arch_atomic##prefix##_xchg_release(atomic##prefix##_t *v, c_t n) \
static __always_inline \
c_t arch_atomic##prefix##_xchg(atomic##prefix##_t *v, c_t n) \
{ \
- return __xchg(&(v->counter), n, size); \
+ return __arch_xchg(&(v->counter), n, size); \
} \
static __always_inline \
c_t arch_atomic##prefix##_cmpxchg_relaxed(atomic##prefix##_t *v, \
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 12debce235e52d..2f4726d3cfcc25 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -114,7 +114,7 @@
_x_, sizeof(*(ptr))); \
})

-#define __xchg(ptr, new, size) \
+#define __arch_xchg(ptr, new, size) \
({ \
__typeof__(ptr) __ptr = (ptr); \
__typeof__(new) __new = (new); \
@@ -143,7 +143,7 @@
#define arch_xchg(ptr, x) \
({ \
__typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg((ptr), _x_, sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __arch_xchg((ptr), _x_, sizeof(*(ptr))); \
})

#define xchg32(ptr, x) \
diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h
index 84c3f0d576c5b1..efc16f4aac8643 100644
--- a/arch/s390/include/asm/cmpxchg.h
+++ b/arch/s390/include/asm/cmpxchg.h
@@ -14,7 +14,7 @@

void __xchg_called_with_bad_pointer(void);

-static __always_inline unsigned long __xchg(unsigned long x,
+static __always_inline unsigned long __arch_xchg(unsigned long x,
unsigned long address, int size)
{
unsigned long old;
@@ -77,7 +77,7 @@ static __always_inline unsigned long __xchg(unsigned long x,
__typeof__(*(ptr)) __ret; \
\
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)(x), (unsigned long)(ptr), \
+ __arch_xchg((unsigned long)(x), (unsigned long)(ptr), \
sizeof(*(ptr))); \
__ret; \
})
diff --git a/arch/sh/include/asm/cmpxchg.h b/arch/sh/include/asm/cmpxchg.h
index 0ed9b3f4a57796..288f6f38d98fb4 100644
--- a/arch/sh/include/asm/cmpxchg.h
+++ b/arch/sh/include/asm/cmpxchg.h
@@ -22,7 +22,7 @@

extern void __xchg_called_with_bad_pointer(void);

-#define __xchg(ptr, x, size) \
+#define __arch_xchg(ptr, x, size) \
({ \
unsigned long __xchg__res; \
volatile void *__xchg_ptr = (ptr); \
@@ -46,7 +46,7 @@ extern void __xchg_called_with_bad_pointer(void);
})

#define arch_xchg(ptr,x) \
- ((__typeof__(*(ptr)))__xchg((ptr),(unsigned long)(x), sizeof(*(ptr))))
+ ((__typeof__(*(ptr)))__arch_xchg((ptr),(unsigned long)(x), sizeof(*(ptr))))

/* This function doesn't exist, so you'll get a linker error
* if something tries to do an invalid cmpxchg(). */
diff --git a/arch/sparc/include/asm/cmpxchg_32.h b/arch/sparc/include/asm/cmpxchg_32.h
index 27a57a3a7597eb..7a1339533d1d7e 100644
--- a/arch/sparc/include/asm/cmpxchg_32.h
+++ b/arch/sparc/include/asm/cmpxchg_32.h
@@ -15,7 +15,7 @@
unsigned long __xchg_u32(volatile u32 *m, u32 new);
void __xchg_called_with_bad_pointer(void);

-static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, __volatile__ void * ptr, int size)
{
switch (size) {
case 4:
@@ -25,7 +25,7 @@ static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr, int
return x;
}

-#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})
+#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})

/* Emulate cmpxchg() the same way we emulate atomics,
* by hashing the object address and indexing into an array
diff --git a/arch/sparc/include/asm/cmpxchg_64.h b/arch/sparc/include/asm/cmpxchg_64.h
index 12d00a42c0a3ed..4c22fd9110c945 100644
--- a/arch/sparc/include/asm/cmpxchg_64.h
+++ b/arch/sparc/include/asm/cmpxchg_64.h
@@ -55,7 +55,7 @@ static inline unsigned long xchg64(__volatile__ unsigned long *m, unsigned long
#define arch_xchg(ptr,x) \
({ __typeof__(*(ptr)) __ret; \
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)(x), (ptr), sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)(x), (ptr), sizeof(*(ptr))); \
__ret; \
})

@@ -87,7 +87,7 @@ xchg16(__volatile__ unsigned short *m, unsigned short val)
return (load32 & mask) >> bit_shift;
}

-static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr,
+static inline unsigned long __arch_xchg(unsigned long x, __volatile__ void * ptr,
int size)
{
switch (size) {
diff --git a/arch/xtensa/include/asm/cmpxchg.h b/arch/xtensa/include/asm/cmpxchg.h
index eb87810357ad88..675a11ea8de76b 100644
--- a/arch/xtensa/include/asm/cmpxchg.h
+++ b/arch/xtensa/include/asm/cmpxchg.h
@@ -170,7 +170,7 @@ static inline unsigned long xchg_u32(volatile int * m, unsigned long val)
}

#define arch_xchg(ptr,x) \
- ((__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr))))
+ ((__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr))))

static inline u32 xchg_small(volatile void *ptr, u32 x, int size)
{
@@ -203,7 +203,7 @@ static inline u32 xchg_small(volatile void *ptr, u32 x, int size)
extern void __xchg_called_with_bad_pointer(void);

static __inline__ unsigned long
-__xchg(unsigned long x, volatile void * ptr, int size)
+__arch_xchg(unsigned long x, volatile void * ptr, int size)
{
switch (size) {
case 1:
--
2.34.1

2023-01-02 08:46:16

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] arch: rename all internal names __xchg to __arch_xchg

On Thu, Dec 29, 2022 at 12:34 PM Andrzej Hajda <[email protected]> wrote:
> __xchg will be used for non-atomic xchg macro.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Reviewed-by: Arnd Bergmann <[email protected]>

Acked-by: Geert Uytterhoeven <[email protected]> [m68k]

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2023-01-03 10:21:18

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH v3] arch: rename all internal names __xchg to __arch_xchg

On Fri, Dec 30, 2022 at 03:15:52PM +0100, Andrzej Hajda wrote:
> __xchg will be used for non-atomic xchg macro.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Reviewed-by: Arnd Bergmann <[email protected]>
> ---
> v2: squashed all arch patches into one
> v3: fixed alpha/xchg_local, thx to [email protected]
> ---
...
> arch/s390/include/asm/cmpxchg.h | 4 ++--
> diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h
> index 84c3f0d576c5b1..efc16f4aac8643 100644
> --- a/arch/s390/include/asm/cmpxchg.h
> +++ b/arch/s390/include/asm/cmpxchg.h
> @@ -14,7 +14,7 @@
>
> void __xchg_called_with_bad_pointer(void);
>
> -static __always_inline unsigned long __xchg(unsigned long x,
> +static __always_inline unsigned long __arch_xchg(unsigned long x,
> unsigned long address, int size)

Please adjust the alignment of the second line.

> @@ -77,7 +77,7 @@ static __always_inline unsigned long __xchg(unsigned long x,
> __typeof__(*(ptr)) __ret; \
> \
> __ret = (__typeof__(*(ptr))) \
> - __xchg((unsigned long)(x), (unsigned long)(ptr), \
> + __arch_xchg((unsigned long)(x), (unsigned long)(ptr), \
> sizeof(*(ptr))); \

Same here.

The same is true for a couple of other architectures - not sure if
they care however.

2023-01-05 10:35:17

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH v4] arch: rename all internal names __xchg to __arch_xchg

__xchg will be used for non-atomic xchg macro.

Signed-off-by: Andrzej Hajda <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
---
v2: squashed all arch patches into one
v3: fixed alpha/xchg_local, thx to [email protected]
v4: adjusted indentation (Heiko)
---
arch/alpha/include/asm/cmpxchg.h | 10 +++++-----
arch/arc/include/asm/cmpxchg.h | 4 ++--
arch/arm/include/asm/cmpxchg.h | 7 ++++---
arch/arm64/include/asm/cmpxchg.h | 7 +++----
arch/hexagon/include/asm/cmpxchg.h | 10 +++++-----
arch/ia64/include/asm/cmpxchg.h | 2 +-
arch/ia64/include/uapi/asm/cmpxchg.h | 4 ++--
arch/loongarch/include/asm/cmpxchg.h | 4 ++--
arch/m68k/include/asm/cmpxchg.h | 6 +++---
arch/mips/include/asm/cmpxchg.h | 4 ++--
arch/openrisc/include/asm/cmpxchg.h | 10 +++++-----
arch/parisc/include/asm/cmpxchg.h | 4 ++--
arch/powerpc/include/asm/cmpxchg.h | 4 ++--
arch/riscv/include/asm/atomic.h | 2 +-
arch/riscv/include/asm/cmpxchg.h | 4 ++--
arch/s390/include/asm/cmpxchg.h | 8 ++++----
arch/sh/include/asm/cmpxchg.h | 4 ++--
arch/sparc/include/asm/cmpxchg_32.h | 4 ++--
arch/sparc/include/asm/cmpxchg_64.h | 6 +++---
arch/xtensa/include/asm/cmpxchg.h | 4 ++--
20 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
index 6e0a850aa9d38c..91d4a4d9258cd2 100644
--- a/arch/alpha/include/asm/cmpxchg.h
+++ b/arch/alpha/include/asm/cmpxchg.h
@@ -6,15 +6,15 @@
* Atomic exchange routines.
*/

-#define ____xchg(type, args...) __xchg ## type ## _local(args)
+#define ____xchg(type, args...) __arch_xchg ## type ## _local(args)
#define ____cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
#include <asm/xchg.h>

#define xchg_local(ptr, x) \
({ \
__typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_, \
- sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __arch_xchg_local((ptr), (unsigned long)_x_,\
+ sizeof(*(ptr))); \
})

#define arch_cmpxchg_local(ptr, o, n) \
@@ -34,7 +34,7 @@

#undef ____xchg
#undef ____cmpxchg
-#define ____xchg(type, args...) __xchg ##type(args)
+#define ____xchg(type, args...) __arch_xchg ##type(args)
#define ____cmpxchg(type, args...) __cmpxchg ##type(args)
#include <asm/xchg.h>

@@ -48,7 +48,7 @@
__typeof__(*(ptr)) _x_ = (x); \
smp_mb(); \
__ret = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
smp_mb(); \
__ret; \
})
diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
index c5b544a5fe8106..e138fde067dea5 100644
--- a/arch/arc/include/asm/cmpxchg.h
+++ b/arch/arc/include/asm/cmpxchg.h
@@ -85,7 +85,7 @@
*/
#ifdef CONFIG_ARC_HAS_LLSC

-#define __xchg(ptr, val) \
+#define __arch_xchg(ptr, val) \
({ \
__asm__ __volatile__( \
" ex %0, [%1] \n" /* set new value */ \
@@ -102,7 +102,7 @@
\
switch(sizeof(*(_p_))) { \
case 4: \
- _val_ = __xchg(_p_, _val_); \
+ _val_ = __arch_xchg(_p_, _val_); \
break; \
default: \
BUILD_BUG(); \
diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 4dfe538dfc689b..44667bdb4707a9 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -25,7 +25,8 @@
#define swp_is_buggy
#endif

-static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size)
+static inline unsigned long
+__arch_xchg(unsigned long x, volatile void *ptr, int size)
{
extern void __bad_xchg(volatile void *, int);
unsigned long ret;
@@ -115,8 +116,8 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
}

#define arch_xchg_relaxed(ptr, x) ({ \
- (__typeof__(*(ptr)))__xchg((unsigned long)(x), (ptr), \
- sizeof(*(ptr))); \
+ (__typeof__(*(ptr)))__arch_xchg((unsigned long)(x), (ptr), \
+ sizeof(*(ptr))); \
})

#include <asm-generic/cmpxchg-local.h>
diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index 497acf134d9923..c6bc5d8ec3ca41 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -62,9 +62,8 @@ __XCHG_CASE( , , mb_, 64, dmb ish, nop, , a, l, "memory")
#undef __XCHG_CASE

#define __XCHG_GEN(sfx) \
-static __always_inline unsigned long __xchg##sfx(unsigned long x, \
- volatile void *ptr, \
- int size) \
+static __always_inline unsigned long \
+__arch_xchg##sfx(unsigned long x, volatile void *ptr, int size) \
{ \
switch (size) { \
case 1: \
@@ -93,7 +92,7 @@ __XCHG_GEN(_mb)
({ \
__typeof__(*(ptr)) __ret; \
__ret = (__typeof__(*(ptr))) \
- __xchg##sfx((unsigned long)(x), (ptr), sizeof(*(ptr))); \
+ __arch_xchg##sfx((unsigned long)(x), (ptr), sizeof(*(ptr))); \
__ret; \
})

diff --git a/arch/hexagon/include/asm/cmpxchg.h b/arch/hexagon/include/asm/cmpxchg.h
index cdb705e1496af8..bf6cf5579cf459 100644
--- a/arch/hexagon/include/asm/cmpxchg.h
+++ b/arch/hexagon/include/asm/cmpxchg.h
@@ -9,7 +9,7 @@
#define _ASM_CMPXCHG_H

/*
- * __xchg - atomically exchange a register and a memory location
+ * __arch_xchg - atomically exchange a register and a memory location
* @x: value to swap
* @ptr: pointer to memory
* @size: size of the value
@@ -19,8 +19,8 @@
* Note: there was an errata for V2 about .new's and memw_locked.
*
*/
-static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
- int size)
+static inline unsigned long
+__arch_xchg(unsigned long x, volatile void *ptr, int size)
{
unsigned long retval;

@@ -42,8 +42,8 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
* Atomically swap the contents of a register with memory. Should be atomic
* between multiple CPU's and within interrupts on the same CPU.
*/
-#define arch_xchg(ptr, v) ((__typeof__(*(ptr)))__xchg((unsigned long)(v), (ptr), \
- sizeof(*(ptr))))
+#define arch_xchg(ptr, v) ((__typeof__(*(ptr)))__arch_xchg((unsigned long)(v), (ptr), \
+ sizeof(*(ptr))))

/*
* see rt-mutex-design.txt; cmpxchg supposedly checks if *ptr == A and swaps.
diff --git a/arch/ia64/include/asm/cmpxchg.h b/arch/ia64/include/asm/cmpxchg.h
index 94ef844298431a..8b2e644ef6a14e 100644
--- a/arch/ia64/include/asm/cmpxchg.h
+++ b/arch/ia64/include/asm/cmpxchg.h
@@ -5,7 +5,7 @@
#include <uapi/asm/cmpxchg.h>

#define arch_xchg(ptr, x) \
-({(__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
+({(__typeof__(*(ptr))) __arch_xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})

#define arch_cmpxchg(ptr, o, n) cmpxchg_acq((ptr), (o), (n))
#define arch_cmpxchg64(ptr, o, n) cmpxchg_acq((ptr), (o), (n))
diff --git a/arch/ia64/include/uapi/asm/cmpxchg.h b/arch/ia64/include/uapi/asm/cmpxchg.h
index ca2e0268534384..3fec9b037051bb 100644
--- a/arch/ia64/include/uapi/asm/cmpxchg.h
+++ b/arch/ia64/include/uapi/asm/cmpxchg.h
@@ -27,7 +27,7 @@
*/
extern void ia64_xchg_called_with_bad_pointer(void);

-#define __xchg(x, ptr, size) \
+#define __arch_xchg(x, ptr, size) \
({ \
unsigned long __xchg_result; \
\
@@ -55,7 +55,7 @@ extern void ia64_xchg_called_with_bad_pointer(void);

#ifndef __KERNEL__
#define xchg(ptr, x) \
-({(__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
+({(__typeof__(*(ptr))) __arch_xchg((unsigned long) (x), (ptr), sizeof(*(ptr)));})
#endif

/*
diff --git a/arch/loongarch/include/asm/cmpxchg.h b/arch/loongarch/include/asm/cmpxchg.h
index ecfa6cf79806e6..979fde61bba8a4 100644
--- a/arch/loongarch/include/asm/cmpxchg.h
+++ b/arch/loongarch/include/asm/cmpxchg.h
@@ -62,7 +62,7 @@ static inline unsigned int __xchg_small(volatile void *ptr, unsigned int val,
}

static __always_inline unsigned long
-__xchg(volatile void *ptr, unsigned long x, int size)
+__arch_xchg(volatile void *ptr, unsigned long x, int size)
{
switch (size) {
case 1:
@@ -87,7 +87,7 @@ __xchg(volatile void *ptr, unsigned long x, int size)
__typeof__(*(ptr)) __res; \
\
__res = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
\
__res; \
})
diff --git a/arch/m68k/include/asm/cmpxchg.h b/arch/m68k/include/asm/cmpxchg.h
index 6cf464cdab067e..d7f3de9c5d6f79 100644
--- a/arch/m68k/include/asm/cmpxchg.h
+++ b/arch/m68k/include/asm/cmpxchg.h
@@ -9,7 +9,7 @@
extern unsigned long __invalid_xchg_size(unsigned long, volatile void *, int);

#ifndef CONFIG_RMW_INSNS
-static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void * ptr, int size)
{
unsigned long flags, tmp;

@@ -40,7 +40,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int siz
return x;
}
#else
-static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, volatile void * ptr, int size)
{
switch (size) {
case 1:
@@ -75,7 +75,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void * ptr, int siz
}
#endif

-#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})
+#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})

#include <asm-generic/cmpxchg-local.h>

diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
index 7ec9493b28614f..feed343ad483a9 100644
--- a/arch/mips/include/asm/cmpxchg.h
+++ b/arch/mips/include/asm/cmpxchg.h
@@ -68,7 +68,7 @@ extern unsigned long __xchg_small(volatile void *ptr, unsigned long val,
unsigned int size);

static __always_inline
-unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
+unsigned long __arch_xchg(volatile void *ptr, unsigned long x, int size)
{
switch (size) {
case 1:
@@ -102,7 +102,7 @@ unsigned long __xchg(volatile void *ptr, unsigned long x, int size)
smp_mb__before_llsc(); \
\
__res = (__typeof__(*(ptr))) \
- __xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)(x), sizeof(*(ptr))); \
\
smp_llsc_mb(); \
\
diff --git a/arch/openrisc/include/asm/cmpxchg.h b/arch/openrisc/include/asm/cmpxchg.h
index 79fd16162ccb6d..8ee151c072e414 100644
--- a/arch/openrisc/include/asm/cmpxchg.h
+++ b/arch/openrisc/include/asm/cmpxchg.h
@@ -147,8 +147,8 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
extern unsigned long __xchg_called_with_bad_pointer(void)
__compiletime_error("Bad argument size for xchg");

-static inline unsigned long __xchg(volatile void *ptr, unsigned long with,
- int size)
+static inline unsigned long
+__arch_xchg(volatile void *ptr, unsigned long with, int size)
{
switch (size) {
case 1:
@@ -163,9 +163,9 @@ static inline unsigned long __xchg(volatile void *ptr, unsigned long with,

#define arch_xchg(ptr, with) \
({ \
- (__typeof__(*(ptr))) __xchg((ptr), \
- (unsigned long)(with), \
- sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __arch_xchg((ptr), \
+ (unsigned long)(with), \
+ sizeof(*(ptr))); \
})

#endif /* __ASM_OPENRISC_CMPXCHG_H */
diff --git a/arch/parisc/include/asm/cmpxchg.h b/arch/parisc/include/asm/cmpxchg.h
index 5f274be105671e..c1d776bb16b4ed 100644
--- a/arch/parisc/include/asm/cmpxchg.h
+++ b/arch/parisc/include/asm/cmpxchg.h
@@ -22,7 +22,7 @@ extern unsigned long __xchg64(unsigned long, volatile unsigned long *);

/* optimizer better get rid of switch since size is a constant */
static inline unsigned long
-__xchg(unsigned long x, volatile void *ptr, int size)
+__arch_xchg(unsigned long x, volatile void *ptr, int size)
{
switch (size) {
#ifdef CONFIG_64BIT
@@ -49,7 +49,7 @@ __xchg(unsigned long x, volatile void *ptr, int size)
__typeof__(*(ptr)) __ret; \
__typeof__(*(ptr)) _x_ = (x); \
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)_x_, (ptr), sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)_x_, (ptr), sizeof(*(ptr))); \
__ret; \
})

diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h
index d0ea0571e79ab5..dbb50c06f0bf4d 100644
--- a/arch/powerpc/include/asm/cmpxchg.h
+++ b/arch/powerpc/include/asm/cmpxchg.h
@@ -229,7 +229,7 @@ __xchg_local(void *ptr, unsigned long x, unsigned int size)
return __xchg_u64_local(ptr, x);
#endif
}
- BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg");
+ BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
return x;
}

@@ -248,7 +248,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size)
return __xchg_u64_relaxed(ptr, x);
#endif
}
- BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
+ BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_relaxed");
return x;
}
#define arch_xchg_local(ptr,x) \
diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 0dfe9d857a762b..bba472928b5393 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -261,7 +261,7 @@ c_t arch_atomic##prefix##_xchg_release(atomic##prefix##_t *v, c_t n) \
static __always_inline \
c_t arch_atomic##prefix##_xchg(atomic##prefix##_t *v, c_t n) \
{ \
- return __xchg(&(v->counter), n, size); \
+ return __arch_xchg(&(v->counter), n, size); \
} \
static __always_inline \
c_t arch_atomic##prefix##_cmpxchg_relaxed(atomic##prefix##_t *v, \
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 12debce235e52d..2f4726d3cfcc25 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -114,7 +114,7 @@
_x_, sizeof(*(ptr))); \
})

-#define __xchg(ptr, new, size) \
+#define __arch_xchg(ptr, new, size) \
({ \
__typeof__(ptr) __ptr = (ptr); \
__typeof__(new) __new = (new); \
@@ -143,7 +143,7 @@
#define arch_xchg(ptr, x) \
({ \
__typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg((ptr), _x_, sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __arch_xchg((ptr), _x_, sizeof(*(ptr))); \
})

#define xchg32(ptr, x) \
diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h
index 84c3f0d576c5b1..ca32deff9880ea 100644
--- a/arch/s390/include/asm/cmpxchg.h
+++ b/arch/s390/include/asm/cmpxchg.h
@@ -14,8 +14,8 @@

void __xchg_called_with_bad_pointer(void);

-static __always_inline unsigned long __xchg(unsigned long x,
- unsigned long address, int size)
+static __always_inline unsigned long
+__arch_xchg(unsigned long x, unsigned long address, int size)
{
unsigned long old;
int shift;
@@ -77,8 +77,8 @@ static __always_inline unsigned long __xchg(unsigned long x,
__typeof__(*(ptr)) __ret; \
\
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)(x), (unsigned long)(ptr), \
- sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)(x), (unsigned long)(ptr), \
+ sizeof(*(ptr))); \
__ret; \
})

diff --git a/arch/sh/include/asm/cmpxchg.h b/arch/sh/include/asm/cmpxchg.h
index 0ed9b3f4a57796..288f6f38d98fb4 100644
--- a/arch/sh/include/asm/cmpxchg.h
+++ b/arch/sh/include/asm/cmpxchg.h
@@ -22,7 +22,7 @@

extern void __xchg_called_with_bad_pointer(void);

-#define __xchg(ptr, x, size) \
+#define __arch_xchg(ptr, x, size) \
({ \
unsigned long __xchg__res; \
volatile void *__xchg_ptr = (ptr); \
@@ -46,7 +46,7 @@ extern void __xchg_called_with_bad_pointer(void);
})

#define arch_xchg(ptr,x) \
- ((__typeof__(*(ptr)))__xchg((ptr),(unsigned long)(x), sizeof(*(ptr))))
+ ((__typeof__(*(ptr)))__arch_xchg((ptr),(unsigned long)(x), sizeof(*(ptr))))

/* This function doesn't exist, so you'll get a linker error
* if something tries to do an invalid cmpxchg(). */
diff --git a/arch/sparc/include/asm/cmpxchg_32.h b/arch/sparc/include/asm/cmpxchg_32.h
index 27a57a3a7597eb..7a1339533d1d7e 100644
--- a/arch/sparc/include/asm/cmpxchg_32.h
+++ b/arch/sparc/include/asm/cmpxchg_32.h
@@ -15,7 +15,7 @@
unsigned long __xchg_u32(volatile u32 *m, u32 new);
void __xchg_called_with_bad_pointer(void);

-static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr, int size)
+static inline unsigned long __arch_xchg(unsigned long x, __volatile__ void * ptr, int size)
{
switch (size) {
case 4:
@@ -25,7 +25,7 @@ static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr, int
return x;
}

-#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})
+#define arch_xchg(ptr,x) ({(__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr)));})

/* Emulate cmpxchg() the same way we emulate atomics,
* by hashing the object address and indexing into an array
diff --git a/arch/sparc/include/asm/cmpxchg_64.h b/arch/sparc/include/asm/cmpxchg_64.h
index 12d00a42c0a3ed..66cd61dde9ec1f 100644
--- a/arch/sparc/include/asm/cmpxchg_64.h
+++ b/arch/sparc/include/asm/cmpxchg_64.h
@@ -55,7 +55,7 @@ static inline unsigned long xchg64(__volatile__ unsigned long *m, unsigned long
#define arch_xchg(ptr,x) \
({ __typeof__(*(ptr)) __ret; \
__ret = (__typeof__(*(ptr))) \
- __xchg((unsigned long)(x), (ptr), sizeof(*(ptr))); \
+ __arch_xchg((unsigned long)(x), (ptr), sizeof(*(ptr))); \
__ret; \
})

@@ -87,8 +87,8 @@ xchg16(__volatile__ unsigned short *m, unsigned short val)
return (load32 & mask) >> bit_shift;
}

-static inline unsigned long __xchg(unsigned long x, __volatile__ void * ptr,
- int size)
+static inline unsigned long
+__arch_xchg(unsigned long x, __volatile__ void * ptr, int size)
{
switch (size) {
case 2:
diff --git a/arch/xtensa/include/asm/cmpxchg.h b/arch/xtensa/include/asm/cmpxchg.h
index eb87810357ad88..675a11ea8de76b 100644
--- a/arch/xtensa/include/asm/cmpxchg.h
+++ b/arch/xtensa/include/asm/cmpxchg.h
@@ -170,7 +170,7 @@ static inline unsigned long xchg_u32(volatile int * m, unsigned long val)
}

#define arch_xchg(ptr,x) \
- ((__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr))))
+ ((__typeof__(*(ptr)))__arch_xchg((unsigned long)(x),(ptr),sizeof(*(ptr))))

static inline u32 xchg_small(volatile void *ptr, u32 x, int size)
{
@@ -203,7 +203,7 @@ static inline u32 xchg_small(volatile void *ptr, u32 x, int size)
extern void __xchg_called_with_bad_pointer(void);

static __inline__ unsigned long
-__xchg(unsigned long x, volatile void * ptr, int size)
+__arch_xchg(unsigned long x, volatile void * ptr, int size)
{
switch (size) {
case 1:
--
2.34.1

2023-01-05 16:34:32

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg

On Thu, Dec 29, 2022 at 10:54:50AM +0100, Andrzej Hajda wrote:
> Forgive me late response - Holidays,
>
> On 22.12.2022 18:21, Andrew Morton wrote:
> > On Thu, 22 Dec 2022 12:46:16 +0100 Andrzej Hajda <[email protected]> wrote:
> >
> > > Hi all,
> > >
> > > I hope there will be place for such tiny helper in kernel.
> > > Quick cocci analyze shows there is probably few thousands places
> > > where it could be useful.
> > So to clarify, the intent here is a simple readability cleanup for
> > existing open-coded exchange operations.
>
> And replace private helpers with common one, see the last patch - the
> ultimate goal
> would be to replace all occurrences of fetch_and_zero with __xchg.
>
> > The intent is *not* to
> > identify existing xchg() sites which are unnecessarily atomic and to
> > optimize them by using the non-atomic version.
> >
> > Have you considered the latter?
>
> If you mean some way of (semi-)automatic detection of such cases, then no.
> Anyway this could be quite interesting challenge.

My take is that unless there is very clear demand for this macro from
outside of i915, it's not worth it. All that fetch_and_zero zero achieved
is make i915 code a lot more confusing to read for people who don't know
this thing. And it replaces 2 entirely standard lines of 0, every often
clearing pointers in data structures where you really want the verbosity
to have a reminder and thinking about the locking.

Plus it smells way too much like the cmpxchg family of atomic functions,
addig further to the locking confuion.

Imo the right approach is to just open code this macro in i915 and then
drop it. Again, unless enough people outside of i915 really really want
this, and want to lift this to a kernel idiom.
-Daniel

>
> >
> > > I am not sure who is good person to review/ack such patches,
> > I can take 'em.
> >
> > > so I've used my intuition to construct to/cc lists, sorry for mistakes.
> > > This is the 2nd approach of the same idea, with comments addressed[0].
> > >
> > > The helper is tiny and there are advices we can leave without it, so
> > > I want to present few arguments why it would be good to have it:
> > >
> > > 1. Code readability/simplification/number of lines:
> > >
> > > Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
> > > - previous_min_rate = evport->qos.min_rate;
> > > - evport->qos.min_rate = min_rate;
> > > + previous_min_rate = __xchg(evport->qos.min_rate, min_rate);
> > >
> > > For sure the code is more compact, and IMHO more readable.
> > >
> > > 2. Presence of similar helpers in other somehow related languages/libs:
> > >
> > > a) Rust[1]: 'replace' from std::mem module, there is also 'take'
> > > helper (__xchg(&x, 0)), which is the same as private helper in
> > > i915 - fetch_and_zero, see latest patch.
> > > b) C++ [2]: 'exchange' from utility header.
> > >
> > > If the idea is OK there are still 2 qestions to answer:
> > >
> > > 1. Name of the helper, __xchg follows kernel conventions,
> > > but for me Rust names are also OK.
> > I like replace(), or, shockingly, exchange().
> >
> > But... Can we simply make swap() return the previous value?
> >
> > previous_min_rate = swap(&evport->qos.min_rate, min_rate);
>
> As Alexander already pointed out, swap requires 'references' to two
> variables,
> in contrast to xchg which requires reference to variable and value.
> So we cannot use swap for cases:
> ??? old_value = __xchg(&x, new_value);
>
> Regards
> Andrzej
>

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2023-01-10 11:04:10

by Andrzej Hajda

[permalink] [raw]
Subject: [RFC DO NOT MERGE] treewide: use __xchg in most obvious places

This patch tries to show usability of __xchg helper.
It is not intended to be merged, but I can convert
it to proper patchset if necessary.

There are many more places where __xchg can be used.
This demo shows the most spectacular cases IMHO:
- previous value is returned from function,
- temporary variables are in use.

As a result readability is much better and diffstat is quite
nice, less local vars to look at.
In many cases whole body of functions is replaced
with __xchg(ptr, val), so as further refactoring the whole
function can be removed and __xchg can be called directly.

Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/arm/probes/uprobes/core.c | 8 ++------
arch/csky/kernel/probes/uprobes.c | 9 ++-------
arch/mips/kernel/irq_txx9.c | 7 ++-----
arch/mips/kernel/process.c | 8 +++-----
arch/mips/kernel/uprobes.c | 10 ++--------
arch/powerpc/include/asm/kvm_ppc.h | 7 ++-----
arch/powerpc/kernel/uprobes.c | 10 ++--------
arch/powerpc/mm/init_64.c | 7 ++-----
arch/riscv/kernel/probes/uprobes.c | 9 ++-------
arch/s390/kernel/uprobes.c | 7 ++-----
arch/s390/kvm/interrupt.c | 6 ++----
arch/sh/kernel/traps_32.c | 6 ++----
.../accessibility/speakup/speakup_dectlk.c | 7 ++-----
drivers/accessibility/speakup/speakup_soft.c | 7 ++-----
drivers/block/drbd/drbd_receiver.c | 5 ++---
drivers/cdrom/cdrom.c | 7 ++-----
drivers/gpu/drm/drm_atomic_uapi.c | 14 +++-----------
drivers/iommu/iova.c | 7 ++-----
drivers/misc/ti-st/st_core.c | 10 +++-------
drivers/mtd/nand/raw/qcom_nandc.c | 11 ++++-------
drivers/net/ethernet/ibm/ehea/ehea_main.c | 11 +++--------
.../microchip/sparx5/sparx5_calendar.c | 10 ++++------
drivers/net/usb/rtl8150.c | 9 +++------
drivers/net/wireless/intel/iwlwifi/pcie/rx.c | 8 +++-----
.../net/wireless/marvell/mwifiex/sta_ioctl.c | 7 +++----
drivers/scsi/device_handler/scsi_dh_alua.c | 8 +++-----
drivers/scsi/lpfc/lpfc_sli.c | 7 ++-----
drivers/staging/rtl8192e/rtllib_tx.c | 7 ++-----
drivers/tty/hvc/hvc_iucv.c | 8 +++-----
drivers/video/fbdev/sis/sis_main.c | 6 ++----
drivers/xen/grant-table.c | 6 ++----
fs/namespace.c | 6 ++----
include/linux/ptr_ring.h | 7 ++-----
include/linux/qed/qed_chain.h | 19 +++++++------------
io_uring/io_uring.c | 7 ++-----
mm/kmsan/init.c | 7 ++-----
mm/memcontrol.c | 8 ++------
net/mac80211/rc80211_minstrel_ht.c | 6 ++----
sound/pci/asihpi/hpidebug.c | 8 +++-----
.../selftests/bpf/progs/dummy_st_ops.c | 7 ++-----
40 files changed, 99 insertions(+), 225 deletions(-)

diff --git a/arch/arm/probes/uprobes/core.c b/arch/arm/probes/uprobes/core.c
index f5f790c6e5f896..77ce8ae431376d 100644
--- a/arch/arm/probes/uprobes/core.c
+++ b/arch/arm/probes/uprobes/core.c
@@ -9,6 +9,7 @@
#include <linux/highmem.h>
#include <linux/sched.h>
#include <linux/uprobes.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/notifier.h>

#include <asm/opcodes.h>
@@ -61,12 +62,7 @@ unsigned long
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
struct pt_regs *regs)
{
- unsigned long orig_ret_vaddr;
-
- orig_ret_vaddr = regs->ARM_lr;
- /* Replace the return addr with trampoline addr */
- regs->ARM_lr = trampoline_vaddr;
- return orig_ret_vaddr;
+ return __xchg(&regs->ARM_lr, trampoline_vaddr);
}

int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
diff --git a/arch/csky/kernel/probes/uprobes.c b/arch/csky/kernel/probes/uprobes.c
index 2d31a12e46cfee..775fe88b5f0016 100644
--- a/arch/csky/kernel/probes/uprobes.c
+++ b/arch/csky/kernel/probes/uprobes.c
@@ -3,6 +3,7 @@
* Copyright (C) 2014-2016 Pratyush Anand <[email protected]>
*/
#include <linux/highmem.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/ptrace.h>
#include <linux/uprobes.h>
#include <asm/cacheflush.h>
@@ -123,13 +124,7 @@ unsigned long
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
struct pt_regs *regs)
{
- unsigned long ra;
-
- ra = regs->lr;
-
- regs->lr = trampoline_vaddr;
-
- return ra;
+ return __xchg(&regs->lr, trampoline_vaddr);
}

int arch_uprobe_exception_notify(struct notifier_block *self,
diff --git a/arch/mips/kernel/irq_txx9.c b/arch/mips/kernel/irq_txx9.c
index af3ef4c9f7de1e..b5abe24ea7cfb9 100644
--- a/arch/mips/kernel/irq_txx9.c
+++ b/arch/mips/kernel/irq_txx9.c
@@ -15,6 +15,7 @@
*/
#include <linux/init.h>
#include <linux/interrupt.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/types.h>
#include <linux/irq.h>
#include <asm/txx9irq.h>
@@ -159,13 +160,9 @@ void __init txx9_irq_init(unsigned long baseaddr)

int __init txx9_irq_set_pri(int irc_irq, int new_pri)
{
- int old_pri;
-
if ((unsigned int)irc_irq >= TXx9_MAX_IR)
return 0;
- old_pri = txx9irq[irc_irq].level;
- txx9irq[irc_irq].level = new_pri;
- return old_pri;
+ return __xchg(&txx9irq[irc_irq].level, new_pri);
}

int txx9_irq(void)
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 093dbbd6b84350..2bafb4d4da034d 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -15,6 +15,7 @@
#include <linux/kallsyms.h>
#include <linux/kernel.h>
#include <linux/nmi.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/personality.h>
#include <linux/prctl.h>
#include <linux/random.h>
@@ -599,11 +600,8 @@ unsigned long notrace unwind_stack_by_address(unsigned long stack_page,
/*
* Return ra if an exception occurred at the first instruction
*/
- if (unlikely(ofs == 0)) {
- pc = *ra;
- *ra = 0;
- return pc;
- }
+ if (unlikely(ofs == 0))
+ return __xchg(ra, 0);

info.func = (void *)(pc - ofs);
info.func_size = ofs; /* analyze from start to ofs */
diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
index 6c063aa188e626..2b1f375c407b87 100644
--- a/arch/mips/kernel/uprobes.c
+++ b/arch/mips/kernel/uprobes.c
@@ -2,6 +2,7 @@
#include <linux/highmem.h>
#include <linux/kdebug.h>
#include <linux/types.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/notifier.h>
#include <linux/sched.h>
#include <linux/uprobes.h>
@@ -197,14 +198,7 @@ void arch_uprobe_abort_xol(struct arch_uprobe *aup,
unsigned long arch_uretprobe_hijack_return_addr(
unsigned long trampoline_vaddr, struct pt_regs *regs)
{
- unsigned long ra;
-
- ra = regs->regs[31];
-
- /* Replace the return address with the trampoline address */
- regs->regs[31] = trampoline_vaddr;
-
- return ra;
+ return __xchg(&regs->regs[31], trampoline_vaddr);
}

/**
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index eae9619b61903c..bf22b12d5e8e09 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -13,6 +13,7 @@
* dependencies. */

#include <linux/mutex.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/timer.h>
#include <linux/types.h>
#include <linux/kvm_types.h>
@@ -434,11 +435,7 @@ static inline void kvmppc_set_xive_tima(int cpu,

static inline u32 kvmppc_get_xics_latch(void)
{
- u32 xirr;
-
- xirr = get_paca()->kvm_hstate.saved_xirr;
- get_paca()->kvm_hstate.saved_xirr = 0;
- return xirr;
+ return __xchg(&get_paca()->kvm_hstate.saved_xirr, 0);
}

/*
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index 95a41ae9dfa755..3c15c320315e6c 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -7,6 +7,7 @@
* Adapted from the x86 port by Ananth N Mavinakayanahalli <[email protected]>
*/
#include <linux/kernel.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/sched.h>
#include <linux/ptrace.h>
#include <linux/uprobes.h>
@@ -197,14 +198,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
unsigned long
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs)
{
- unsigned long orig_ret_vaddr;
-
- orig_ret_vaddr = regs->link;
-
- /* Replace the return addr with trampoline addr */
- regs->link = trampoline_vaddr;
-
- return orig_ret_vaddr;
+ return __xchg(&regs->link, trampoline_vaddr);
}

bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index 05b0d584e50b82..9f6232f85a24f7 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -24,6 +24,7 @@
#include <linux/types.h>
#include <linux/mman.h>
#include <linux/mm.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/swap.h>
#include <linux/stddef.h>
#include <linux/vmalloc.h>
@@ -138,14 +139,10 @@ static int num_freed;

static __meminit struct vmemmap_backing * vmemmap_list_alloc(int node)
{
- struct vmemmap_backing *vmem_back;
/* get from freed entries first */
if (num_freed) {
num_freed--;
- vmem_back = next;
- next = next->list;
-
- return vmem_back;
+ return __xchg(&next, next->list);
}

/* allocate a page when required and hand out chunks */
diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
index c976a21cd4bd5b..5c8415e216536d 100644
--- a/arch/riscv/kernel/probes/uprobes.c
+++ b/arch/riscv/kernel/probes/uprobes.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only

#include <linux/highmem.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/ptrace.h>
#include <linux/uprobes.h>

@@ -122,13 +123,7 @@ unsigned long
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
struct pt_regs *regs)
{
- unsigned long ra;
-
- ra = regs->ra;
-
- regs->ra = trampoline_vaddr;
-
- return ra;
+ return __xchg(&regs->ra, trampoline_vaddr);
}

int arch_uprobe_exception_notify(struct notifier_block *self,
diff --git a/arch/s390/kernel/uprobes.c b/arch/s390/kernel/uprobes.c
index b88345ef8bd9ed..18591ca40ae7e5 100644
--- a/arch/s390/kernel/uprobes.c
+++ b/arch/s390/kernel/uprobes.c
@@ -11,6 +11,7 @@
#include <linux/compat.h>
#include <linux/kdebug.h>
#include <linux/sched/task_stack.h>
+#include <linux/non-atomic/xchg.h>

#include <asm/switch_to.h>
#include <asm/facility.h>
@@ -144,11 +145,7 @@ void arch_uprobe_abort_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline,
struct pt_regs *regs)
{
- unsigned long orig;
-
- orig = regs->gprs[14];
- regs->gprs[14] = trampoline;
- return orig;
+ return __xchg(&regs->gprs[14], trampoline);
}

bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 1dae78deddf286..56e4a7146affe8 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -14,6 +14,7 @@
#include <linux/kvm_host.h>
#include <linux/hrtimer.h>
#include <linux/mmu_context.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/nospec.h>
#include <linux/signal.h>
#include <linux/slab.h>
@@ -2484,14 +2485,11 @@ static int register_io_adapter(struct kvm_device *dev,

int kvm_s390_mask_adapter(struct kvm *kvm, unsigned int id, bool masked)
{
- int ret;
struct s390_io_adapter *adapter = get_io_adapter(kvm, id);

if (!adapter || !adapter->maskable)
return -EINVAL;
- ret = adapter->masked;
- adapter->masked = masked;
- return ret;
+ return __xchg(&adapter->masked, masked);
}

void kvm_s390_destroy_adapters(struct kvm *kvm)
diff --git a/arch/sh/kernel/traps_32.c b/arch/sh/kernel/traps_32.c
index 6cdda3a621a1e5..15402c827a6936 100644
--- a/arch/sh/kernel/traps_32.c
+++ b/arch/sh/kernel/traps_32.c
@@ -12,6 +12,7 @@
#include <linux/ptrace.h>
#include <linux/hardirq.h>
#include <linux/init.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/spinlock.h>
#include <linux/kallsyms.h>
#include <linux/io.h>
@@ -752,11 +753,8 @@ void per_cpu_trap_init(void)
void *set_exception_table_vec(unsigned int vec, void *handler)
{
extern void *exception_handling_table[];
- void *old_handler;

- old_handler = exception_handling_table[vec];
- exception_handling_table[vec] = handler;
- return old_handler;
+ return __xchg(&exception_handling_table[vec], handler);
}

void __init trap_init(void)
diff --git a/drivers/accessibility/speakup/speakup_dectlk.c b/drivers/accessibility/speakup/speakup_dectlk.c
index 56334405d865c5..05a2f6dbc4a92f 100644
--- a/drivers/accessibility/speakup/speakup_dectlk.c
+++ b/drivers/accessibility/speakup/speakup_dectlk.c
@@ -12,6 +12,7 @@
#include <linux/unistd.h>
#include <linux/proc_fs.h>
#include <linux/jiffies.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/spinlock.h>
#include <linux/sched.h>
#include <linux/timer.h>
@@ -172,11 +173,7 @@ static u_char lastind;

static unsigned char get_index(struct spk_synth *synth)
{
- u_char rv;
-
- rv = lastind;
- lastind = 0;
- return rv;
+ return __xchg(&lastind, 0);
}

static void read_buff_add(u_char c)
diff --git a/drivers/accessibility/speakup/speakup_soft.c b/drivers/accessibility/speakup/speakup_soft.c
index 6d446824677b45..81c586bbd754e2 100644
--- a/drivers/accessibility/speakup/speakup_soft.c
+++ b/drivers/accessibility/speakup/speakup_soft.c
@@ -11,6 +11,7 @@

#include <linux/unistd.h>
#include <linux/miscdevice.h> /* for misc_register, and MISC_DYNAMIC_MINOR */
+#include <linux/non-atomic/xchg.h>
#include <linux/poll.h> /* for poll_wait() */

/* schedule(), signal_pending(), TASK_INTERRUPTIBLE */
@@ -366,11 +367,7 @@ static __poll_t softsynth_poll(struct file *fp, struct poll_table_struct *wait)

static unsigned char get_index(struct spk_synth *synth)
{
- int rv;
-
- rv = last_index;
- last_index = 0;
- return rv;
+ return __xchg(&last_index, 0);
}

static const struct file_operations softsynth_fops = {
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 757f4692b5bd80..ad5762aa0d8846 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -23,6 +23,7 @@
#include <linux/mm.h>
#include <linux/memcontrol.h>
#include <linux/mm_inline.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/slab.h>
#include <uapi/linux/sched/types.h>
#include <linux/sched/signal.h>
@@ -99,9 +100,7 @@ static struct page *page_chain_del(struct page **head, int n)
/* add end of list marker for the returned list */
set_page_private(page, 0);
/* actual return value, and adjustment of head */
- page = *head;
- *head = tmp;
- return page;
+ return __xchg(head, tmp);
}

/* may be used outside of locks to find the tail of a (usually short)
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 416f723a2dbb33..48247e177a3739 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -264,6 +264,7 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/mm.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/slab.h>
#include <linux/cdrom.h>
#include <linux/sysctl.h>
@@ -1490,12 +1491,8 @@ static void cdrom_update_events(struct cdrom_device_info *cdi,
unsigned int cdrom_check_events(struct cdrom_device_info *cdi,
unsigned int clearing)
{
- unsigned int events;
-
cdrom_update_events(cdi, clearing);
- events = cdi->vfs_events;
- cdi->vfs_events = 0;
- return events;
+ return __xchg(&cdi->vfs_events, 0);
}
EXPORT_SYMBOL(cdrom_check_events);

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index d867e7f9f2cd58..1ee36101a3ebe9 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -36,6 +36,7 @@
#include <drm/drm_vblank.h>

#include <linux/dma-fence.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/uaccess.h>
#include <linux/sync_file.h>
#include <linux/file.h>
@@ -325,12 +326,7 @@ static void set_out_fence_for_crtc(struct drm_atomic_state *state,
static s32 __user *get_out_fence_for_crtc(struct drm_atomic_state *state,
struct drm_crtc *crtc)
{
- s32 __user *fence_ptr;
-
- fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
- state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
-
- return fence_ptr;
+ return __xchg(&state->crtcs[drm_crtc_index(crtc)].out_fence_ptr, NULL);
}

static int set_out_fence_for_connector(struct drm_atomic_state *state,
@@ -354,12 +350,8 @@ static s32 __user *get_out_fence_for_connector(struct drm_atomic_state *state,
struct drm_connector *connector)
{
unsigned int index = drm_connector_index(connector);
- s32 __user *fence_ptr;
-
- fence_ptr = state->connectors[index].out_fence_ptr;
- state->connectors[index].out_fence_ptr = NULL;

- return fence_ptr;
+ return __xchg(&state->connectors[index].out_fence_ptr, NULL);
}

static int
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 7d246bce7d8483..ee10135992a8cf 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -7,6 +7,7 @@

#include <linux/iova.h>
#include <linux/module.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/slab.h>
#include <linux/smp.h>
#include <linux/bitops.h>
@@ -668,7 +669,6 @@ static unsigned long iova_magazine_pop(struct iova_magazine *mag,
unsigned long limit_pfn)
{
int i;
- unsigned long pfn;

/* Only fall back to the rbtree if we have no suitable pfns at all */
for (i = mag->size - 1; mag->pfns[i] > limit_pfn; i--)
@@ -676,10 +676,7 @@ static unsigned long iova_magazine_pop(struct iova_magazine *mag,
return 0;

/* Swap it to pop it */
- pfn = mag->pfns[i];
- mag->pfns[i] = mag->pfns[--mag->size];
-
- return pfn;
+ return __xchg(&mag->pfns[i], mag->pfns[--mag->size]);
}

static void iova_magazine_push(struct iova_magazine *mag, unsigned long pfn)
diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c
index 7f6976a9f508bd..678b966708a237 100644
--- a/drivers/misc/ti-st/st_core.c
+++ b/drivers/misc/ti-st/st_core.c
@@ -9,6 +9,7 @@
#define pr_fmt(fmt) "(stc): " fmt
#include <linux/module.h>
#include <linux/kernel.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/tty.h>

#include <linux/seq_file.h>
@@ -397,14 +398,9 @@ void st_int_recv(void *disc_data,
*/
static struct sk_buff *st_int_dequeue(struct st_data_s *st_gdata)
{
- struct sk_buff *returning_skb;
-
pr_debug("%s", __func__);
- if (st_gdata->tx_skb != NULL) {
- returning_skb = st_gdata->tx_skb;
- st_gdata->tx_skb = NULL;
- return returning_skb;
- }
+ if (st_gdata->tx_skb != NULL)
+ return __xchg(&st_gdata->tx_skb, NULL);
return skb_dequeue(&st_gdata->txq);
}

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 198a44794d2dc0..6b98e0e943937e 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -11,6 +11,7 @@
#include <linux/module.h>
#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/delay.h>
@@ -2507,13 +2508,9 @@ static uint8_t qcom_nandc_read_byte(struct nand_chip *chip)
u8 *buf = nandc->data_buffer;
u8 ret = 0x0;

- if (host->last_command == NAND_CMD_STATUS) {
- ret = host->status;
-
- host->status = NAND_STATUS_READY | NAND_STATUS_WP;
-
- return ret;
- }
+ if (host->last_command == NAND_CMD_STATUS)
+ return __xchg(&host->status,
+ NAND_STATUS_READY | NAND_STATUS_WP);

if (nandc->buf_start < nandc->buf_count)
ret = buf[nandc->buf_start++];
diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index b4aff59b3eb4fd..fba3c4e5e23f04 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -17,6 +17,7 @@
#include <linux/device.h>
#include <linux/in.h>
#include <linux/ip.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/tcp.h>
#include <linux/udp.h>
#include <linux/if.h>
@@ -564,7 +565,6 @@ static inline struct sk_buff *get_skb_by_index(struct sk_buff **skb_array,
struct ehea_cqe *cqe)
{
int skb_index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX, cqe->wr_id);
- struct sk_buff *skb;
void *pref;
int x;

@@ -583,15 +583,12 @@ static inline struct sk_buff *get_skb_by_index(struct sk_buff **skb_array,
prefetch(pref + EHEA_CACHE_LINE * 3);
}

- skb = skb_array[skb_index];
- skb_array[skb_index] = NULL;
- return skb;
+ return __xchg(&skb_array[skb_index], NULL);
}

static inline struct sk_buff *get_skb_by_index_ll(struct sk_buff **skb_array,
int arr_len, int wqe_index)
{
- struct sk_buff *skb;
void *pref;
int x;

@@ -608,9 +605,7 @@ static inline struct sk_buff *get_skb_by_index_ll(struct sk_buff **skb_array,
prefetchw(pref + EHEA_CACHE_LINE);
}

- skb = skb_array[wqe_index];
- skb_array[wqe_index] = NULL;
- return skb;
+ return __xchg(&skb_array[wqe_index], NULL);
}

static int ehea_treat_poll_error(struct ehea_port_res *pr, int rq,
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c b/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
index 76a8bb596aec1e..0e99a2343f4765 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
@@ -6,6 +6,7 @@

#include <linux/module.h>
#include <linux/device.h>
+#include <linux/non-atomic/xchg.h>

#include "sparx5_main_regs.h"
#include "sparx5_main.h"
@@ -265,14 +266,11 @@ static u32 sparx5_dsm_cal_len(u32 *cal)

static u32 sparx5_dsm_cp_cal(u32 *sched)
{
- u32 idx = 0, tmp;
+ u32 idx = 0;

while (idx < SPX5_DSM_CAL_LEN) {
- if (sched[idx] != SPX5_DSM_CAL_EMPTY) {
- tmp = sched[idx];
- sched[idx] = SPX5_DSM_CAL_EMPTY;
- return tmp;
- }
+ if (sched[idx] != SPX5_DSM_CAL_EMPTY)
+ return __xchg(&sched[idx], SPX5_DSM_CAL_EMPTY);
idx++;
}
return SPX5_DSM_CAL_EMPTY;
diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 97afd7335d8685..717a1abb5a4d4a 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -12,6 +12,7 @@
#include <linux/ethtool.h>
#include <linux/usb.h>
#include <linux/uaccess.h>
+#include <linux/non-atomic/xchg.h>

/* Version Information */
#define DRIVER_VERSION "v0.6.2 (2004/08/27)"
@@ -354,15 +355,11 @@ static void unlink_all_urbs(rtl8150_t * dev)

static inline struct sk_buff *pull_skb(rtl8150_t *dev)
{
- struct sk_buff *skb;
int i;

for (i = 0; i < RX_SKB_POOL_SIZE; i++) {
- if (dev->rx_skb_pool[i]) {
- skb = dev->rx_skb_pool[i];
- dev->rx_skb_pool[i] = NULL;
- return skb;
- }
+ if (dev->rx_skb_pool[i])
+ return __xchg(&dev->rx_skb_pool[i], NULL);
}
return NULL;
}
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/rx.c b/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
index 9c9f87fe837770..c595caa36d94a2 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
@@ -7,6 +7,7 @@
#include <linux/sched.h>
#include <linux/wait.h>
#include <linux/gfp.h>
+#include <linux/non-atomic/xchg.h>

#include "iwl-prph.h"
#include "iwl-io.h"
@@ -1430,11 +1431,8 @@ static struct iwl_rx_mem_buffer *iwl_pcie_get_rxb(struct iwl_trans *trans,
BUILD_BUG_ON(sizeof(struct iwl_rx_completion_desc) != 32);
BUILD_BUG_ON(sizeof(struct iwl_rx_completion_desc_bz) != 4);

- if (!trans->trans_cfg->mq_rx_supported) {
- rxb = rxq->queue[i];
- rxq->queue[i] = NULL;
- return rxb;
- }
+ if (!trans->trans_cfg->mq_rx_supported)
+ return __xchg(&rxq->queue[i], NULL);

if (trans->trans_cfg->device_family >= IWL_DEVICE_FAMILY_BZ) {
struct iwl_rx_completion_desc_bz *cd = rxq->used_bd;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index a2ad2b53f01683..ffad53f0a0b6dd 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -5,6 +5,8 @@
* Copyright 2011-2020 NXP
*/

+#include <linux/non-atomic/xchg.h>
+
#include "decl.h"
#include "ioctl.h"
#include "util.h"
@@ -60,10 +62,7 @@ int mwifiex_wait_queue_complete(struct mwifiex_adapter *adapter,
return status;
}

- status = adapter->cmd_wait_q.status;
- adapter->cmd_wait_q.status = 0;
-
- return status;
+ return __xchg(&adapter->cmd_wait_q.status, 0);
}

/*
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 49cc18a8747313..8d20ca91d10302 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -8,6 +8,7 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/module.h>
+#include <linux/non-atomic/xchg.h>
#include <asm/unaligned.h>
#include <scsi/scsi.h>
#include <scsi/scsi_proto.h>
@@ -821,7 +822,7 @@ static struct scsi_device * __must_check
alua_rtpg_select_sdev(struct alua_port_group *pg)
{
struct alua_dh_data *h;
- struct scsi_device *sdev = NULL, *prev_sdev;
+ struct scsi_device *sdev = NULL;

lockdep_assert_held(&pg->lock);
if (WARN_ON(!pg->rtpg_sdev))
@@ -857,10 +858,7 @@ alua_rtpg_select_sdev(struct alua_port_group *pg)

sdev_printk(KERN_INFO, sdev, "rtpg retry on different device\n");

- prev_sdev = pg->rtpg_sdev;
- pg->rtpg_sdev = sdev;
-
- return prev_sdev;
+ return __xchg(&pg->rtpg_sdev, sdev);
}

static void alua_rtpg_work(struct work_struct *work)
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 182aaae6038689..16b5317572832e 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -27,6 +27,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/lockdep.h>
+#include <linux/non-atomic/xchg.h>

#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -943,11 +944,7 @@ __lpfc_sli_get_iocbq(struct lpfc_hba *phba)
struct lpfc_sglq *
__lpfc_clear_active_sglq(struct lpfc_hba *phba, uint16_t xritag)
{
- struct lpfc_sglq *sglq;
-
- sglq = phba->sli4_hba.lpfc_sglq_active_list[xritag];
- phba->sli4_hba.lpfc_sglq_active_list[xritag] = NULL;
- return sglq;
+ return __xchg(&phba->sli4_hba.lpfc_sglq_active_list[xritag], NULL);
}

/**
diff --git a/drivers/staging/rtl8192e/rtllib_tx.c b/drivers/staging/rtl8192e/rtllib_tx.c
index 9ab8ee46ef6689..56c7d666e40433 100644
--- a/drivers/staging/rtl8192e/rtllib_tx.c
+++ b/drivers/staging/rtl8192e/rtllib_tx.c
@@ -20,6 +20,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/netdevice.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/pci.h>
#include <linux/proc_fs.h>
#include <linux/skbuff.h>
@@ -493,8 +494,6 @@ static void rtllib_txrate_selectmode(struct rtllib_device *ieee,
static u16 rtllib_query_seqnum(struct rtllib_device *ieee, struct sk_buff *skb,
u8 *dst)
{
- u16 seqnum = 0;
-
if (is_multicast_ether_addr(dst))
return 0;
if (IsQoSDataFrame(skb->data)) {
@@ -503,9 +502,7 @@ static u16 rtllib_query_seqnum(struct rtllib_device *ieee, struct sk_buff *skb,
if (!GetTs(ieee, (struct ts_common_info **)(&pTS), dst,
skb->priority, TX_DIR, true))
return 0;
- seqnum = pTS->TxCurSeq;
- pTS->TxCurSeq = (pTS->TxCurSeq + 1) % 4096;
- return seqnum;
+ return __xchg(&pTS->TxCurSeq, (pTS->TxCurSeq + 1) % 4096);
}
return 0;
}
diff --git a/drivers/tty/hvc/hvc_iucv.c b/drivers/tty/hvc/hvc_iucv.c
index 7d49a872de48ab..14b19bdb54add6 100644
--- a/drivers/tty/hvc/hvc_iucv.c
+++ b/drivers/tty/hvc/hvc_iucv.c
@@ -21,6 +21,7 @@
#include <linux/init.h>
#include <linux/mempool.h>
#include <linux/moduleparam.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/tty.h>
#include <linux/wait.h>
#include <net/iucv/iucv.h>
@@ -388,7 +389,7 @@ static int hvc_iucv_queue(struct hvc_iucv_private *priv, const char *buf,
static int hvc_iucv_send(struct hvc_iucv_private *priv)
{
struct iucv_tty_buffer *sb;
- int rc, len;
+ int rc;

if (priv->iucv_state == IUCV_SEVERED)
return -EPIPE;
@@ -419,10 +420,7 @@ static int hvc_iucv_send(struct hvc_iucv_private *priv)
list_del(&sb->list);
destroy_tty_buffer(sb);
}
- len = priv->sndbuf_len;
- priv->sndbuf_len = 0;
-
- return len;
+ return __xchg(&priv->sndbuf_len, 0);
}

/**
diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c
index cfba776afcea6d..a899d9bcf45361 100644
--- a/drivers/video/fbdev/sis/sis_main.c
+++ b/drivers/video/fbdev/sis/sis_main.c
@@ -27,6 +27,7 @@
#include <linux/errno.h>
#include <linux/string.h>
#include <linux/mm.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/screen_info.h>
#include <linux/slab.h>
#include <linux/fb.h>
@@ -3261,10 +3262,7 @@ sisfb_poh_new_node(struct SIS_HEAP *memheap)
memheap->poh_freelist = &poha->aoh[0];
}

- poh = memheap->poh_freelist;
- memheap->poh_freelist = poh->poh_next;
-
- return poh;
+ return __xchg(&memheap->poh_freelist, poh->poh_next);
}

static struct SIS_OH *
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index e1ec725c2819d4..471e8aacb87c1d 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -37,6 +37,7 @@
#include <linux/memblock.h>
#include <linux/sched.h>
#include <linux/mm.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
#include <linux/uaccess.h>
@@ -901,10 +902,7 @@ static inline struct page *cache_deq(struct gnttab_page_cache *cache)
{
struct page *page;

- page = cache->pages;
- cache->pages = page->zone_device_data;
-
- return page;
+ return __xchg(&cache->pages, page->zone_device_data);
}

static inline void cache_enq(struct gnttab_page_cache *cache, struct page *page)
diff --git a/fs/namespace.c b/fs/namespace.c
index ab467ee5834115..77e479b7dcd011 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -12,6 +12,7 @@
#include <linux/export.h>
#include <linux/capability.h>
#include <linux/mnt_namespace.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/user_namespace.h>
#include <linux/namei.h>
#include <linux/security.h>
@@ -990,15 +991,12 @@ static void __touch_mnt_namespace(struct mnt_namespace *ns)
*/
static struct mountpoint *unhash_mnt(struct mount *mnt)
{
- struct mountpoint *mp;
mnt->mnt_parent = mnt;
mnt->mnt_mountpoint = mnt->mnt.mnt_root;
list_del_init(&mnt->mnt_child);
hlist_del_init_rcu(&mnt->mnt_hash);
hlist_del_init(&mnt->mnt_mp_list);
- mp = mnt->mnt_mp;
- mnt->mnt_mp = NULL;
- return mp;
+ return __xchg(&mnt->mnt_mp, NULL);
}

/*
diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
index 808f9d3ee54654..62403901aaf391 100644
--- a/include/linux/ptr_ring.h
+++ b/include/linux/ptr_ring.h
@@ -24,6 +24,7 @@
#include <linux/compiler.h>
#include <linux/slab.h>
#include <linux/mm.h>
+#include <linux/non-atomic/xchg.h>
#include <asm/errno.h>
#endif

@@ -560,7 +561,6 @@ static inline void **__ptr_ring_swap_queue(struct ptr_ring *r, void **queue,
void (*destroy)(void *))
{
int producer = 0;
- void **old;
void *ptr;

while ((ptr = __ptr_ring_consume(r)))
@@ -575,10 +575,7 @@ static inline void **__ptr_ring_swap_queue(struct ptr_ring *r, void **queue,
r->producer = producer;
r->consumer_head = 0;
r->consumer_tail = 0;
- old = r->queue;
- r->queue = queue;
-
- return old;
+ return __xchg(&r->queue, queue);
}

/*
diff --git a/include/linux/qed/qed_chain.h b/include/linux/qed/qed_chain.h
index a84063492c71ae..3ea0d9442543ec 100644
--- a/include/linux/qed/qed_chain.h
+++ b/include/linux/qed/qed_chain.h
@@ -11,6 +11,7 @@
#include <asm/byteorder.h>
#include <linux/kernel.h>
#include <linux/list.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/sizes.h>
#include <linux/slab.h>
#include <linux/qed/common_hsi.h>
@@ -368,7 +369,7 @@ static inline void qed_chain_return_produced(struct qed_chain *p_chain)
*/
static inline void *qed_chain_produce(struct qed_chain *p_chain)
{
- void *p_ret = NULL, *p_prod_idx, *p_prod_page_idx;
+ void *p_prod_idx, *p_prod_page_idx;

if (is_chain_u16(p_chain)) {
if ((p_chain->u.chain16.prod_idx &
@@ -390,11 +391,8 @@ static inline void *qed_chain_produce(struct qed_chain *p_chain)
p_chain->u.chain32.prod_idx++;
}

- p_ret = p_chain->p_prod_elem;
- p_chain->p_prod_elem = (void *)(((u8 *)p_chain->p_prod_elem) +
- p_chain->elem_size);
-
- return p_ret;
+ return __xchg(&p_chain->p_prod_elem,
+ (void *)(((u8 *)p_chain->p_prod_elem) + p_chain->elem_size));
}

/**
@@ -439,7 +437,7 @@ static inline void qed_chain_recycle_consumed(struct qed_chain *p_chain)
*/
static inline void *qed_chain_consume(struct qed_chain *p_chain)
{
- void *p_ret = NULL, *p_cons_idx, *p_cons_page_idx;
+ void *p_cons_idx, *p_cons_page_idx;

if (is_chain_u16(p_chain)) {
if ((p_chain->u.chain16.cons_idx &
@@ -461,11 +459,8 @@ static inline void *qed_chain_consume(struct qed_chain *p_chain)
p_chain->u.chain32.cons_idx++;
}

- p_ret = p_chain->p_cons_elem;
- p_chain->p_cons_elem = (void *)(((u8 *)p_chain->p_cons_elem) +
- p_chain->elem_size);
-
- return p_ret;
+ return __xchg(&p_chain->p_cons_elem,
+ (void *)(((u8 *)p_chain->p_cons_elem) + p_chain->elem_size));
}

/**
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 58ac13b69dc8dc..f6881099307670 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -54,6 +54,7 @@
#include <linux/fdtable.h>
#include <linux/mm.h>
#include <linux/mman.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/percpu.h>
#include <linux/slab.h>
#include <linux/bvec.h>
@@ -1096,8 +1097,6 @@ static void __io_req_find_next_prep(struct io_kiocb *req)

static inline struct io_kiocb *io_req_find_next(struct io_kiocb *req)
{
- struct io_kiocb *nxt;
-
/*
* If LINK is set, we have dependent requests in this chain. If we
* didn't fail this request, queue the first one up, moving any other
@@ -1106,9 +1105,7 @@ static inline struct io_kiocb *io_req_find_next(struct io_kiocb *req)
*/
if (unlikely(req->flags & IO_DISARM_MASK))
__io_req_find_next_prep(req);
- nxt = req->link;
- req->link = NULL;
- return nxt;
+ return __xchg(&req->link, NULL);
}

static void ctx_flush_and_put(struct io_ring_ctx *ctx, bool *locked)
diff --git a/mm/kmsan/init.c b/mm/kmsan/init.c
index 7fb794242fad01..2857276fb8a5b7 100644
--- a/mm/kmsan/init.c
+++ b/mm/kmsan/init.c
@@ -12,6 +12,7 @@
#include <asm/sections.h>
#include <linux/mm.h>
#include <linux/memblock.h>
+#include <linux/non-atomic/xchg.h>

#include "../internal.h"

@@ -154,13 +155,9 @@ static void smallstack_push(struct smallstack *stack, struct page *pages)

static struct page *smallstack_pop(struct smallstack *stack)
{
- struct page *ret;
-
KMSAN_WARN_ON(stack->index == 0);
stack->index--;
- ret = stack->items[stack->index];
- stack->items[stack->index] = NULL;
- return ret;
+ return __xchg(&stack->items[stack->index], NULL);
}

static void do_collection(void)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ab457f0394ab6e..120cf6e8c265e1 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -27,6 +27,7 @@

#include <linux/page_counter.h>
#include <linux/memcontrol.h>
+#include <linux/non-atomic/xchg.h>
#include <linux/cgroup.h>
#include <linux/pagewalk.h>
#include <linux/sched/mm.h>
@@ -5968,16 +5969,11 @@ static const struct mm_walk_ops precharge_walk_ops = {

static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
{
- unsigned long precharge;
-
mmap_read_lock(mm);
walk_page_range(mm, 0, ULONG_MAX, &precharge_walk_ops, NULL);
mmap_read_unlock(mm);

- precharge = mc.precharge;
- mc.precharge = 0;
-
- return precharge;
+ return __xchg(&mc.precharge, 0);
}

static int mem_cgroup_precharge_mc(struct mm_struct *mm)
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 762346598338d3..291903561049a1 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -11,6 +11,7 @@
#include <linux/moduleparam.h>
#include <linux/ieee80211.h>
#include <linux/minmax.h>
+#include <linux/non-atomic/xchg.h>
#include <net/mac80211.h>
#include "rate.h"
#include "sta_info.h"
@@ -702,16 +703,13 @@ __minstrel_ht_get_sample_rate(struct minstrel_ht_sta *mi,
enum minstrel_sample_type type)
{
u16 *rates = mi->sample[type].sample_rates;
- u16 cur;
int i;

for (i = 0; i < MINSTREL_SAMPLE_RATES; i++) {
if (!rates[i])
continue;

- cur = rates[i];
- rates[i] = 0;
- return cur;
+ return __xchg(&rates[i], 0);
}

return 0;
diff --git a/sound/pci/asihpi/hpidebug.c b/sound/pci/asihpi/hpidebug.c
index 9570d9a44fe8db..3ad85f7423c5a3 100644
--- a/sound/pci/asihpi/hpidebug.c
+++ b/sound/pci/asihpi/hpidebug.c
@@ -9,6 +9,8 @@ Debug macro translation.

************************************************************************/

+#include <linux/non-atomic/xchg.h>
+
#include "hpi_internal.h"
#include "hpidebug.h"

@@ -22,11 +24,7 @@ void hpi_debug_init(void)

int hpi_debug_level_set(int level)
{
- int old_level;
-
- old_level = hpi_debug_level;
- hpi_debug_level = level;
- return old_level;
+ return __xchg(&hpi_debug_level, level);
}

int hpi_debug_level_get(void)
diff --git a/tools/testing/selftests/bpf/progs/dummy_st_ops.c b/tools/testing/selftests/bpf/progs/dummy_st_ops.c
index ead87edb75e257..8acfbbfcf6006a 100644
--- a/tools/testing/selftests/bpf/progs/dummy_st_ops.c
+++ b/tools/testing/selftests/bpf/progs/dummy_st_ops.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (C) 2021. Huawei Technologies Co., Ltd */
#include <linux/bpf.h>
+#include <linux/non-atomic/xchg.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>

@@ -19,14 +20,10 @@ char _license[] SEC("license") = "GPL";
SEC("struct_ops/test_1")
int BPF_PROG(test_1, struct bpf_dummy_ops_state *state)
{
- int ret;
-
if (!state)
return 0xf2f3f4f5;

- ret = state->val;
- state->val = 0x5a;
- return ret;
+ return __xchg(&state->val, 0x5a);
}

__u64 test_2_args[5];
--
2.34.1

2023-01-10 11:44:28

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC DO NOT MERGE] treewide: use __xchg in most obvious places

On Tue, Jan 10, 2023 at 11:53:06AM +0100, Andrzej Hajda wrote:
> This patch tries to show usability of __xchg helper.
> It is not intended to be merged, but I can convert
> it to proper patchset if necessary.
>
> There are many more places where __xchg can be used.
> This demo shows the most spectacular cases IMHO:
> - previous value is returned from function,
> - temporary variables are in use.
>
> As a result readability is much better and diffstat is quite
> nice, less local vars to look at.
> In many cases whole body of functions is replaced
> with __xchg(ptr, val), so as further refactoring the whole
> function can be removed and __xchg can be called directly.

...

> arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
> struct pt_regs *regs)
> {
> - unsigned long orig_ret_vaddr;
> -
> - orig_ret_vaddr = regs->ARM_lr;
> - /* Replace the return addr with trampoline addr */
> - regs->ARM_lr = trampoline_vaddr;
> - return orig_ret_vaddr;
> + return __xchg(&regs->ARM_lr, trampoline_vaddr);
> }

If it's not a callback, the entire function can be killed.
And this is a good example of the function usage.
OTOH, these places might have a side effect (if it's in deep CPU
handlers), means we need to do this carefully.

...

> static inline void *qed_chain_produce(struct qed_chain *p_chain)
> {
> - void *p_ret = NULL, *p_prod_idx, *p_prod_page_idx;
> + void *p_prod_idx, *p_prod_page_idx;
>
> if (is_chain_u16(p_chain)) {
> if ((p_chain->u.chain16.prod_idx &
> @@ -390,11 +391,8 @@ static inline void *qed_chain_produce(struct qed_chain *p_chain)
> p_chain->u.chain32.prod_idx++;
> }
>
> - p_ret = p_chain->p_prod_elem;
> - p_chain->p_prod_elem = (void *)(((u8 *)p_chain->p_prod_elem) +
> - p_chain->elem_size);
> -
> - return p_ret;
> + return __xchg(&p_chain->p_prod_elem,
> + (void *)(((u8 *)p_chain->p_prod_elem) + p_chain->elem_size));

Wondering if you still need a (void *) casting after the change. Ditto for the
rest of similar cases.

> }

...

Btw, is it done by coccinelle? If no, why not providing the script?

--
With Best Regards,
Andy Shevchenko


2023-01-10 13:07:36

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [Intel-gfx] [RFC DO NOT MERGE] treewide: use __xchg in most obvious places

On 10.01.2023 12:07, Andy Shevchenko wrote:
> On Tue, Jan 10, 2023 at 11:53:06AM +0100, Andrzej Hajda wrote:
>> This patch tries to show usability of __xchg helper.
>> It is not intended to be merged, but I can convert
>> it to proper patchset if necessary.
>>
>> There are many more places where __xchg can be used.
>> This demo shows the most spectacular cases IMHO:
>> - previous value is returned from function,
>> - temporary variables are in use.
>>
>> As a result readability is much better and diffstat is quite
>> nice, less local vars to look at.
>> In many cases whole body of functions is replaced
>> with __xchg(ptr, val), so as further refactoring the whole
>> function can be removed and __xchg can be called directly.
>
> ...
>
>> arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
>> struct pt_regs *regs)
>> {
>> - unsigned long orig_ret_vaddr;
>> -
>> - orig_ret_vaddr = regs->ARM_lr;
>> - /* Replace the return addr with trampoline addr */
>> - regs->ARM_lr = trampoline_vaddr;
>> - return orig_ret_vaddr;
>> + return __xchg(&regs->ARM_lr, trampoline_vaddr);
>> }
>
> If it's not a callback, the entire function can be killed.
> And this is a good example of the function usage.
> OTOH, these places might have a side effect (if it's in deep CPU
> handlers), means we need to do this carefully.
>
> ...
>
>> static inline void *qed_chain_produce(struct qed_chain *p_chain)
>> {
>> - void *p_ret = NULL, *p_prod_idx, *p_prod_page_idx;
>> + void *p_prod_idx, *p_prod_page_idx;
>>
>> if (is_chain_u16(p_chain)) {
>> if ((p_chain->u.chain16.prod_idx &
>> @@ -390,11 +391,8 @@ static inline void *qed_chain_produce(struct qed_chain *p_chain)
>> p_chain->u.chain32.prod_idx++;
>> }
>>
>> - p_ret = p_chain->p_prod_elem;
>> - p_chain->p_prod_elem = (void *)(((u8 *)p_chain->p_prod_elem) +
>> - p_chain->elem_size);
>> -
>> - return p_ret;
>> + return __xchg(&p_chain->p_prod_elem,
>> + (void *)(((u8 *)p_chain->p_prod_elem) + p_chain->elem_size));
>
> Wondering if you still need a (void *) casting after the change. Ditto for the
> rest of similar cases.

IMHO it is not needed also before the change and IIRC gcc has an
extension which allows to drop (u8 *) cast as well [1].

[1]: https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html

>
>> }
>
> ...
>
> Btw, is it done by coccinelle? If no, why not providing the script?
>

Yes I have used cocci. My cocci skills are far from perfect, so I did
not want to share my dirty code, but this is nothing secret:

@[email protected]
expression x, v;
local idexpression p;
@@
- p = x;
- x = v;
- return p;
+ return __xchg(&x, v);

@depends on [email protected]
expression e;
@@
__xchg(
- &*e,
+ e,
...)

@depends on [email protected]
expression t;
@@
- if (t) {
+ if (t)
return __xchg(...);
- }

@depends on [email protected]
type t;
identifier p;
expression e;
@@
(
- t p;
|
- t p = e;
)
... when != p

Regards
Andrzej

2023-01-10 13:58:16

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [Intel-gfx] [RFC DO NOT MERGE] treewide: use __xchg in most obvious places

On Tue, Jan 10, 2023 at 01:46:37PM +0100, Andrzej Hajda wrote:
> On 10.01.2023 12:07, Andy Shevchenko wrote:
> > On Tue, Jan 10, 2023 at 11:53:06AM +0100, Andrzej Hajda wrote:

...

> > > + return __xchg(&p_chain->p_prod_elem,
> > > + (void *)(((u8 *)p_chain->p_prod_elem) + p_chain->elem_size));
> >
> > Wondering if you still need a (void *) casting after the change. Ditto for the
> > rest of similar cases.
>
> IMHO it is not needed also before the change and IIRC gcc has an extension
> which allows to drop (u8 *) cast as well [1].

I guess you can drop at least the former one.

> [1]: https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html

...

> > Btw, is it done by coccinelle? If no, why not providing the script?
>
> Yes I have used cocci. My cocci skills are far from perfect, so I did not
> want to share my dirty code, but this is nothing secret:

Thank you! It's not about secrecy, it's about automation / error proofness.

--
With Best Regards,
Andy Shevchenko


2023-01-16 11:10:03

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v4] arch: rename all internal names __xchg to __arch_xchg

On Thu, Jan 5, 2023 at 10:54 AM Andrzej Hajda <[email protected]> wrote:
> __xchg will be used for non-atomic xchg macro.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Reviewed-by: Arnd Bergmann <[email protected]>
> ---
> v2: squashed all arch patches into one
> v3: fixed alpha/xchg_local, thx to [email protected]
> v4: adjusted indentation (Heiko)

> arch/m68k/include/asm/cmpxchg.h | 6 +++---

Acked-by: Geert Uytterhoeven <[email protected]> [m68k]

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds