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 ?
> Given that set_bit is a relaxed RMW atomic, you can implement set_bit_release
> with just
>
> smp_mb__before_atomic();
> set_bit(nr, addr);
>
> Which should be correct, but is too strong on some architectures (like
> RISC-V),
> becuase it will produce a full MB instead. So we can either add release
> barriers
> (hint: I really like it), or we can add arch_set_bit_release and implement it
> per arch; on RISC-V for example it can then be translated to amoor.rl (which
> is
> not being produced by any existing bit ops).c
All fine by me.
P.
