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]

Message-ID: <Y7bK8drngH/NIlOa@phenom.ffwll.local>
Date:   Thu, 5 Jan 2023 14:04:49 +0100
From:   Daniel Vetter <daniel@...ll.ch>
To:     David Laight <David.Laight@...lab.com>
Cc:     'Andrzej Hajda' <andrzej.hajda@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        Arnd Bergmann <arnd@...db.de>,
        Andi Shyti <andi.shyti@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.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 Mon, Dec 12, 2022 at 09:38:12AM +0000, David Laight wrote:
> From: Andrzej Hajda <andrzej.hajda@...el.com>
> > Sent: 09 December 2022 15:49
> > 
> > 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.
> 
> Dunno, if it is non-atomic then two separate assignment statements
> is decidedly more obvious and needs less brain cells to process.
> Otherwise someone will assume 'something clever' is going on
> and the operation is atomic.

Yes, this also my take. The i915 code that uses this to excess is decidely
unreadable imo, and the macro should simply be replaced by open-coded
versions.

Not moved into shared headers where even more people can play funny games
with it.

I think swap() is a standard idiom in C, this one here just isn't.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch