[RFC][PATCH 0/5] refcount: Improve code-gen

[RFC][PATCH 0/5] refcount: Improve code-gen

[Peter Zijlstra]

Hi,

Improves the refcount_t code-gen; I've still got to go through the latest thing
Linus suggested, but figured I should get these patches out to see if there's
other concerns etc..

[RFC][PATCH 1/5] atomic: Introduce atomic_{inc,dec,dec_and_test}_ofl()

[Peter Zijlstra]

In order to facilitate architecture support for refcount_t, introduce
a number of new atomic primitives that have a uaccess style exception
for overflow.

Notably:

  atomic_inc_ofl(v, Label) -- increment and goto Label when
			      v is zero or negative.

  atomic_dec_ofl(v, Label) -- decrement and goto Label when
			      the result is zero or negative

  atomic_dec_and_test_ofl(v, Label) -- decrement and return true when
				       the result is zero and goto Label
				       when the result is negative

Since the GCC 'Labels as Values' extention doesn't allow having the
goto in an inline function, these new 'functions' must in fact be
implemented as macro magic.

This meant extending the atomic generation scripts to deal with
wrapping macros instead of inline functions. Since
xchg/cmpxchg/try_cmpxchg were already macro magic, there was existant
code for that. While extending/improving that a few latent
'instrumentation' bugs were uncovered and 'accidentally' fixed.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/atomic/atomic-arch-fallback.h |   64 ++++++++++++
 include/linux/atomic/atomic-instrumented.h  |  139 ++++++++++++++++++++--------
 include/linux/atomic/atomic-long.h          |   32 ++++++
 scripts/atomic/atomic-tbl.sh                |    6 +
 scripts/atomic/atomics.tbl                  |    6 +
 scripts/atomic/fallbacks/dec_and_test_ofl   |   12 ++
 scripts/atomic/fallbacks/dec_ofl            |    8 +
 scripts/atomic/fallbacks/inc_ofl            |    8 +
 scripts/atomic/gen-atomic-fallback.sh       |    4 
 scripts/atomic/gen-atomic-instrumented.sh   |  117 +++++++++++++++++++----
 scripts/atomic/gen-atomic-long.sh           |   32 +++++-
 11 files changed, 359 insertions(+), 69 deletions(-)

--- a/include/linux/atomic/atomic-arch-fallback.h
+++ b/include/linux/atomic/atomic-arch-fallback.h
@@ -1250,6 +1250,37 @@ arch_atomic_dec_if_positive(atomic_t *v)
 #define arch_atomic_dec_if_positive arch_atomic_dec_if_positive
 #endif
 
+#ifndef arch_atomic_inc_ofl
+#define arch_atomic_inc_ofl(_v, _label)				\
+do {									\
+	int __old = arch_atomic_fetch_inc(_v);			\
+	if (unlikely(__old <= 0))					\
+		goto _label;						\
+} while (0)
+#endif
+
+#ifndef arch_atomic_dec_ofl
+#define arch_atomic_dec_ofl(_v, _label)				\
+do {									\
+	int __new = arch_atomic_dec_return(_v);			\
+	if (unlikely(__new <= 0))					\
+		goto _label;						\
+} while (0)
+#endif
+
+#ifndef arch_atomic_dec_and_test_ofl
+#define arch_atomic_dec_and_test_ofl(_v, _label)			\
+({									\
+	bool __ret = false;						\
+	int __new = arch_atomic_dec_return(_v);			\
+	if (unlikely(__new < 0))					\
+		goto _label;						\
+	if (unlikely(__new == 0))					\
+		__ret = true;						\
+	__ret;								\
+})
+#endif
+
 #ifdef CONFIG_GENERIC_ATOMIC64
 #include <asm-generic/atomic64.h>
 #endif
@@ -2357,5 +2388,36 @@ arch_atomic64_dec_if_positive(atomic64_t
 #define arch_atomic64_dec_if_positive arch_atomic64_dec_if_positive
 #endif
 
+#ifndef arch_atomic64_inc_ofl
+#define arch_atomic64_inc_ofl(_v, _label)				\
+do {									\
+	s64 __old = arch_atomic64_fetch_inc(_v);			\
+	if (unlikely(__old <= 0))					\
+		goto _label;						\
+} while (0)
+#endif
+
+#ifndef arch_atomic64_dec_ofl
+#define arch_atomic64_dec_ofl(_v, _label)				\
+do {									\
+	s64 __new = arch_atomic64_dec_return(_v);			\
+	if (unlikely(__new <= 0))					\
+		goto _label;						\
+} while (0)
+#endif
+
+#ifndef arch_atomic64_dec_and_test_ofl
+#define arch_atomic64_dec_and_test_ofl(_v, _label)			\
+({									\
+	bool __ret = false;						\
+	s64 __new = arch_atomic64_dec_return(_v);			\
+	if (unlikely(__new < 0))					\
+		goto _label;						\
+	if (unlikely(__new == 0))					\
+		__ret = true;						\
+	__ret;								\
+})
+#endif
+
 #endif /* _LINUX_ATOMIC_FALLBACK_H */
-// cca554917d7ea73d5e3e7397dd70c484cad9b2c4
+// a59904b14db62a38bbab8699edc4a785a97871fb
--- a/include/linux/atomic/atomic-instrumented.h
+++ b/include/linux/atomic/atomic-instrumented.h
@@ -501,7 +501,7 @@ static __always_inline bool
 atomic_try_cmpxchg(atomic_t *v, int *old, int new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic_try_cmpxchg(v, old, new);
 }
 
@@ -509,7 +509,7 @@ static __always_inline bool
 atomic_try_cmpxchg_acquire(atomic_t *v, int *old, int new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic_try_cmpxchg_acquire(v, old, new);
 }
 
@@ -517,7 +517,7 @@ static __always_inline bool
 atomic_try_cmpxchg_release(atomic_t *v, int *old, int new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic_try_cmpxchg_release(v, old, new);
 }
 
@@ -525,7 +525,7 @@ static __always_inline bool
 atomic_try_cmpxchg_relaxed(atomic_t *v, int *old, int new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic_try_cmpxchg_relaxed(v, old, new);
 }
 
@@ -599,6 +599,27 @@ atomic_dec_if_positive(atomic_t *v)
 	return arch_atomic_dec_if_positive(v);
 }
 
+#define atomic_inc_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic_inc_ofl(__ai_v, L); \
+})
+
+#define atomic_dec_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic_dec_ofl(__ai_v, L); \
+})
+
+#define atomic_dec_and_test_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic_dec_and_test_ofl(__ai_v, L); \
+})
+
 static __always_inline s64
 atomic64_read(const atomic64_t *v)
 {
@@ -1079,7 +1100,7 @@ static __always_inline bool
 atomic64_try_cmpxchg(atomic64_t *v, s64 *old, s64 new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic64_try_cmpxchg(v, old, new);
 }
 
@@ -1087,7 +1108,7 @@ static __always_inline bool
 atomic64_try_cmpxchg_acquire(atomic64_t *v, s64 *old, s64 new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic64_try_cmpxchg_acquire(v, old, new);
 }
 
@@ -1095,7 +1116,7 @@ static __always_inline bool
 atomic64_try_cmpxchg_release(atomic64_t *v, s64 *old, s64 new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic64_try_cmpxchg_release(v, old, new);
 }
 
@@ -1103,7 +1124,7 @@ static __always_inline bool
 atomic64_try_cmpxchg_relaxed(atomic64_t *v, s64 *old, s64 new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic64_try_cmpxchg_relaxed(v, old, new);
 }
 
@@ -1177,6 +1198,27 @@ atomic64_dec_if_positive(atomic64_t *v)
 	return arch_atomic64_dec_if_positive(v);
 }
 
+#define atomic64_inc_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic64_inc_ofl(__ai_v, L); \
+})
+
+#define atomic64_dec_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic64_dec_ofl(__ai_v, L); \
+})
+
+#define atomic64_dec_and_test_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic64_dec_and_test_ofl(__ai_v, L); \
+})
+
 static __always_inline long
 atomic_long_read(const atomic_long_t *v)
 {
@@ -1657,7 +1699,7 @@ static __always_inline bool
 atomic_long_try_cmpxchg(atomic_long_t *v, long *old, long new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic_long_try_cmpxchg(v, old, new);
 }
 
@@ -1665,7 +1707,7 @@ static __always_inline bool
 atomic_long_try_cmpxchg_acquire(atomic_long_t *v, long *old, long new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic_long_try_cmpxchg_acquire(v, old, new);
 }
 
@@ -1673,7 +1715,7 @@ static __always_inline bool
 atomic_long_try_cmpxchg_release(atomic_long_t *v, long *old, long new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic_long_try_cmpxchg_release(v, old, new);
 }
 
@@ -1681,7 +1723,7 @@ static __always_inline bool
 atomic_long_try_cmpxchg_relaxed(atomic_long_t *v, long *old, long new)
 {
 	instrument_atomic_read_write(v, sizeof(*v));
-	instrument_atomic_read_write(old, sizeof(*old));
+	instrument_read_write(old, sizeof(*old));
 	return arch_atomic_long_try_cmpxchg_relaxed(v, old, new);
 }
 
@@ -1755,87 +1797,108 @@ atomic_long_dec_if_positive(atomic_long_
 	return arch_atomic_long_dec_if_positive(v);
 }
 
+#define atomic_long_inc_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic_long_inc_ofl(__ai_v, L); \
+})
+
+#define atomic_long_dec_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic_long_dec_ofl(__ai_v, L); \
+})
+
+#define atomic_long_dec_and_test_ofl(v, L) \
+({ \
+	typeof(v) __ai_v = (v); \
+	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
+	arch_atomic_long_dec_and_test_ofl(__ai_v, L); \
+})
+
 #define xchg(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_xchg(__ai_ptr, __VA_ARGS__); \
 })
 
 #define xchg_acquire(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_xchg_acquire(__ai_ptr, __VA_ARGS__); \
 })
 
 #define xchg_release(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_xchg_release(__ai_ptr, __VA_ARGS__); \
 })
 
 #define xchg_relaxed(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_xchg_relaxed(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg_acquire(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg_acquire(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg_release(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg_release(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg_relaxed(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg_relaxed(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg64(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg64(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg64_acquire(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg64_acquire(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg64_release(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg64_release(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg64_relaxed(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg64_relaxed(__ai_ptr, __VA_ARGS__); \
 })
 
@@ -1843,8 +1906,8 @@ atomic_long_dec_if_positive(atomic_long_
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
 	typeof(oldp) __ai_oldp = (oldp); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
-	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_read_write(__ai_oldp, sizeof(*__ai_oldp)); \
 	arch_try_cmpxchg(__ai_ptr, __ai_oldp, __VA_ARGS__); \
 })
 
@@ -1852,8 +1915,8 @@ atomic_long_dec_if_positive(atomic_long_
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
 	typeof(oldp) __ai_oldp = (oldp); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
-	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_read_write(__ai_oldp, sizeof(*__ai_oldp)); \
 	arch_try_cmpxchg_acquire(__ai_ptr, __ai_oldp, __VA_ARGS__); \
 })
 
@@ -1861,8 +1924,8 @@ atomic_long_dec_if_positive(atomic_long_
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
 	typeof(oldp) __ai_oldp = (oldp); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
-	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_read_write(__ai_oldp, sizeof(*__ai_oldp)); \
 	arch_try_cmpxchg_release(__ai_ptr, __ai_oldp, __VA_ARGS__); \
 })
 
@@ -1870,36 +1933,36 @@ atomic_long_dec_if_positive(atomic_long_
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
 	typeof(oldp) __ai_oldp = (oldp); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
-	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_read_write(__ai_oldp, sizeof(*__ai_oldp)); \
 	arch_try_cmpxchg_relaxed(__ai_ptr, __ai_oldp, __VA_ARGS__); \
 })
 
 #define cmpxchg_local(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg_local(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg64_local(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg64_local(__ai_ptr, __VA_ARGS__); \
 })
 
 #define sync_cmpxchg(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_sync_cmpxchg(__ai_ptr, __VA_ARGS__); \
 })
 
 #define cmpxchg_double(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, 2 * sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg_double(__ai_ptr, __VA_ARGS__); \
 })
 
@@ -1907,9 +1970,9 @@ atomic_long_dec_if_positive(atomic_long_
 #define cmpxchg_double_local(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
-	instrument_atomic_write(__ai_ptr, 2 * sizeof(*__ai_ptr)); \
+	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
 	arch_cmpxchg_double_local(__ai_ptr, __VA_ARGS__); \
 })
 
 #endif /* _LINUX_ATOMIC_INSTRUMENTED_H */
-// 2a9553f0a9d5619f19151092df5cabbbf16ce835
+// 214f9a7e972966a9a8e28e1568665cfb75decf91
--- a/include/linux/atomic/atomic-long.h
+++ b/include/linux/atomic/atomic-long.h
@@ -515,6 +515,21 @@ arch_atomic_long_dec_if_positive(atomic_
 	return arch_atomic64_dec_if_positive(v);
 }
 
+#define arch_atomic_long_inc_ofl(v, L) \
+({ \
+	arch_atomic64_inc_ofl((v), L) \
+})
+
+#define arch_atomic_long_dec_ofl(v, L) \
+({ \
+	arch_atomic64_dec_ofl((v), L) \
+})
+
+#define arch_atomic_long_dec_and_test_ofl(v, L) \
+({ \
+	arch_atomic64_dec_and_test_ofl((v), L) \
+})
+
 #else /* CONFIG_64BIT */
 
 static __always_inline long
@@ -1009,6 +1024,21 @@ arch_atomic_long_dec_if_positive(atomic_
 	return arch_atomic_dec_if_positive(v);
 }
 
+#define arch_atomic_long_inc_ofl(v, L) \
+({ \
+	arch_atomic_inc_ofl((v), L) \
+})
+
+#define arch_atomic_long_dec_ofl(v, L) \
+({ \
+	arch_atomic_dec_ofl((v), L) \
+})
+
+#define arch_atomic_long_dec_and_test_ofl(v, L) \
+({ \
+	arch_atomic_dec_and_test_ofl((v), L) \
+})
+
 #endif /* CONFIG_64BIT */
 #endif /* _LINUX_ATOMIC_LONG_H */
-// e8f0e08ff072b74d180eabe2ad001282b38c2c88
+// 120f718985fa4c8f0e884cc4f23db8aa950255fb
--- a/scripts/atomic/atomic-tbl.sh
+++ b/scripts/atomic/atomic-tbl.sh
@@ -36,6 +36,12 @@ meta_has_relaxed()
 	meta_in "$1" "BFIR"
 }
 
+#meta_has_macro(meta)
+meta_has_macro()
+{
+	meta_in "$1" "m"
+}
+
 #find_fallback_template(pfx, name, sfx, order)
 find_fallback_template()
 {
--- a/scripts/atomic/atomics.tbl
+++ b/scripts/atomic/atomics.tbl
@@ -10,12 +10,15 @@
 # * F/f	- fetch: returns base type (has fetch_ variants)
 # * l	- load: returns base type (has _acquire order variant)
 # * s	- store: returns void (has _release order variant)
+# * m	- macro:
 #
 # Where args contains list of type[:name], where type is:
 # * cv	- const pointer to atomic base type (atomic_t/atomic64_t/atomic_long_t)
 # * v	- pointer to atomic base type (atomic_t/atomic64_t/atomic_long_t)
 # * i	- base type (int/s64/long)
 # * p	- pointer to base type (int/s64/long)
+# * L	- label for exception case
+# * V:... - vararg
 #
 read			l	cv
 set			s	v	i
@@ -39,3 +42,6 @@ inc_not_zero		b	v
 inc_unless_negative	b	v
 dec_unless_positive	b	v
 dec_if_positive		i	v
+inc_ofl			m	v	L
+dec_ofl			m	v	L
+dec_and_test_ofl	m	v	L
--- /dev/null
+++ b/scripts/atomic/fallbacks/dec_and_test_ofl
@@ -0,0 +1,12 @@
+cat << EOF
+#define arch_${atomic}_dec_and_test_ofl(_v, _label)			\\
+({									\\
+	bool __ret = false;						\\
+	${int} __new = arch_${atomic}_dec_return(_v);			\\
+	if (unlikely(__new < 0))					\\
+		goto _label;						\\
+	if (unlikely(__new == 0))					\\
+		__ret = true;						\\
+	__ret;								\\
+})
+EOF
--- /dev/null
+++ b/scripts/atomic/fallbacks/dec_ofl
@@ -0,0 +1,8 @@
+cat << EOF
+#define arch_${atomic}_dec_ofl(_v, _label)				\\
+do {									\\
+	${int} __new = arch_${atomic}_dec_return(_v);			\\
+	if (unlikely(__new <= 0))					\\
+		goto _label;						\\
+} while (0)
+EOF
--- /dev/null
+++ b/scripts/atomic/fallbacks/inc_ofl
@@ -0,0 +1,8 @@
+cat << EOF
+#define arch_${atomic}_inc_ofl(_v, _label)				\\
+do {									\\
+	${int} __old = arch_${atomic}_fetch_inc(_v);			\\
+	if (unlikely(__old <= 0))					\\
+		goto _label;						\\
+} while (0)
+EOF
--- a/scripts/atomic/gen-atomic-fallback.sh
+++ b/scripts/atomic/gen-atomic-fallback.sh
@@ -27,7 +27,9 @@ gen_template_fallback()
 	if [ ! -z "${template}" ]; then
 		printf "#ifndef ${atomicname}\n"
 		. ${template}
-		printf "#define ${atomicname} ${atomicname}\n"
+		if ! meta_has_macro "${meta}"; then
+			printf "#define ${atomicname} ${atomicname}\n"
+		fi
 		printf "#endif\n\n"
 	fi
 }
--- a/scripts/atomic/gen-atomic-instrumented.sh
+++ b/scripts/atomic/gen-atomic-instrumented.sh
@@ -13,9 +13,13 @@ gen_param_check()
 	local type="${arg%%:*}"
 	local name="$(gen_param_name "${arg}")"
 	local rw="write"
+	local pfx;
 
 	case "${type#c}" in
+	v) pfx="atomic_";;
 	i) return;;
+	L) return;;
+	V) return;;
 	esac
 
 	if [ ${type#c} != ${type} ]; then
@@ -27,7 +31,16 @@ gen_param_check()
 		rw="read_write"
 	fi
 
-	printf "\tinstrument_atomic_${rw}(${name}, sizeof(*${name}));\n"
+	if meta_has_macro "${meta}"; then
+		name="__ai_${name}"
+	fi
+
+	printf "\tinstrument_${pfx}${rw}(${name}, sizeof(*${name}));"
+	if meta_has_macro "${meta}"; then
+		printf " \\"
+	fi
+	printf "\n"
+
 }
 
 #gen_params_checks(meta, arg...)
@@ -41,6 +54,52 @@ gen_params_checks()
 	done
 }
 
+#gen_var(arg)
+gen_var()
+{
+	local type="${1%%:*}"
+	local name="$(gen_param_name "$1")"
+
+	case "${type#c}" in
+	L) return;;
+	V) return;;
+	esac
+
+	printf "\ttypeof(${name}) __ai_${name} = (${name}); \\\\\n";
+}
+
+#gen_vars(arg...)
+gen_vars()
+{
+	while [ "$#" -gt 0 ]; do
+		gen_var "$1"
+		shift
+	done
+}
+
+#gen_varg(arg)
+gen_varg()
+{
+	local type="${1%%:*}"
+	local name="$(gen_param_name "$1")"
+
+	case "${type#c}" in
+	L)	printf "${name}";;
+	V)	printf "__VA_ARGS__";;
+	*)	printf "__ai_${name}";;
+	esac
+}
+
+#gen_vargs(arg...)
+gen_vargs()
+{
+	while [ "$#" -gt 0 ]; do
+		printf "$(gen_varg "$1")"
+		[ "$#" -gt 1 ] && printf ", "
+		shift
+	done
+}
+
 #gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, int, arg...)
 gen_proto_order_variant()
 {
@@ -54,11 +113,28 @@ gen_proto_order_variant()
 
 	local atomicname="${atomic}_${pfx}${name}${sfx}${order}"
 
-	local ret="$(gen_ret_type "${meta}" "${int}")"
-	local params="$(gen_params "${int}" "${atomic}" "$@")"
 	local checks="$(gen_params_checks "${meta}" "$@")"
 	local args="$(gen_args "$@")"
-	local retstmt="$(gen_ret_stmt "${meta}")"
+
+	if meta_has_macro "${meta}"; then
+
+		local vars="$(gen_vars "$@")"
+		local vargs="$(gen_vargs "$@")"
+
+cat <<EOF
+#define ${atomicname}(${args}) \\
+({ \\
+${vars}
+${checks}
+	arch_${atomicname}(${vargs}); \\
+})
+EOF
+
+	else
+
+		local ret="$(gen_ret_type "${meta}" "${int}")"
+		local params="$(gen_params "${int}" "${atomic}" "$@")"
+		local retstmt="$(gen_ret_stmt "${meta}")"
 
 cat <<EOF
 static __always_inline ${ret}
@@ -69,6 +145,8 @@ ${checks}
 }
 EOF
 
+	fi
+
 	printf "\n"
 }
 
@@ -76,32 +154,27 @@ gen_xchg()
 {
 	local xchg="$1"; shift
 	local mult="$1"; shift
+	local ARGS;
 
 	if [ "${xchg%${xchg#try_cmpxchg}}" = "try_cmpxchg" ] ; then
-
-cat <<EOF
-#define ${xchg}(ptr, oldp, ...) \\
-({ \\
-	typeof(ptr) __ai_ptr = (ptr); \\
-	typeof(oldp) __ai_oldp = (oldp); \\
-	instrument_atomic_write(__ai_ptr, ${mult}sizeof(*__ai_ptr)); \\
-	instrument_atomic_write(__ai_oldp, ${mult}sizeof(*__ai_oldp)); \\
-	arch_${xchg}(__ai_ptr, __ai_oldp, __VA_ARGS__); \\
-})
-EOF
-
+		ARGS="v:ptr p:oldp V:..."
 	else
+		ARGS="v:ptr V:..."
+	fi
+
+	local args="$(gen_args ${ARGS})"
+	local vars="$(gen_vars ${ARGS})"
+	local checks="$(gen_params_checks "m" ${ARGS})"
+	local vargs="$(gen_vargs ${ARGS})"
 
 cat <<EOF
-#define ${xchg}(ptr, ...) \\
+#define ${xchg}(${args}) \\
 ({ \\
-	typeof(ptr) __ai_ptr = (ptr); \\
-	instrument_atomic_write(__ai_ptr, ${mult}sizeof(*__ai_ptr)); \\
-	arch_${xchg}(__ai_ptr, __VA_ARGS__); \\
+${vars}
+${checks}
+	arch_${xchg}(${vargs}); \\
 })
 EOF
-
-	fi
 }
 
 cat << EOF
--- a/scripts/atomic/gen-atomic-long.sh
+++ b/scripts/atomic/gen-atomic-long.sh
@@ -17,16 +17,21 @@ gen_cast()
 	printf "($(gen_param_type "${arg}" "${int}" "${atomic}"))"
 }
 
-#gen_args_cast(int, atomic, arg...)
+#gen_args_cast(meta, int, atomic, arg...)
 gen_args_cast()
 {
+	local meta=$1; shift
 	local int="$1"; shift
 	local atomic="$1"; shift
 
 	while [ "$#" -gt 0 ]; do
 		local cast="$(gen_cast "$1" "${int}" "${atomic}")"
 		local arg="$(gen_param_name "$1")"
-		printf "${cast}${arg}"
+		if meta_has_macro "${meta}" && [ "${1%%:*}" != "L" ]; then
+			printf "${cast}(${arg})"
+		else
+			printf "${cast}${arg}"
+		fi
 		[ "$#" -gt 1 ] && printf ", "
 		shift;
 	done
@@ -40,10 +45,24 @@ gen_proto_order_variant()
 	local atomic="$1"; shift
 	local int="$1"; shift
 
-	local ret="$(gen_ret_type "${meta}" "long")"
-	local params="$(gen_params "long" "atomic_long" "$@")"
-	local argscast="$(gen_args_cast "${int}" "${atomic}" "$@")"
-	local retstmt="$(gen_ret_stmt "${meta}")"
+	local argscast="$(gen_args_cast "${meta}" "${int}" "${atomic}" "$@")"
+
+	if meta_has_macro "${meta}"; then
+
+		local args="$(gen_args "$@")"
+
+cat <<EOF
+#define arch_atomic_long_${name}(${args}) \\
+({ \\
+	arch_${atomic}_${name}(${argscast}) \\
+})
+
+EOF
+	else
+
+		local ret="$(gen_ret_type "${meta}" "long")"
+		local params="$(gen_params "long" "atomic_long" "$@")"
+		local retstmt="$(gen_ret_stmt "${meta}")"
 
 cat <<EOF
 static __always_inline ${ret}
@@ -53,6 +72,7 @@ arch_atomic_long_${name}(${params})
 }
 
 EOF
+	fi
 }
 
 cat << EOF

[RFC][PATCH 2/5] refcount: Use atomic_*_ofl()

[Peter Zijlstra]

Use the shiny new atomic_*_ofl() functions in order to have better
code-gen.

Notably refcount_inc() case no longer distinguishes between
inc-from-zero and inc-negative in the fast path, this improves
code-gen:

    4b88:       b8 01 00 00 00          mov    $0x1,%eax
    4b8d:       f0 0f c1 43 28          lock xadd %eax,0x28(%rbx)
    4b92:       85 c0                   test   %eax,%eax
    4b94:       74 1b                   je     4bb1 <alloc_perf_context+0xf1>
    4b96:       8d 50 01                lea    0x1(%rax),%edx
    4b99:       09 c2                   or     %eax,%edx
    4b9b:       78 20                   js     4bbd <alloc_perf_context+0xfd>

to:

    4768:       b8 01 00 00 00          mov    $0x1,%eax
    476d:       f0 0f c1 43 28          lock xadd %eax,0x28(%rbx)
    4772:       85 c0                   test   %eax,%eax
    4774:       7e 14                   jle    478a <alloc_perf_context+0xea>


without sacrificing on functionality; the only thing that suffers is
the reported error condition, which might now 'spuriously' report
'saturated' instead of 'inc-from-zero'.

refcount_dec_and_test() is also improved:

    aa40:       b8 ff ff ff ff          mov    $0xffffffff,%eax
    aa45:       f0 0f c1 07             lock xadd %eax,(%rdi)
    aa49:       83 f8 01                cmp    $0x1,%eax
    aa4c:       74 05                   je     aa53 <ring_buffer_put+0x13>
    aa4e:       85 c0                   test   %eax,%eax
    aa50:       7e 1e                   jle    aa70 <ring_buffer_put+0x30>
    aa52:       c3                      ret

to:

    a980:       b8 ff ff ff ff          mov    $0xffffffff,%eax
    a985:       f0 0f c1 07             lock xadd %eax,(%rdi)
    a989:       83 e8 01                sub    $0x1,%eax
    a98c:       78 20                   js     a9ae <ring_buffer_put+0x2e>
    a98e:       74 01                   je     a991 <ring_buffer_put+0x11>
    a990:       c3                      ret

XXX atomic_dec_and_test_ofl() is strictly stronger ordered than
refcount_dec_and_test() is defined -- Power and Arrghh64 ?

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/refcount.h |   15 ++++++++++++---
 lib/refcount.c           |    5 +++++
 2 files changed, 17 insertions(+), 3 deletions(-)

--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -264,7 +264,10 @@ static inline void __refcount_inc(refcou
  */
 static inline void refcount_inc(refcount_t *r)
 {
-	__refcount_inc(r, NULL);
+	atomic_inc_ofl(&r->refs, Eoverflow);
+	return;
+Eoverflow:
+	refcount_warn_saturate(r, REFCOUNT_ADD_OVF);
 }
 
 static inline __must_check bool __refcount_sub_and_test(int i, refcount_t *r, int *oldp)
@@ -330,7 +333,10 @@ static inline __must_check bool __refcou
  */
 static inline __must_check bool refcount_dec_and_test(refcount_t *r)
 {
-	return __refcount_dec_and_test(r, NULL);
+	return atomic_dec_and_test_ofl(&r->refs, Eoverflow);
+Eoverflow:
+	refcount_warn_saturate(r, REFCOUNT_SUB_UAF);
+	return false;
 }
 
 static inline void __refcount_dec(refcount_t *r, int *oldp)
@@ -356,7 +362,10 @@ static inline void __refcount_dec(refcou
  */
 static inline void refcount_dec(refcount_t *r)
 {
-	__refcount_dec(r, NULL);
+	atomic_dec_ofl(&r->refs, Eoverflow);
+	return;
+Eoverflow:
+	refcount_warn_saturate(r, REFCOUNT_DEC_LEAK);
 }
 
 extern __must_check bool refcount_dec_if_one(refcount_t *r);
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -12,8 +12,13 @@
 
 void refcount_warn_saturate(refcount_t *r, enum refcount_saturation_type t)
 {
+	int old = refcount_read(r);
 	refcount_set(r, REFCOUNT_SATURATED);
 
+	/* racy; who cares */
+	if (old == 1 && t == REFCOUNT_ADD_OVF)
+		t = REFCOUNT_ADD_UAF;
+
 	switch (t) {
 	case REFCOUNT_ADD_NOT_ZERO_OVF:
 		REFCOUNT_WARN("saturated; leaking memory");

[RFC][PATCH 3/5] refcount: Improve out-of-line code-gen

[Peter Zijlstra]

Allow a number of ops to tail-call refcount_warn_saturate() in order
to generate smaller out-of-line code.

   text    data     bss     dec     hex filename
  97341    4985    1116  103442   19412 defconfig-build/kernel/events/core.o
  97299    4985    1116  103400   193e8 defconfig-build/kernel/events/core.o

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/refcount.h |    9 ++++-----
 lib/refcount.c           |    4 +++-
 2 files changed, 7 insertions(+), 6 deletions(-)

--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -124,7 +124,7 @@ enum refcount_saturation_type {
 	REFCOUNT_DEC_LEAK,
 };
 
-void refcount_warn_saturate(refcount_t *r, enum refcount_saturation_type t);
+bool refcount_warn_saturate(refcount_t *r, enum refcount_saturation_type t);
 
 /**
  * refcount_set - set a refcount's value
@@ -160,7 +160,7 @@ static inline __must_check bool __refcou
 		*oldp = old;
 
 	if (unlikely(old < 0 || old + i < 0))
-		refcount_warn_saturate(r, REFCOUNT_ADD_NOT_ZERO_OVF);
+		return refcount_warn_saturate(r, REFCOUNT_ADD_NOT_ZERO_OVF);
 
 	return old;
 }
@@ -283,7 +283,7 @@ static inline __must_check bool __refcou
 	}
 
 	if (unlikely(old < 0 || old - i < 0))
-		refcount_warn_saturate(r, REFCOUNT_SUB_UAF);
+		return refcount_warn_saturate(r, REFCOUNT_SUB_UAF);
 
 	return false;
 }
@@ -335,8 +335,7 @@ static inline __must_check bool refcount
 {
 	return atomic_dec_and_test_ofl(&r->refs, Eoverflow);
 Eoverflow:
-	refcount_warn_saturate(r, REFCOUNT_SUB_UAF);
-	return false;
+	return refcount_warn_saturate(r, REFCOUNT_SUB_UAF);
 }
 
 static inline void __refcount_dec(refcount_t *r, int *oldp)
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -10,7 +10,7 @@
 
 #define REFCOUNT_WARN(str)	WARN_ONCE(1, "refcount_t: " str ".\n")
 
-void refcount_warn_saturate(refcount_t *r, enum refcount_saturation_type t)
+bool refcount_warn_saturate(refcount_t *r, enum refcount_saturation_type t)
 {
 	int old = refcount_read(r);
 	refcount_set(r, REFCOUNT_SATURATED);
@@ -38,6 +38,8 @@ void refcount_warn_saturate(refcount_t *
 	default:
 		REFCOUNT_WARN("unknown saturation event!?");
 	}
+
+	return false;
 }
 EXPORT_SYMBOL(refcount_warn_saturate);
 

[RFC][PATCH 4/5] atomic,x86: Implement atomic_dec_and_test_ofl()

[Peter Zijlstra]

Provide a better implementation of atomic_{inc,dec_and_test}_ofl() by
making use of the atomic-op condition codes.

This further improves the fast path code:

    a980:       b8 ff ff ff ff          mov    $0xffffffff,%eax
    a985:       f0 0f c1 07             lock xadd %eax,(%rdi)
    a989:       83 e8 01                sub    $0x1,%eax
    a98c:       78 20                   js     a9ae <ring_buffer_put+0x2e>
    a98e:       74 01                   je     a991 <ring_buffer_put+0x11>
    a990:       c3                      ret

to:

    ab81:       48 89 fb                mov    %rdi,%rbx
    ab84:       f0 ff 0f                lock decl (%rdi)
    ab87:       7c 04                   jl     ab8d <ring_buffer_put+0xd>
    ab89:       74 10                   je     ab9b <ring_buffer_put+0x1b>
    ab8b:       c3                      ret

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/atomic.h |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

--- a/arch/x86/include/asm/atomic.h
+++ b/arch/x86/include/asm/atomic.h
@@ -263,6 +263,29 @@ static __always_inline int arch_atomic_f
 }
 #define arch_atomic_fetch_xor arch_atomic_fetch_xor
 
+#define arch_atomic_dec_ofl(_v, _label)					\
+	asm_volatile_goto(LOCK_PREFIX "decl %[var]\n\t"			\
+			  "jle %l1"					\
+			  : : [var] "m" ((_v)->counter)			\
+			  : "memory"					\
+			  : _label)
+
+#define arch_atomic_dec_and_test_ofl(_v, _label)			\
+({									\
+	__label__ __zero;						\
+	__label__ __out;						\
+	bool __ret = false;						\
+	asm_volatile_goto(LOCK_PREFIX "decl %[var]\n\t"			\
+			  "jl %l2\n\t"					\
+			  "je %l[__zero]"				\
+			  : : [var] "m" ((_v)->counter)			\
+			  : "memory"					\
+			  : __zero, _label);				\
+	goto __out;							\
+__zero:	__ret = true;							\
+__out:	__ret;								\
+})
+
 #ifdef CONFIG_X86_32
 # include <asm/atomic64_32.h>
 #else

[RFC][PATCH 5/5] atomic: Document the atomic_{}_ofl() functions

[Peter Zijlstra]

Them special, them cause confusion, them needing docs for reading.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 Documentation/atomic_t.txt |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

--- a/Documentation/atomic_t.txt
+++ b/Documentation/atomic_t.txt
@@ -44,6 +44,10 @@ The 'full' API consists of (atomic64_ an
   atomic_add_unless(), atomic_inc_not_zero()
   atomic_sub_and_test(), atomic_dec_and_test()
 
+Reference count with overflow (as used by refcount_t):
+
+  atomic_inc_ofl(), atomic_dec_ofl()
+  atomic_dec_and_test_ofl()
 
 Misc:
 
@@ -157,6 +161,22 @@ atomic variable) can be fully ordered an
 visible.
 
 
+Overflow ops:
+
+The atomic_{}_ofl() ops are similar to their !_ofl() bretheren with the
+notable exception that they take a label as their final argument to jump to
+when the atomic op overflows.
+
+Overflow for inc is zero-or-negative on the value prior to increment.
+Overflow for dec is zero-or-negative on the value after the decrement.
+Overflow for dec_and_test is negative on the value after the decrement.
+
+These semantics match the reference count use-case (for which they were
+created). Specifically incrementing from zero is a failure because 0 means the
+object is freed (IOW use-after-free). decrementing to zero is a failure
+because it goes undetected (see dec_and_test) and the object would leak.
+
+
 ORDERING  (go read memory-barriers.txt first)
 --------
 

Re: [RFC][PATCH 2/5] refcount: Use atomic_*_ofl()

[Peter Zijlstra]

On Wed, Dec 08, 2021 at 07:36:57PM +0100, Peter Zijlstra wrote:
> Use the shiny new atomic_*_ofl() functions in order to have better
> code-gen.
> 

*sigh*, so I forgot to adjust the other primitives to the slightly
tweaked range of these new functions..

Re: [RFC][PATCH 2/5] refcount: Use atomic_*_ofl()

[Peter Zijlstra]

On Wed, Dec 08, 2021 at 08:19:35PM +0100, Peter Zijlstra wrote:
> On Wed, Dec 08, 2021 at 07:36:57PM +0100, Peter Zijlstra wrote:
> > Use the shiny new atomic_*_ofl() functions in order to have better
> > code-gen.
> > 
> 
> *sigh*, so I forgot to adjust the other primitives to the slightly
> tweaked range of these new functions..

Looks like it all just works. The add/sub stuff needs to check before
and after anyway, and it saturing slightly earlier isn't a problem.

The only difference I found was, so I'll just fold that.

--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -346,7 +346,7 @@ static inline void __refcount_dec(refcou
 	if (oldp)
 		*oldp = old;
 
-	if (unlikely(old <= 1))
+	if (unlikely(old - 1 <= 0))
 		refcount_warn_saturate(r, REFCOUNT_DEC_LEAK);
 }
 

Re: [RFC][PATCH 0/5] refcount: Improve code-gen

[Peter Zijlstra]

On Wed, Dec 08, 2021 at 07:36:55PM +0100, Peter Zijlstra wrote:
> Hi,
> 
> Improves the refcount_t code-gen; I've still got to go through the latest thing
> Linus suggested, but figured I should get these patches out to see if there's
> other concerns etc..
> 

Bah, I forgot to Cc Jens, let me bounce him the lot.

Re: [RFC][PATCH 3/5] refcount: Improve out-of-line code-gen

[Peter Zijlstra]

On Wed, Dec 08, 2021 at 07:36:58PM +0100, Peter Zijlstra wrote:
> Allow a number of ops to tail-call refcount_warn_saturate() in order
> to generate smaller out-of-line code.
> 
>    text    data     bss     dec     hex filename
>   97341    4985    1116  103442   19412 defconfig-build/kernel/events/core.o
>   97299    4985    1116  103400   193e8 defconfig-build/kernel/events/core.o
> 

This patch also makes GCC do worse code-gen on the fast path, so I'll
drop it.

For some obscure raisin it causes this:

ring_buffer_put:
    a950:       f0 ff 0f                lock decl (%rdi)
    a953:       7c 20                   jl     a975 <ring_buffer_put+0x25>
    a955:       74 01                   je     a958 <ring_buffer_put+0x8>
    a957:       c3                      ret


ring_buffer_put:
    a940:       53                      push   %rbx
    a941:       48 89 fb                mov    %rdi,%rbx
    a944:       f0 ff 0f                lock decl (%rdi)
    a947:       7c 04                   jl     a94d <ring_buffer_put+0xd>
    a949:       74 10                   je     a95b <ring_buffer_put+0x1b>
    a94b:       5b                      pop    %rbx
    a94c:       c3                      ret

Which is just unacceptible...

Re: [RFC][PATCH 1/5] atomic: Introduce atomic_{inc,dec,dec_and_test}_ofl()

[Mark Rutland]

On Wed, Dec 08, 2021 at 07:36:56PM +0100, Peter Zijlstra wrote:
> In order to facilitate architecture support for refcount_t, introduce
> a number of new atomic primitives that have a uaccess style exception
> for overflow.
> 
> Notably:
> 
>   atomic_inc_ofl(v, Label) -- increment and goto Label when
> 			      v is zero or negative.
> 
>   atomic_dec_ofl(v, Label) -- decrement and goto Label when
> 			      the result is zero or negative
> 
>   atomic_dec_and_test_ofl(v, Label) -- decrement and return true when
> 				       the result is zero and goto Label
> 				       when the result is negative

Just to check, atomic_inc_ofl() tests the *old* value of `v`, and the other
cases check the *new* value of `v`?

For clarity, in the descriptions it might be worth:

  s/v/the old value of v/
  s/the result/the new value of v/

... which I think makes that clearer.

> Since the GCC 'Labels as Values' extention doesn't allow having the
> goto in an inline function, these new 'functions' must in fact be
> implemented as macro magic.

Oh; fun... :(

GCC allows the label to be passed into in a *nested* function, so I had a play
with that, hoping we might be able to get scoping and evaluatio with something
like:

| #define arch_${atomic}_dec_and_test_ofl(_v, _label)					\\
| ({											\\
| 	bool __nested_arch_${atomic}_dec_and_test_ovf(${atomic}_t *v, void *label)	\\
| 	{										\\
| 		${int} new = arch_${atomic}_dec_return(v);				\\
| 		if (unlikely(new < 0))							\\
| 			goto label;							\\
| 		if (unlikely(new == 0))							\\
| 			return true;							\\
| 		return false;								\\
| 	}										\\
| 	__nested_arch_${atomic}_dec_and_test_ovf(_v, &&_label);				\\
| })

... but GCC refused to inline things, and Clang doesn't appear to support
nested functions, so it does appear we're stuck with using macros...

It's a shame to have to use macros, since we have to take great care to avoid
multiple-evaluation and/or shadowing of variables, but we're probably OK in
practice given these are simple enough.

> This meant extending the atomic generation scripts to deal with
> wrapping macros instead of inline functions. Since
> xchg/cmpxchg/try_cmpxchg were already macro magic, there was existant
> code for that. While extending/improving that a few latent
> 'instrumentation' bugs were uncovered and 'accidentally' fixed.

I assume for non-RFC we can split that out into a preparatory patch. :)

Having played about a bit, I don't have any suggestions for improving the
scripting, and I think that looks OK.

Thanks,
Mark.

> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  include/linux/atomic/atomic-arch-fallback.h |   64 ++++++++++++
>  include/linux/atomic/atomic-instrumented.h  |  139 ++++++++++++++++++++--------
>  include/linux/atomic/atomic-long.h          |   32 ++++++
>  scripts/atomic/atomic-tbl.sh                |    6 +
>  scripts/atomic/atomics.tbl                  |    6 +
>  scripts/atomic/fallbacks/dec_and_test_ofl   |   12 ++
>  scripts/atomic/fallbacks/dec_ofl            |    8 +
>  scripts/atomic/fallbacks/inc_ofl            |    8 +
>  scripts/atomic/gen-atomic-fallback.sh       |    4 
>  scripts/atomic/gen-atomic-instrumented.sh   |  117 +++++++++++++++++++----
>  scripts/atomic/gen-atomic-long.sh           |   32 +++++-
>  11 files changed, 359 insertions(+), 69 deletions(-)
> 
> --- a/include/linux/atomic/atomic-arch-fallback.h
> +++ b/include/linux/atomic/atomic-arch-fallback.h
> @@ -1250,6 +1250,37 @@ arch_atomic_dec_if_positive(atomic_t *v)
>  #define arch_atomic_dec_if_positive arch_atomic_dec_if_positive
>  #endif
>  
> +#ifndef arch_atomic_inc_ofl
> +#define arch_atomic_inc_ofl(_v, _label)				\
> +do {									\
> +	int __old = arch_atomic_fetch_inc(_v);			\
> +	if (unlikely(__old <= 0))					\
> +		goto _label;						\
> +} while (0)
> +#endif
> +
> +#ifndef arch_atomic_dec_ofl
> +#define arch_atomic_dec_ofl(_v, _label)				\
> +do {									\
> +	int __new = arch_atomic_dec_return(_v);			\
> +	if (unlikely(__new <= 0))					\
> +		goto _label;						\
> +} while (0)
> +#endif
> +
> +#ifndef arch_atomic_dec_and_test_ofl
> +#define arch_atomic_dec_and_test_ofl(_v, _label)			\
> +({									\
> +	bool __ret = false;						\
> +	int __new = arch_atomic_dec_return(_v);			\
> +	if (unlikely(__new < 0))					\
> +		goto _label;						\
> +	if (unlikely(__new == 0))					\
> +		__ret = true;						\
> +	__ret;								\
> +})
> +#endif
> +
>  #ifdef CONFIG_GENERIC_ATOMIC64
>  #include <asm-generic/atomic64.h>
>  #endif
> @@ -2357,5 +2388,36 @@ arch_atomic64_dec_if_positive(atomic64_t
>  #define arch_atomic64_dec_if_positive arch_atomic64_dec_if_positive
>  #endif
>  
> +#ifndef arch_atomic64_inc_ofl
> +#define arch_atomic64_inc_ofl(_v, _label)				\
> +do {									\
> +	s64 __old = arch_atomic64_fetch_inc(_v);			\
> +	if (unlikely(__old <= 0))					\
> +		goto _label;						\
> +} while (0)
> +#endif
> +
> +#ifndef arch_atomic64_dec_ofl
> +#define arch_atomic64_dec_ofl(_v, _label)				\
> +do {									\
> +	s64 __new = arch_atomic64_dec_return(_v);			\
> +	if (unlikely(__new <= 0))					\
> +		goto _label;						\
> +} while (0)
> +#endif
> +
> +#ifndef arch_atomic64_dec_and_test_ofl
> +#define arch_atomic64_dec_and_test_ofl(_v, _label)			\
> +({									\
> +	bool __ret = false;						\
> +	s64 __new = arch_atomic64_dec_return(_v);			\
> +	if (unlikely(__new < 0))					\
> +		goto _label;						\
> +	if (unlikely(__new == 0))					\
> +		__ret = true;						\
> +	__ret;								\
> +})
> +#endif
> +
>  #endif /* _LINUX_ATOMIC_FALLBACK_H */
> -// cca554917d7ea73d5e3e7397dd70c484cad9b2c4
> +// a59904b14db62a38bbab8699edc4a785a97871fb
> --- a/include/linux/atomic/atomic-instrumented.h
> +++ b/include/linux/atomic/atomic-instrumented.h
> @@ -501,7 +501,7 @@ static __always_inline bool
>  atomic_try_cmpxchg(atomic_t *v, int *old, int new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic_try_cmpxchg(v, old, new);
>  }
>  
> @@ -509,7 +509,7 @@ static __always_inline bool
>  atomic_try_cmpxchg_acquire(atomic_t *v, int *old, int new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic_try_cmpxchg_acquire(v, old, new);
>  }
>  
> @@ -517,7 +517,7 @@ static __always_inline bool
>  atomic_try_cmpxchg_release(atomic_t *v, int *old, int new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic_try_cmpxchg_release(v, old, new);
>  }
>  
> @@ -525,7 +525,7 @@ static __always_inline bool
>  atomic_try_cmpxchg_relaxed(atomic_t *v, int *old, int new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic_try_cmpxchg_relaxed(v, old, new);
>  }
>  
> @@ -599,6 +599,27 @@ atomic_dec_if_positive(atomic_t *v)
>  	return arch_atomic_dec_if_positive(v);
>  }
>  
> +#define atomic_inc_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic_inc_ofl(__ai_v, L); \
> +})
> +
> +#define atomic_dec_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic_dec_ofl(__ai_v, L); \
> +})
> +
> +#define atomic_dec_and_test_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic_dec_and_test_ofl(__ai_v, L); \
> +})
> +
>  static __always_inline s64
>  atomic64_read(const atomic64_t *v)
>  {
> @@ -1079,7 +1100,7 @@ static __always_inline bool
>  atomic64_try_cmpxchg(atomic64_t *v, s64 *old, s64 new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic64_try_cmpxchg(v, old, new);
>  }
>  
> @@ -1087,7 +1108,7 @@ static __always_inline bool
>  atomic64_try_cmpxchg_acquire(atomic64_t *v, s64 *old, s64 new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic64_try_cmpxchg_acquire(v, old, new);
>  }
>  
> @@ -1095,7 +1116,7 @@ static __always_inline bool
>  atomic64_try_cmpxchg_release(atomic64_t *v, s64 *old, s64 new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic64_try_cmpxchg_release(v, old, new);
>  }
>  
> @@ -1103,7 +1124,7 @@ static __always_inline bool
>  atomic64_try_cmpxchg_relaxed(atomic64_t *v, s64 *old, s64 new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic64_try_cmpxchg_relaxed(v, old, new);
>  }
>  
> @@ -1177,6 +1198,27 @@ atomic64_dec_if_positive(atomic64_t *v)
>  	return arch_atomic64_dec_if_positive(v);
>  }
>  
> +#define atomic64_inc_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic64_inc_ofl(__ai_v, L); \
> +})
> +
> +#define atomic64_dec_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic64_dec_ofl(__ai_v, L); \
> +})
> +
> +#define atomic64_dec_and_test_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic64_dec_and_test_ofl(__ai_v, L); \
> +})
> +
>  static __always_inline long
>  atomic_long_read(const atomic_long_t *v)
>  {
> @@ -1657,7 +1699,7 @@ static __always_inline bool
>  atomic_long_try_cmpxchg(atomic_long_t *v, long *old, long new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic_long_try_cmpxchg(v, old, new);
>  }
>  
> @@ -1665,7 +1707,7 @@ static __always_inline bool
>  atomic_long_try_cmpxchg_acquire(atomic_long_t *v, long *old, long new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic_long_try_cmpxchg_acquire(v, old, new);
>  }
>  
> @@ -1673,7 +1715,7 @@ static __always_inline bool
>  atomic_long_try_cmpxchg_release(atomic_long_t *v, long *old, long new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic_long_try_cmpxchg_release(v, old, new);
>  }
>  
> @@ -1681,7 +1723,7 @@ static __always_inline bool
>  atomic_long_try_cmpxchg_relaxed(atomic_long_t *v, long *old, long new)
>  {
>  	instrument_atomic_read_write(v, sizeof(*v));
> -	instrument_atomic_read_write(old, sizeof(*old));
> +	instrument_read_write(old, sizeof(*old));
>  	return arch_atomic_long_try_cmpxchg_relaxed(v, old, new);
>  }
>  
> @@ -1755,87 +1797,108 @@ atomic_long_dec_if_positive(atomic_long_
>  	return arch_atomic_long_dec_if_positive(v);
>  }
>  

> +#define atomic_long_inc_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic_long_inc_ofl(__ai_v, L); \
> +})
> +
> +#define atomic_long_dec_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic_long_dec_ofl(__ai_v, L); \
> +})
> +
> +#define atomic_long_dec_and_test_ofl(v, L) \
> +({ \
> +	typeof(v) __ai_v = (v); \
> +	instrument_atomic_read_write(__ai_v, sizeof(*__ai_v)); \
> +	arch_atomic_long_dec_and_test_ofl(__ai_v, L); \
> +})
> +
>  #define xchg(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_xchg(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define xchg_acquire(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_xchg_acquire(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define xchg_release(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_xchg_release(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define xchg_relaxed(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_xchg_relaxed(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg_acquire(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg_acquire(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg_release(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg_release(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg_relaxed(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg_relaxed(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg64(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg64(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg64_acquire(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg64_acquire(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg64_release(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg64_release(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg64_relaxed(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg64_relaxed(__ai_ptr, __VA_ARGS__); \
>  })
>  
> @@ -1843,8 +1906,8 @@ atomic_long_dec_if_positive(atomic_long_
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
>  	typeof(oldp) __ai_oldp = (oldp); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> -	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_read_write(__ai_oldp, sizeof(*__ai_oldp)); \
>  	arch_try_cmpxchg(__ai_ptr, __ai_oldp, __VA_ARGS__); \
>  })
>  
> @@ -1852,8 +1915,8 @@ atomic_long_dec_if_positive(atomic_long_
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
>  	typeof(oldp) __ai_oldp = (oldp); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> -	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_read_write(__ai_oldp, sizeof(*__ai_oldp)); \
>  	arch_try_cmpxchg_acquire(__ai_ptr, __ai_oldp, __VA_ARGS__); \
>  })
>  
> @@ -1861,8 +1924,8 @@ atomic_long_dec_if_positive(atomic_long_
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
>  	typeof(oldp) __ai_oldp = (oldp); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> -	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_read_write(__ai_oldp, sizeof(*__ai_oldp)); \
>  	arch_try_cmpxchg_release(__ai_ptr, __ai_oldp, __VA_ARGS__); \
>  })
>  
> @@ -1870,36 +1933,36 @@ atomic_long_dec_if_positive(atomic_long_
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
>  	typeof(oldp) __ai_oldp = (oldp); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> -	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_read_write(__ai_oldp, sizeof(*__ai_oldp)); \
>  	arch_try_cmpxchg_relaxed(__ai_ptr, __ai_oldp, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg_local(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg_local(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg64_local(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg64_local(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define sync_cmpxchg(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_sync_cmpxchg(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #define cmpxchg_double(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, 2 * sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg_double(__ai_ptr, __VA_ARGS__); \
>  })
>  
> @@ -1907,9 +1970,9 @@ atomic_long_dec_if_positive(atomic_long_
>  #define cmpxchg_double_local(ptr, ...) \
>  ({ \
>  	typeof(ptr) __ai_ptr = (ptr); \
> -	instrument_atomic_write(__ai_ptr, 2 * sizeof(*__ai_ptr)); \
> +	instrument_atomic_read_write(__ai_ptr, sizeof(*__ai_ptr)); \
>  	arch_cmpxchg_double_local(__ai_ptr, __VA_ARGS__); \
>  })
>  
>  #endif /* _LINUX_ATOMIC_INSTRUMENTED_H */
> -// 2a9553f0a9d5619f19151092df5cabbbf16ce835
> +// 214f9a7e972966a9a8e28e1568665cfb75decf91
> --- a/include/linux/atomic/atomic-long.h
> +++ b/include/linux/atomic/atomic-long.h
> @@ -515,6 +515,21 @@ arch_atomic_long_dec_if_positive(atomic_
>  	return arch_atomic64_dec_if_positive(v);
>  }
>  
> +#define arch_atomic_long_inc_ofl(v, L) \
> +({ \
> +	arch_atomic64_inc_ofl((v), L) \
> +})
> +
> +#define arch_atomic_long_dec_ofl(v, L) \
> +({ \
> +	arch_atomic64_dec_ofl((v), L) \
> +})
> +
> +#define arch_atomic_long_dec_and_test_ofl(v, L) \
> +({ \
> +	arch_atomic64_dec_and_test_ofl((v), L) \
> +})
> +
>  #else /* CONFIG_64BIT */
>  
>  static __always_inline long
> @@ -1009,6 +1024,21 @@ arch_atomic_long_dec_if_positive(atomic_
>  	return arch_atomic_dec_if_positive(v);
>  }
>  
> +#define arch_atomic_long_inc_ofl(v, L) \
> +({ \
> +	arch_atomic_inc_ofl((v), L) \
> +})
> +
> +#define arch_atomic_long_dec_ofl(v, L) \
> +({ \
> +	arch_atomic_dec_ofl((v), L) \
> +})
> +
> +#define arch_atomic_long_dec_and_test_ofl(v, L) \
> +({ \
> +	arch_atomic_dec_and_test_ofl((v), L) \
> +})
> +
>  #endif /* CONFIG_64BIT */
>  #endif /* _LINUX_ATOMIC_LONG_H */


> -// e8f0e08ff072b74d180eabe2ad001282b38c2c88
> +// 120f718985fa4c8f0e884cc4f23db8aa950255fb
> --- a/scripts/atomic/atomic-tbl.sh
> +++ b/scripts/atomic/atomic-tbl.sh
> @@ -36,6 +36,12 @@ meta_has_relaxed()
>  	meta_in "$1" "BFIR"
>  }
>  
> +#meta_has_macro(meta)
> +meta_has_macro()
> +{
> +	meta_in "$1" "m"
> +}
> +
>  #find_fallback_template(pfx, name, sfx, order)
>  find_fallback_template()
>  {
> --- a/scripts/atomic/atomics.tbl
> +++ b/scripts/atomic/atomics.tbl
> @@ -10,12 +10,15 @@
>  # * F/f	- fetch: returns base type (has fetch_ variants)
>  # * l	- load: returns base type (has _acquire order variant)
>  # * s	- store: returns void (has _release order variant)
> +# * m	- macro:
>  #
>  # Where args contains list of type[:name], where type is:
>  # * cv	- const pointer to atomic base type (atomic_t/atomic64_t/atomic_long_t)
>  # * v	- pointer to atomic base type (atomic_t/atomic64_t/atomic_long_t)
>  # * i	- base type (int/s64/long)
>  # * p	- pointer to base type (int/s64/long)
> +# * L	- label for exception case
> +# * V:... - vararg



>  #
>  read			l	cv
>  set			s	v	i
> @@ -39,3 +42,6 @@ inc_not_zero		b	v
>  inc_unless_negative	b	v
>  dec_unless_positive	b	v
>  dec_if_positive		i	v
> +inc_ofl			m	v	L
> +dec_ofl			m	v	L
> +dec_and_test_ofl	m	v	L
> --- /dev/null
> +++ b/scripts/atomic/fallbacks/dec_and_test_ofl
> @@ -0,0 +1,12 @@
> +cat << EOF
> +#define arch_${atomic}_dec_and_test_ofl(_v, _label)			\\
> +({									\\
> +	bool __ret = false;						\\
> +	${int} __new = arch_${atomic}_dec_return(_v);			\\
> +	if (unlikely(__new < 0))					\\
> +		goto _label;						\\
> +	if (unlikely(__new == 0))					\\
> +		__ret = true;						\\
> +	__ret;								\\
> +})
> +EOF
> --- /dev/null
> +++ b/scripts/atomic/fallbacks/dec_ofl
> @@ -0,0 +1,8 @@
> +cat << EOF
> +#define arch_${atomic}_dec_ofl(_v, _label)				\\
> +do {									\\
> +	${int} __new = arch_${atomic}_dec_return(_v);			\\
> +	if (unlikely(__new <= 0))					\\
> +		goto _label;						\\
> +} while (0)
> +EOF
> --- /dev/null
> +++ b/scripts/atomic/fallbacks/inc_ofl
> @@ -0,0 +1,8 @@
> +cat << EOF
> +#define arch_${atomic}_inc_ofl(_v, _label)				\\
> +do {									\\
> +	${int} __old = arch_${atomic}_fetch_inc(_v);			\\
> +	if (unlikely(__old <= 0))					\\
> +		goto _label;						\\
> +} while (0)
> +EOF
> --- a/scripts/atomic/gen-atomic-fallback.sh
> +++ b/scripts/atomic/gen-atomic-fallback.sh
> @@ -27,7 +27,9 @@ gen_template_fallback()
>  	if [ ! -z "${template}" ]; then
>  		printf "#ifndef ${atomicname}\n"
>  		. ${template}
> -		printf "#define ${atomicname} ${atomicname}\n"
> +		if ! meta_has_macro "${meta}"; then
> +			printf "#define ${atomicname} ${atomicname}\n"
> +		fi
>  		printf "#endif\n\n"
>  	fi
>  }
> --- a/scripts/atomic/gen-atomic-instrumented.sh
> +++ b/scripts/atomic/gen-atomic-instrumented.sh
> @@ -13,9 +13,13 @@ gen_param_check()
>  	local type="${arg%%:*}"
>  	local name="$(gen_param_name "${arg}")"
>  	local rw="write"
> +	local pfx;
>  
>  	case "${type#c}" in
> +	v) pfx="atomic_";;
>  	i) return;;
> +	L) return;;
> +	V) return;;
>  	esac
>  
>  	if [ ${type#c} != ${type} ]; then
> @@ -27,7 +31,16 @@ gen_param_check()
>  		rw="read_write"
>  	fi
>  
> -	printf "\tinstrument_atomic_${rw}(${name}, sizeof(*${name}));\n"
> +	if meta_has_macro "${meta}"; then
> +		name="__ai_${name}"
> +	fi
> +
> +	printf "\tinstrument_${pfx}${rw}(${name}, sizeof(*${name}));"
> +	if meta_has_macro "${meta}"; then
> +		printf " \\"
> +	fi
> +	printf "\n"
> +
>  }
>  
>  #gen_params_checks(meta, arg...)
> @@ -41,6 +54,52 @@ gen_params_checks()
>  	done
>  }
>  
> +#gen_var(arg)
> +gen_var()
> +{
> +	local type="${1%%:*}"
> +	local name="$(gen_param_name "$1")"
> +
> +	case "${type#c}" in
> +	L) return;;
> +	V) return;;
> +	esac
> +
> +	printf "\ttypeof(${name}) __ai_${name} = (${name}); \\\\\n";
> +}
> +
> +#gen_vars(arg...)
> +gen_vars()
> +{
> +	while [ "$#" -gt 0 ]; do
> +		gen_var "$1"
> +		shift
> +	done
> +}
> +
> +#gen_varg(arg)
> +gen_varg()
> +{
> +	local type="${1%%:*}"
> +	local name="$(gen_param_name "$1")"
> +
> +	case "${type#c}" in
> +	L)	printf "${name}";;
> +	V)	printf "__VA_ARGS__";;
> +	*)	printf "__ai_${name}";;
> +	esac
> +}
> +
> +#gen_vargs(arg...)
> +gen_vargs()
> +{
> +	while [ "$#" -gt 0 ]; do
> +		printf "$(gen_varg "$1")"
> +		[ "$#" -gt 1 ] && printf ", "
> +		shift
> +	done
> +}
> +
>  #gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, int, arg...)
>  gen_proto_order_variant()
>  {
> @@ -54,11 +113,28 @@ gen_proto_order_variant()
>  
>  	local atomicname="${atomic}_${pfx}${name}${sfx}${order}"
>  
> -	local ret="$(gen_ret_type "${meta}" "${int}")"
> -	local params="$(gen_params "${int}" "${atomic}" "$@")"
>  	local checks="$(gen_params_checks "${meta}" "$@")"
>  	local args="$(gen_args "$@")"
> -	local retstmt="$(gen_ret_stmt "${meta}")"
> +
> +	if meta_has_macro "${meta}"; then
> +
> +		local vars="$(gen_vars "$@")"
> +		local vargs="$(gen_vargs "$@")"
> +
> +cat <<EOF
> +#define ${atomicname}(${args}) \\
> +({ \\
> +${vars}
> +${checks}
> +	arch_${atomicname}(${vargs}); \\
> +})
> +EOF
> +
> +	else
> +
> +		local ret="$(gen_ret_type "${meta}" "${int}")"
> +		local params="$(gen_params "${int}" "${atomic}" "$@")"
> +		local retstmt="$(gen_ret_stmt "${meta}")"
>  
>  cat <<EOF
>  static __always_inline ${ret}
> @@ -69,6 +145,8 @@ ${checks}
>  }
>  EOF
>  
> +	fi
> +
>  	printf "\n"
>  }
>  
> @@ -76,32 +154,27 @@ gen_xchg()
>  {
>  	local xchg="$1"; shift
>  	local mult="$1"; shift
> +	local ARGS;
>  
>  	if [ "${xchg%${xchg#try_cmpxchg}}" = "try_cmpxchg" ] ; then
> -
> -cat <<EOF
> -#define ${xchg}(ptr, oldp, ...) \\
> -({ \\
> -	typeof(ptr) __ai_ptr = (ptr); \\
> -	typeof(oldp) __ai_oldp = (oldp); \\
> -	instrument_atomic_write(__ai_ptr, ${mult}sizeof(*__ai_ptr)); \\
> -	instrument_atomic_write(__ai_oldp, ${mult}sizeof(*__ai_oldp)); \\
> -	arch_${xchg}(__ai_ptr, __ai_oldp, __VA_ARGS__); \\
> -})
> -EOF
> -
> +		ARGS="v:ptr p:oldp V:..."
>  	else
> +		ARGS="v:ptr V:..."
> +	fi
> +
> +	local args="$(gen_args ${ARGS})"
> +	local vars="$(gen_vars ${ARGS})"
> +	local checks="$(gen_params_checks "m" ${ARGS})"
> +	local vargs="$(gen_vargs ${ARGS})"
>  
>  cat <<EOF
> -#define ${xchg}(ptr, ...) \\
> +#define ${xchg}(${args}) \\
>  ({ \\
> -	typeof(ptr) __ai_ptr = (ptr); \\
> -	instrument_atomic_write(__ai_ptr, ${mult}sizeof(*__ai_ptr)); \\
> -	arch_${xchg}(__ai_ptr, __VA_ARGS__); \\
> +${vars}
> +${checks}
> +	arch_${xchg}(${vargs}); \\
>  })
>  EOF
> -
> -	fi
>  }
>  
>  cat << EOF
> --- a/scripts/atomic/gen-atomic-long.sh
> +++ b/scripts/atomic/gen-atomic-long.sh
> @@ -17,16 +17,21 @@ gen_cast()
>  	printf "($(gen_param_type "${arg}" "${int}" "${atomic}"))"
>  }
>  
> -#gen_args_cast(int, atomic, arg...)
> +#gen_args_cast(meta, int, atomic, arg...)
>  gen_args_cast()
>  {
> +	local meta=$1; shift
>  	local int="$1"; shift
>  	local atomic="$1"; shift
>  
>  	while [ "$#" -gt 0 ]; do
>  		local cast="$(gen_cast "$1" "${int}" "${atomic}")"
>  		local arg="$(gen_param_name "$1")"
> -		printf "${cast}${arg}"
> +		if meta_has_macro "${meta}" && [ "${1%%:*}" != "L" ]; then
> +			printf "${cast}(${arg})"
> +		else
> +			printf "${cast}${arg}"
> +		fi
>  		[ "$#" -gt 1 ] && printf ", "
>  		shift;
>  	done
> @@ -40,10 +45,24 @@ gen_proto_order_variant()
>  	local atomic="$1"; shift
>  	local int="$1"; shift
>  
> -	local ret="$(gen_ret_type "${meta}" "long")"
> -	local params="$(gen_params "long" "atomic_long" "$@")"
> -	local argscast="$(gen_args_cast "${int}" "${atomic}" "$@")"
> -	local retstmt="$(gen_ret_stmt "${meta}")"
> +	local argscast="$(gen_args_cast "${meta}" "${int}" "${atomic}" "$@")"
> +
> +	if meta_has_macro "${meta}"; then
> +
> +		local args="$(gen_args "$@")"
> +
> +cat <<EOF
> +#define arch_atomic_long_${name}(${args}) \\
> +({ \\
> +	arch_${atomic}_${name}(${argscast}) \\
> +})
> +
> +EOF
> +	else
> +
> +		local ret="$(gen_ret_type "${meta}" "long")"
> +		local params="$(gen_params "long" "atomic_long" "$@")"
> +		local retstmt="$(gen_ret_stmt "${meta}")"
>  
>  cat <<EOF
>  static __always_inline ${ret}
> @@ -53,6 +72,7 @@ arch_atomic_long_${name}(${params})
>  }
>  
>  EOF
> +	fi
>  }
>  
>  cat << EOF
> 
> 

Re: [RFC][PATCH 2/5] refcount: Use atomic_*_ofl()

[Mark Rutland]

On Wed, Dec 08, 2021 at 07:36:57PM +0100, Peter Zijlstra wrote:
> Use the shiny new atomic_*_ofl() functions in order to have better
> code-gen.
> 
> Notably refcount_inc() case no longer distinguishes between
> inc-from-zero and inc-negative in the fast path, this improves
> code-gen:
> 
>     4b88:       b8 01 00 00 00          mov    $0x1,%eax
>     4b8d:       f0 0f c1 43 28          lock xadd %eax,0x28(%rbx)
>     4b92:       85 c0                   test   %eax,%eax
>     4b94:       74 1b                   je     4bb1 <alloc_perf_context+0xf1>
>     4b96:       8d 50 01                lea    0x1(%rax),%edx
>     4b99:       09 c2                   or     %eax,%edx
>     4b9b:       78 20                   js     4bbd <alloc_perf_context+0xfd>
> 
> to:
> 
>     4768:       b8 01 00 00 00          mov    $0x1,%eax
>     476d:       f0 0f c1 43 28          lock xadd %eax,0x28(%rbx)
>     4772:       85 c0                   test   %eax,%eax
>     4774:       7e 14                   jle    478a <alloc_perf_context+0xea>

For comparison, I generated the same for arm64 below with kernel.org crosstool
GCC 11.1.0 and defconfig.

For arm64 there's an existing sub-optimiality for inc/dec where the register
for `1` or `-1` gets generated with a `MOV;MOV` chain or `MOV;NEG` chain rather
than a single `MOV` in either case. I think taht's due to the way we build
LSE/LL-SC variants of add() and build a common inc() atop, and the compiler
just loses the opportunity to constant-fold. I'll take a look at how to make
that neater.

Regardless, the code goes from:

    51f4:       52800024        mov     w4, #0x1                        // #1
    ...
    5224:       2a0403e1        mov     w1, w4
    5228:       b8210001        ldadd   w1, w1, [x0]
    522c:       34000261        cbz     w1, 5278 <alloc_perf_context+0xf8>
    5230:       11000422        add     w2, w1, #0x1
    5234:       2a010041        orr     w1, w2, w1
    5238:       37f80181        tbnz    w1, #31, 5268 <alloc_perf_context+0xe8>

to:

    40e8:       52800024        mov     w4, #0x1                        // #1
    ...
    4118:       2a0403e1        mov     w1, w4
    411c:       b8e10001        ldaddal w1, w1, [x0]
    4120:       7100003f        cmp     w1, #0x0
    4124:       5400018d        b.le    4154 <alloc_perf_context+0xe0>

> without sacrificing on functionality; the only thing that suffers is
> the reported error condition, which might now 'spuriously' report
> 'saturated' instead of 'inc-from-zero'.
> 
> refcount_dec_and_test() is also improved:
> 
>     aa40:       b8 ff ff ff ff          mov    $0xffffffff,%eax
>     aa45:       f0 0f c1 07             lock xadd %eax,(%rdi)
>     aa49:       83 f8 01                cmp    $0x1,%eax
>     aa4c:       74 05                   je     aa53 <ring_buffer_put+0x13>
>     aa4e:       85 c0                   test   %eax,%eax
>     aa50:       7e 1e                   jle    aa70 <ring_buffer_put+0x30>
>     aa52:       c3                      ret
> 
> to:
> 
>     a980:       b8 ff ff ff ff          mov    $0xffffffff,%eax
>     a985:       f0 0f c1 07             lock xadd %eax,(%rdi)
>     a989:       83 e8 01                sub    $0x1,%eax
>     a98c:       78 20                   js     a9ae <ring_buffer_put+0x2e>
>     a98e:       74 01                   je     a991 <ring_buffer_put+0x11>
>     a990:       c3                      ret

For arm64 (with your fixlet applied) that goes from:

    c42c:       52800021        mov     w1, #0x1                        // #1
    c430:       4b0103e1        neg     w1, w1
    c434:       b8610001        ldaddl  w1, w1, [x0]
    c438:       7100043f        cmp     w1, #0x1
    c43c:       54000140        b.eq    c464 <ring_buffer_put+0x50>  // b.none
    c440:       7100003f        cmp     w1, #0x0
    c444:       5400028d        b.le    c494 <ring_buffer_put+0x80>

to:

    c3dc:       52800021        mov     w1, #0x1                        // #1
    c3e0:       4b0103e1        neg     w1, w1
    c3e4:       b8e10002        ldaddal w1, w2, [x0]
    c3e8:       0b020021        add     w1, w1, w2
    c3ec:       7100003f        cmp     w1, #0x0
    c3f0:       5400012b        b.lt    c414 <ring_buffer_put+0x50>  // b.tstop
    c3f4:       540001a0        b.eq    c428 <ring_buffer_put+0x64>  // b.none

I think the add here is due to  the change in your fixlet:

-       if (unlikely(old <= 1))                                                                                                                                                                             
+       if (unlikely(old - 1 <= 0)) 

> XXX atomic_dec_and_test_ofl() is strictly stronger ordered than
> refcount_dec_and_test() is defined -- Power and Arrghh64 ?

I'll leave the ordering to Will.

Thanks,
Mark.

> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  include/linux/refcount.h |   15 ++++++++++++---
>  lib/refcount.c           |    5 +++++
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> --- a/include/linux/refcount.h
> +++ b/include/linux/refcount.h
> @@ -264,7 +264,10 @@ static inline void __refcount_inc(refcou
>   */
>  static inline void refcount_inc(refcount_t *r)
>  {
> -	__refcount_inc(r, NULL);
> +	atomic_inc_ofl(&r->refs, Eoverflow);
> +	return;
> +Eoverflow:
> +	refcount_warn_saturate(r, REFCOUNT_ADD_OVF);
>  }
>  
>  static inline __must_check bool __refcount_sub_and_test(int i, refcount_t *r, int *oldp)
> @@ -330,7 +333,10 @@ static inline __must_check bool __refcou
>   */
>  static inline __must_check bool refcount_dec_and_test(refcount_t *r)
>  {
> -	return __refcount_dec_and_test(r, NULL);
> +	return atomic_dec_and_test_ofl(&r->refs, Eoverflow);
> +Eoverflow:
> +	refcount_warn_saturate(r, REFCOUNT_SUB_UAF);
> +	return false;
>  }
>  
>  static inline void __refcount_dec(refcount_t *r, int *oldp)
> @@ -356,7 +362,10 @@ static inline void __refcount_dec(refcou
>   */
>  static inline void refcount_dec(refcount_t *r)
>  {
> -	__refcount_dec(r, NULL);
> +	atomic_dec_ofl(&r->refs, Eoverflow);
> +	return;
> +Eoverflow:
> +	refcount_warn_saturate(r, REFCOUNT_DEC_LEAK);
>  }
>  
>  extern __must_check bool refcount_dec_if_one(refcount_t *r);
> --- a/lib/refcount.c
> +++ b/lib/refcount.c
> @@ -12,8 +12,13 @@
>  
>  void refcount_warn_saturate(refcount_t *r, enum refcount_saturation_type t)
>  {
> +	int old = refcount_read(r);
>  	refcount_set(r, REFCOUNT_SATURATED);
>  
> +	/* racy; who cares */
> +	if (old == 1 && t == REFCOUNT_ADD_OVF)
> +		t = REFCOUNT_ADD_UAF;
> +
>  	switch (t) {
>  	case REFCOUNT_ADD_NOT_ZERO_OVF:
>  		REFCOUNT_WARN("saturated; leaking memory");
> 
> 

Re: [RFC][PATCH 1/5] atomic: Introduce atomic_{inc,dec,dec_and_test}_ofl()

[Peter Zijlstra]

On Thu, Dec 09, 2021 at 12:42:47PM +0000, Mark Rutland wrote:
> On Wed, Dec 08, 2021 at 07:36:56PM +0100, Peter Zijlstra wrote:
> > In order to facilitate architecture support for refcount_t, introduce
> > a number of new atomic primitives that have a uaccess style exception
> > for overflow.
> > 
> > Notably:
> > 
> >   atomic_inc_ofl(v, Label) -- increment and goto Label when
> > 			      v is zero or negative.
> > 
> >   atomic_dec_ofl(v, Label) -- decrement and goto Label when
> > 			      the result is zero or negative
> > 
> >   atomic_dec_and_test_ofl(v, Label) -- decrement and return true when
> > 				       the result is zero and goto Label
> > 				       when the result is negative
> 
> Just to check, atomic_inc_ofl() tests the *old* value of `v`, and the other
> cases check the *new* value of `v`?
> 
> For clarity, in the descriptions it might be worth:
> 
>   s/v/the old value of v/
>   s/the result/the new value of v/
> 
> ... which I think makes that clearer.

Right, I'll clarify.

> > Since the GCC 'Labels as Values' extention doesn't allow having the
> > goto in an inline function, these new 'functions' must in fact be
> > implemented as macro magic.
> 
> Oh; fun... :(

Yeah, I tried all sorta things, it's all >.< close to working but then
GCC refuses to do the sensible thing.

> > This meant extending the atomic generation scripts to deal with
> > wrapping macros instead of inline functions. Since
> > xchg/cmpxchg/try_cmpxchg were already macro magic, there was existant
> > code for that. While extending/improving that a few latent
> > 'instrumentation' bugs were uncovered and 'accidentally' fixed.
> 
> I assume for non-RFC we can split that out into a preparatory patch. :)

Sure, I can split it in two; one add the infra and fix bugs and two
introduce the new ops.

Re: [RFC][PATCH 0/5] refcount: Improve code-gen

[Jens Axboe]

On 12/9/21 1:25 AM, Peter Zijlstra wrote:
> On Wed, Dec 08, 2021 at 07:36:55PM +0100, Peter Zijlstra wrote:
>> Hi,
>>
>> Improves the refcount_t code-gen; I've still got to go through the latest thing
>> Linus suggested, but figured I should get these patches out to see if there's
>> other concerns etc..
>>
> 
> Bah, I forgot to Cc Jens, let me bounce him the lot.

Traveling the next few days, just put me on v2 and I should have time to
look at this start next week. Bouncing only helps for initial messages,
I'll have to do dig for followups :-)

-- 
Jens Axboe

Re: [RFC][PATCH 0/5] refcount: Improve code-gen

[Peter Zijlstra]

On Thu, Dec 09, 2021 at 09:19:58AM -0700, Jens Axboe wrote:
> On 12/9/21 1:25 AM, Peter Zijlstra wrote:
> > On Wed, Dec 08, 2021 at 07:36:55PM +0100, Peter Zijlstra wrote:
> >> Hi,
> >>
> >> Improves the refcount_t code-gen; I've still got to go through the latest thing
> >> Linus suggested, but figured I should get these patches out to see if there's
> >> other concerns etc..
> >>
> > 
> > Bah, I forgot to Cc Jens, let me bounce him the lot.
> 
> Traveling the next few days, just put me on v2 and I should have time to
> look at this start next week. Bouncing only helps for initial messages,
> I'll have to do dig for followups :-)

Hmm.. I tagged the whole thread and did tag-bounce. Anyway, I'll be sure
to copy you on v2. Thanks!

Re: [RFC][PATCH 2/5] refcount: Use atomic_*_ofl()

[Mark Rutland]

On Thu, Dec 09, 2021 at 01:17:33PM +0000, Mark Rutland wrote:
> On Wed, Dec 08, 2021 at 07:36:57PM +0100, Peter Zijlstra wrote:
> > Use the shiny new atomic_*_ofl() functions in order to have better
> > code-gen.
> > 
> > Notably refcount_inc() case no longer distinguishes between
> > inc-from-zero and inc-negative in the fast path, this improves
> > code-gen:
> > 
> >     4b88:       b8 01 00 00 00          mov    $0x1,%eax
> >     4b8d:       f0 0f c1 43 28          lock xadd %eax,0x28(%rbx)
> >     4b92:       85 c0                   test   %eax,%eax
> >     4b94:       74 1b                   je     4bb1 <alloc_perf_context+0xf1>
> >     4b96:       8d 50 01                lea    0x1(%rax),%edx
> >     4b99:       09 c2                   or     %eax,%edx
> >     4b9b:       78 20                   js     4bbd <alloc_perf_context+0xfd>
> > 
> > to:
> > 
> >     4768:       b8 01 00 00 00          mov    $0x1,%eax
> >     476d:       f0 0f c1 43 28          lock xadd %eax,0x28(%rbx)
> >     4772:       85 c0                   test   %eax,%eax
> >     4774:       7e 14                   jle    478a <alloc_perf_context+0xea>
> 
> For comparison, I generated the same for arm64 below with kernel.org crosstool
> GCC 11.1.0 and defconfig.
> 
> For arm64 there's an existing sub-optimiality for inc/dec where the register
> for `1` or `-1` gets generated with a `MOV;MOV` chain or `MOV;NEG` chain rather
> than a single `MOV` in either case. I think taht's due to the way we build
> LSE/LL-SC variants of add() and build a common inc() atop, and the compiler
> just loses the opportunity to constant-fold. I'll take a look at how to make
> that neater.

With some improvement's to arm64's LSE atomics, that becomes a comparable
sequence to x86's:

    2df8:       52800021        mov     w1, #0x1                        // #1
    ...
    2e20:       b8e10002        ldaddal w1, w2, [x0]
    2e24:       7100005f        cmp     w2, #0x0
    2e28:       5400012d        b.le    2e4c <alloc_perf_context+0xc8>

> > without sacrificing on functionality; the only thing that suffers is
> > the reported error condition, which might now 'spuriously' report
> > 'saturated' instead of 'inc-from-zero'.
> > 
> > refcount_dec_and_test() is also improved:
> > 
> >     aa40:       b8 ff ff ff ff          mov    $0xffffffff,%eax
> >     aa45:       f0 0f c1 07             lock xadd %eax,(%rdi)
> >     aa49:       83 f8 01                cmp    $0x1,%eax
> >     aa4c:       74 05                   je     aa53 <ring_buffer_put+0x13>
> >     aa4e:       85 c0                   test   %eax,%eax
> >     aa50:       7e 1e                   jle    aa70 <ring_buffer_put+0x30>
> >     aa52:       c3                      ret
> > 
> > to:
> > 
> >     a980:       b8 ff ff ff ff          mov    $0xffffffff,%eax
> >     a985:       f0 0f c1 07             lock xadd %eax,(%rdi)
> >     a989:       83 e8 01                sub    $0x1,%eax
> >     a98c:       78 20                   js     a9ae <ring_buffer_put+0x2e>
> >     a98e:       74 01                   je     a991 <ring_buffer_put+0x11>
> >     a990:       c3                      ret

Likewise I can get the arm64 equivalent down to:

    bebc:       12800001        mov     w1, #0xffffffff                 // #-1
    ...
    becc:       b8e10001        ldaddal w1, w1, [x0]
    bed0:       71000421        subs    w1, w1, #0x1
    bed4:       540000c4        b.mi    beec <ring_buffer_put+0x3c>  // b.first
    bed8:       54000120        b.eq    befc <ring_buffer_put+0x4c>  // b.none

I've pushed my WIP patches to:

  https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/atomics/improvements
  git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm64/atomics/improvements

... and I'll try to get those cleaned up and posted soon.

Thanks,
Mark.

Re: [RFC][PATCH 3/5] refcount: Improve out-of-line code-gen

[Linus Torvalds]

On Thu, Dec 9, 2021 at 12:33 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> For some obscure raisin it causes this: [snip]

Looks like the code generation at one point thought that it needs to
get the return value from the slow-path function call, and by the time
it had optimized that away it had already ended up with enough
temporaries that it needed to spill things into %rbx.

That looks like a fairly "internal to gcc" thing, but I think dropping
that patch is indeed the right thing to do.

When you made it do

        return refcount_warn_saturate(r, ..);

it now means that those inline functions don't obviously always return
'false' any more, so the compiler did end up needing a variable for
the return value at one point, and it really was a much more complex
decision. After it has inlined that top-level function, the return
value is not visible to the compiler because it doesn't see that
refcount_warn_saturate() always returns false.

The fact that quite often that whole refcount_warn_saturate() call
ended up then being in a cold part of the code, and could then be
optimized to a tailcall if that was the last thing generated, doesn't
end up helping in the general case, and instead only hurts: the
compiler doesn't see what the return value is for the warning case, so
it might end up doing other things in the place that the function was
inlined into.

I think the original patch was likely a mis-optimization triggered by
your test-case being a function that *only* did that
refcount_add_not_zero(), and returned the value. It didn't actually
have other code that then depended on the return value of it.

              Linus