On Wed, Oct 21, 2026, at 11:55, Philipp Stanner wrote:
> The kernel already has test_bit_acquire(). A set_bit_release(), however,
> was missing so far. It can be expected to be useful for parties that
> need release ordering, but not the stricter full ordering of the other
> bit-setting functions.
>
> Add set_bit_release().
>
> Signed-off-by: Philipp Stanner <[email protected]>
> +/**
> + * set_bit_release - Set a bit with release ordering
> + * @nr: Bit to set
> + * @addr: Address to start counting from
> + *
> + * This is an atomic ordered operation (implied write memory barrier).
> + */
Can we fix the mismatch between code comments and documentation for
the set_bit()/clear_bit()/change_bit() barrier semantics while adding
this? I'm always confused by this and the new function makes it harder
to understand in the current form:
- Your patch description mentions that set_bit() has strong rw
barrier semantics and that set_bit_release() would be a weaker
write-barrier version of it. This makes sense given the
set_bit() implementation based on strong raw_atomic_long_or()
in arch_set_bit() and the text in Documentation/memory-barriers.txt
- Contrary to that, the kerneldoc comment over set_bit() mentions
"This is a relaxed atomic operation (no implied memory barriers)"
and leaves out the kcsan_mb() that is in test_and_set_bit().
This makes the new set_bit_release() sound like a /stronger/
version of set_bit() the same way that test_bit_acquire()
is a stronger version of the (relaxed) test_bit().
As I understand it, the kerneldoc comments here are incorrect
and trace back to the include/asm-i386/bitops.h file before the
git era. The patch below should make them consistent with your
patch and with the actual implementation.
Arnd
diff --git a/include/asm-generic/bitops/instrumented-atomic.h
b/include/asm-generic/bitops/instrumented-atomic.h
index c010d54275e4..df200802e929 100644
--- a/include/asm-generic/bitops/instrumented-atomic.h
+++ b/include/asm-generic/bitops/instrumented-atomic.h
@@ -18,13 +18,14 @@
* @nr: the bit to set
* @addr: the address to start counting from
*
- * This is a relaxed atomic operation (no implied memory barriers).
+ * This is an atomic fully-ordered operation (implied full memory barrier).
*
* Note that @nr may be almost arbitrarily large; this function is not
* restricted to acting on a single-word quantity.
*/
static __always_inline void set_bit(long nr, volatile unsigned long *addr)
{
+ kcsan_mb();
instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
arch_set_bit(nr, addr);
}
@@ -34,10 +35,11 @@ static __always_inline void set_bit(long nr, volatile
unsigned long *addr)
* @nr: Bit to clear
* @addr: Address to start counting from
*
- * This is a relaxed atomic operation (no implied memory barriers).
+ * This is an atomic fully-ordered operation (implied full memory barrier).
*/
static __always_inline void clear_bit(long nr, volatile unsigned long *addr)
{
+ kcsan_mb();
instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
arch_clear_bit(nr, addr);
}
@@ -47,13 +49,14 @@ static __always_inline void clear_bit(long nr, volatile
unsigned long *addr)
* @nr: Bit to change
* @addr: Address to start counting from
*
- * This is a relaxed atomic operation (no implied memory barriers).
+ * This is an atomic fully-ordered operation (implied full memory barrier).
*
* Note that @nr may be almost arbitrarily large; this function is not
* restricted to acting on a single-word quantity.
*/
static __always_inline void change_bit(long nr, volatile unsigned long *addr)
{
+ kcsan_mb();
instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
arch_change_bit(nr, addr);
}
