linux-kernel - Re: [PATCH v2 (RESEND) 2/2] riscv: atomic: Using ARCH

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a1ygwS7jTXqYXCfppEEonCASqG_5GM9O_AtE9YgdgNqVQ@mail.gmail.com>
Date:   Mon, 19 Apr 2021 13:45:36 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Guo Ren <guoren@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-csky@...r.kernel.org,
        linux-arch <linux-arch@...r.kernel.org>,
        Guo Ren <guoren@...ux.alibaba.com>,
        Anup Patel <anup@...infault.org>,
        Palmer Dabbelt <palmerdabbelt@...gle.com>
Subject: Re: [PATCH v2 (RESEND) 2/2] riscv: atomic: Using ARCH_ATOMIC in asm/atomic.h

On Sat, Apr 17, 2021 at 6:45 AM <guoren@...nel.org> 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