linux-kernel - Re: [PATCH 1/5] linux/minmax.h: add non-atomic version of xchg

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]

Date:   Fri, 09 Dec 2022 18:16:25 +0100
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Andrzej Hajda" <andrzej.hajda@...el.com>,
        linux-kernel@...r.kernel.org, intel-gfx@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org
Cc:     "Jani Nikula" <jani.nikula@...ux.intel.com>,
        "Joonas Lahtinen" <joonas.lahtinen@...ux.intel.com>,
        "Rodrigo Vivi" <rodrigo.vivi@...el.com>,
        "Tvrtko Ursulin" <tvrtko.ursulin@...ux.intel.com>,
        "Andi Shyti" <andi.shyti@...ux.intel.com>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        "Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH 1/5] linux/minmax.h: add non-atomic version of xchg

On Fri, Dec 9, 2022, at 16:48, Andrzej Hajda wrote:
> The pattern of setting variable with new value and returning old
> one is very common in kernel. Usually atomicity of the operation
> is not required, so xchg seems to be suboptimal and confusing in
> such cases. Since name xchg is already in use and __xchg is used
> in architecture code, proposition is to name the macro exchange.
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@...el.com>

While I generally don't like type invariant calling conventions
of xchg() and cmpxchg(), having a new function that has a similar
name without being able to tell which one is which from the
name seems more confusing.

Since __xchg() is only used on 11 architectures as an internal
name for the backing of arch_xchg() or arch_xchg_relaxed(),
maybe we can instead rename those to __arch_xchg() and use the
__xchg() name for the new non-atomic version?

> +/**
> + * exchange - set variable pointed by @ptr to @val, return old value
> + * @ptr: pointer to affected variable
> + * @val: value to be written
> + *
> + * This is non-atomic variant of xchg.
> + */
> +#define exchange(ptr, val) ({		\
> +	typeof(ptr) __ptr = ptr;	\
> +	typeof(*__ptr) __t = *__ptr;	\

I think you can better express this using __auto_type than typeof(),
it is now provided by all supported compilers now.

     Arnd