Re: [PATCH v2] atomics: fix atomic64_{read_acquire,set_release} fallbacks

From: Mark Rutland
Date: Mon Nov 07 2022 - 05:37:26 EST


On Sat, Nov 05, 2022 at 12:05:30AM +0100, Thorsten Glaser wrote:
> On Thu, 3 Feb 2022, Mark Rutland wrote:
>
> > - smp_store_release(&(v)->counter, i);
> > + if (__native_word(atomic_t)) {
> > + smp_store_release(&(v)->counter, i);
> > + } else {
> > + __atomic_release_fence();
> > + arch_atomic_set(v, i);
> > + }
>
> Shouldn’t this also update Documentation/atomic_t.txt which
> currently states:
>
> | The non-RMW ops are (typically) regular LOADs and STOREs and are canonically
> | implemented using READ_ONCE(), WRITE_ONCE(), smp_load_acquire() and
> | smp_store_release() respectively. Therefore, if you find yourself only using
> | the Non-RMW operations of atomic_t, you do not in fact need atomic_t at all
> | and are doing it wrong.
>
> With this, direct use of atomic64_set_release() and atomic64_read_acquire()
> is (IIUC) not “doing it wrong” any more?

Direct use was never "wrong" if you were doing anything other than
atomic64_set_release() and atomic64_read_acquire(), and I suspect we don't want
to see those abused as a way to get a 64-bit smp_store_release() or
smp_load_acquire() since those won't necessarily do the right thing w.r.t. a
plain READ_ONCE() and so on.

So I think this is still correct as-is.

Do you have a particular case in mind that you care about?

Thanks,
Mark.