Re: [PATCH 1/2] asm-generic/bitops: Add set_bit_release()

["Gary Guo" <gary@garyguo.net>]

On Wed Oct 21, 2026 at 1:17 PM BST, Philipp Stanner wrote:
> On Wed, 2026-10-21 at 13:06 +0100, Gary Guo wrote:
>
>
> […]
>
>> > +/**
>> > + * 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).
>> > + */
>> > +static __always_inline void set_bit_release(long nr, volatile unsigned long *addr)
>> > +{
>> > +	kcsan_wmb();
>> > +	instrument_atomic_write(addr + BIT_WORD(nr), sizeof(long));
>> > +	arch_set_bit(nr, addr);
>> 
>> This is just wrong. There's no code here that gives you the release semantics.
>> 
>> kcsan_wmb() is just an annotation and does nothing. wmb is also not release
>> semantics as it does not order LOAD->STORE.
>
> What are the other functions here then even about?
>
> /**
>  * test_and_set_bit - Set a bit and return its old value
>  * @nr: Bit to set
>  * @addr: Address to count from
>  *
>  * This is an atomic fully-ordered operation (implied full memory barrier).
>  */
> static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
> {
> 	kcsan_mb();
> 	instrument_atomic_read_write(addr + BIT_WORD(nr), sizeof(long));
> 	return arch_test_and_set_bit(nr, addr);
> }
>
> Where does the "full ordering" come from? I suppose implied barrier
> then means it comes from the architecture's implementation. But what's
> then the purpose of kcsan_XX ?

It's from arch_test_and_set_bit.

KCSAN doesn't see arch code, so the kcsan_mb() and in fact
instrumnent_atomic_read_write() both are annotations to indicate that this is
conceptually a mb() + STORE for instrumentation purpose.

Best,
Gary