Provide the generic sync_try_cmpxchg function from the
raw_ prefixed version, also adding explicit instrumentation.
Cc: Will Deacon <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: Mark Rutland <[email protected]>
Signed-off-by: Uros Bizjak <[email protected]>
---
include/linux/atomic/atomic-arch-fallback.h | 15 +++++++++-
include/linux/atomic/atomic-instrumented.h | 10 ++++++-
scripts/atomic/gen-atomic-fallback.sh | 33 +++++++++++----------
scripts/atomic/gen-atomic-instrumented.sh | 3 +-
4 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/include/linux/atomic/atomic-arch-fallback.h b/include/linux/atomic/atomic-arch-fallback.h
index b83ef19da13d..5e95faa959c4 100644
--- a/include/linux/atomic/atomic-arch-fallback.h
+++ b/include/linux/atomic/atomic-arch-fallback.h
@@ -428,6 +428,19 @@ extern void raw_cmpxchg128_relaxed_not_implemented(void);
#define raw_sync_cmpxchg arch_sync_cmpxchg
+#ifdef arch_sync_try_cmpxchg
+#define raw_sync_try_cmpxchg arch_sync_try_cmpxchg
+#else
+#define raw_sync_try_cmpxchg(_ptr, _oldp, _new) \
+({ \
+ typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
+ ___r = raw_sync_cmpxchg((_ptr), ___o, (_new)); \
+ if (unlikely(___r != ___o)) \
+ *___op = ___r; \
+ likely(___r == ___o); \
+})
+#endif
+
/**
* raw_atomic_read() - atomic load with relaxed ordering
* @v: pointer to atomic_t
@@ -4649,4 +4662,4 @@ raw_atomic64_dec_if_positive(atomic64_t *v)
}
#endif /* _LINUX_ATOMIC_FALLBACK_H */
-// 2fdd6702823fa842f9cea57a002e6e4476ae780c
+// eec048affea735b8464f58e6d96992101f8f85f1
diff --git a/include/linux/atomic/atomic-instrumented.h b/include/linux/atomic/atomic-instrumented.h
index d401b406ef7c..54d7bbe0aeaa 100644
--- a/include/linux/atomic/atomic-instrumented.h
+++ b/include/linux/atomic/atomic-instrumented.h
@@ -4998,6 +4998,14 @@ atomic_long_dec_if_positive(atomic_long_t *v)
raw_try_cmpxchg128_local(__ai_ptr, __ai_oldp, __VA_ARGS__); \
})
+#define sync_try_cmpxchg(ptr, ...) \
+({ \
+ typeof(ptr) __ai_ptr = (ptr); \
+ kcsan_mb(); \
+ instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
+ raw_sync_try_cmpxchg(__ai_ptr, __VA_ARGS__); \
+})
+
#endif /* _LINUX_ATOMIC_INSTRUMENTED_H */
-// 1568f875fef72097413caab8339120c065a39aa4
+// 2cc4bc990fef44d3836ec108f11b610f3f438184
diff --git a/scripts/atomic/gen-atomic-fallback.sh b/scripts/atomic/gen-atomic-fallback.sh
index a45154cefa48..f80d69cfeb1f 100755
--- a/scripts/atomic/gen-atomic-fallback.sh
+++ b/scripts/atomic/gen-atomic-fallback.sh
@@ -223,14 +223,15 @@ gen_xchg_fallbacks()
gen_try_cmpxchg_fallback()
{
+ local prefix="$1"; shift
local cmpxchg="$1"; shift;
- local order="$1"; shift;
+ local suffix="$1"; shift;
cat <<EOF
-#define raw_try_${cmpxchg}${order}(_ptr, _oldp, _new) \\
+#define raw_${prefix}try_${cmpxchg}${suffix}(_ptr, _oldp, _new) \\
({ \\
typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \\
- ___r = raw_${cmpxchg}${order}((_ptr), ___o, (_new)); \\
+ ___r = raw_${prefix}${cmpxchg}${suffix}((_ptr), ___o, (_new)); \\
if (unlikely(___r != ___o)) \\
*___op = ___r; \\
likely(___r == ___o); \\
@@ -259,11 +260,11 @@ gen_try_cmpxchg_order_fallback()
fi
printf "#else\n"
- gen_try_cmpxchg_fallback "${cmpxchg}" "${order}"
+ gen_try_cmpxchg_fallback "" "${cmpxchg}" "${order}"
printf "#endif\n\n"
}
-gen_try_cmpxchg_fallbacks()
+gen_try_cmpxchg_order_fallbacks()
{
local cmpxchg="$1"; shift;
@@ -272,15 +273,17 @@ gen_try_cmpxchg_fallbacks()
done
}
-gen_cmpxchg_local_fallbacks()
+gen_def_and_try_cmpxchg_fallback()
{
+ local prefix="$1"; shift
local cmpxchg="$1"; shift
+ local suffix="$1"; shift
- printf "#define raw_${cmpxchg} arch_${cmpxchg}\n\n"
- printf "#ifdef arch_try_${cmpxchg}\n"
- printf "#define raw_try_${cmpxchg} arch_try_${cmpxchg}\n"
+ printf "#define raw_${prefix}${cmpxchg}${suffix} arch_${prefix}${cmpxchg}${suffix}\n\n"
+ printf "#ifdef arch_${prefix}try_${cmpxchg}${suffix}\n"
+ printf "#define raw_${prefix}try_${cmpxchg}${suffix} arch_${prefix}try_${cmpxchg}${suffix}\n"
printf "#else\n"
- gen_try_cmpxchg_fallback "${cmpxchg}" ""
+ gen_try_cmpxchg_fallback "${prefix}" "${cmpxchg}" "${suffix}"
printf "#endif\n\n"
}
@@ -302,15 +305,15 @@ for xchg in "xchg" "cmpxchg" "cmpxchg64" "cmpxchg128"; do
done
for cmpxchg in "cmpxchg" "cmpxchg64" "cmpxchg128"; do
- gen_try_cmpxchg_fallbacks "${cmpxchg}"
+ gen_try_cmpxchg_order_fallbacks "${cmpxchg}"
done
-for cmpxchg in "cmpxchg_local" "cmpxchg64_local" "cmpxchg128_local"; do
- gen_cmpxchg_local_fallbacks "${cmpxchg}" ""
+for cmpxchg in "cmpxchg" "cmpxchg64" "cmpxchg128"; do
+ gen_def_and_try_cmpxchg_fallback "" "${cmpxchg}" "_local"
done
-for cmpxchg in "sync_cmpxchg"; do
- printf "#define raw_${cmpxchg} arch_${cmpxchg}\n\n"
+for cmpxchg in "cmpxchg"; do
+ gen_def_and_try_cmpxchg_fallback "sync_" "${cmpxchg}" ""
done
grep '^[a-z]' "$1" | while read name meta args; do
diff --git a/scripts/atomic/gen-atomic-instrumented.sh b/scripts/atomic/gen-atomic-instrumented.sh
index 8f8f8e3b20f9..592f3ec89b5f 100755
--- a/scripts/atomic/gen-atomic-instrumented.sh
+++ b/scripts/atomic/gen-atomic-instrumented.sh
@@ -169,7 +169,8 @@ for xchg in "xchg" "cmpxchg" "cmpxchg64" "cmpxchg128" "try_cmpxchg" "try_cmpxchg
done
done
-for xchg in "cmpxchg_local" "cmpxchg64_local" "cmpxchg128_local" "sync_cmpxchg" "try_cmpxchg_local" "try_cmpxchg64_local" "try_cmpxchg128_local"; do
+for xchg in "cmpxchg_local" "cmpxchg64_local" "cmpxchg128_local" "sync_cmpxchg" \
+ "try_cmpxchg_local" "try_cmpxchg64_local" "try_cmpxchg128_local" "sync_try_cmpxchg"; do
gen_xchg "${xchg}" ""
printf "\n"
done
--
2.41.0
Implement target specific support for sync_try_cmpxchg.
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Signed-off-by: Uros Bizjak <[email protected]>
---
arch/x86/include/asm/cmpxchg.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
index d53636506134..5612648b0202 100644
--- a/arch/x86/include/asm/cmpxchg.h
+++ b/arch/x86/include/asm/cmpxchg.h
@@ -221,12 +221,18 @@ extern void __add_wrong_size(void)
#define __try_cmpxchg(ptr, pold, new, size) \
__raw_try_cmpxchg((ptr), (pold), (new), (size), LOCK_PREFIX)
+#define __sync_try_cmpxchg(ptr, pold, new, size) \
+ __raw_try_cmpxchg((ptr), (pold), (new), (size), "lock; ")
+
#define __try_cmpxchg_local(ptr, pold, new, size) \
__raw_try_cmpxchg((ptr), (pold), (new), (size), "")
#define arch_try_cmpxchg(ptr, pold, new) \
__try_cmpxchg((ptr), (pold), (new), sizeof(*(ptr)))
+#define arch_sync_try_cmpxchg(ptr, pold, new) \
+ __sync_try_cmpxchg((ptr), (pold), (new), sizeof(*(ptr)))
+
#define arch_try_cmpxchg_local(ptr, pold, new) \
__try_cmpxchg_local((ptr), (pold), (new), sizeof(*(ptr)))
--
2.41.0
* Uros Bizjak <[email protected]> wrote:
> Implement target specific support for sync_try_cmpxchg.
Could you please provide a before/after description of how
this improves things exactly?
Thanks,
Ingo
On Thu, Sep 28, 2023 at 10:53 PM Ingo Molnar <[email protected]> wrote:
>
>
> * Uros Bizjak <[email protected]> wrote:
>
> > Implement target specific support for sync_try_cmpxchg.
>
> Could you please provide a before/after description of how
> this improves things exactly?
The improvement [1] was demonstrated in the original patch submission.
The resent part of the patch series is an infrastructure part, which
introduces a new locking primitive, together with its fallback, so it
has to be committed first. The improvement is in the other part of the
kernel, and this part requires the infrastructure part.
To avoid unwanted patch dependencies, I propose to commit the
infrastructure part first, and after that commit the XEN part of the
series. (Please note that the XEN part is already acked by the
maintainer). I will mention the concrete improvement (code
simplification as shown in [1] and better generated assembly for x86
targets) in the formal submission of the XEN part.
Please also note checksums in include/linux/atomic header files. It is
necessary to regenerate the headers every time the checksum changes.
The resend of the infrastructure part was thus necessary due to the
recent fixes in this area.
[1] https://lore.kernel.org/lkml/[email protected]/
Thanks,
Uros.
* Uros Bizjak <[email protected]> wrote:
> On Thu, Sep 28, 2023 at 10:53 PM Ingo Molnar <[email protected]> wrote:
> >
> >
> > * Uros Bizjak <[email protected]> wrote:
> >
> > > Implement target specific support for sync_try_cmpxchg.
> >
> > Could you please provide a before/after description of how
> > this improves things exactly?
>
> The improvement [1] was demonstrated in the original patch submission.
What I'm saying: please integrate the required context & arguments into the
changelogs of the patches you submit.
Patches that change code generation should demonstrate what they achieve.
- If existing code changes, then describe/demonstrate it with disassembly.
- If existing code generation is unchanged, then *declare that property in
the changelog*, and mention that a future patch relies those changes.
You can either include that future patch in this series, or you can
describe/demonstrate the benefits in the changelog while noting that those
changes will come in future patches.
Your submission, as-is, provided no context whatsoever, it described only
the 'how', not the 'why'.
Thanks,
Ingo