Re: [PATCH v2 (RESEND) 2/2] riscv: atomic: Using ARCH_ATOMIC in asm/atomic.h

From: Arnd Bergmann
Date: Mon Apr 19 2021 - 07:46:06 EST


On Sat, Apr 17, 2021 at 6:45 AM <guoren@xxxxxxxxxx> wrote:
> +#define arch_atomic_read(v) __READ_ONCE((v)->counter)
> +#define arch_atomic_set(v, i) __WRITE_ONCE(((v)->counter), (i))

> +#define ATOMIC64_INIT ATOMIC_INIT
> +#define arch_atomic64_read arch_atomic_read
> +#define arch_atomic64_set arch_atomic_set
> #endif

I think it's a bit confusing to define arch_atomic64_read() etc in terms
of arch_atomic_read(), given that they operate on different types.

IMHO the clearest would be to define both in terms of the open-coded
version you have for the 32-bit atomics.

Also, given that all three architectures (x86, arm64, riscv) use the same
definitions for the six macros above, maybe those can just get moved
into a common file with a possible override?

x86 uses an inline function here instead of the macro. This would also
be my preference, but it may add complexity to avoid circular header
dependencies.

The rest of this patch looks good to me.

Arnd