From: Daniel Vetter <daniel@ffwll.ch>
To: David Laight <David.Laight@aculab.com>
Cc: 'Andrzej Hajda' <andrzej.hajda@intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
Arnd Bergmann <arnd@arndb.de>,
Andi Shyti <andi.shyti@linux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>,
Andrew Morton <akpm@linux-foundation.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH 1/5] linux/minmax.h: add non-atomic version of xchg
Date: Thu, 5 Jan 2023 14:04:49 +0100 [thread overview]
Message-ID: <Y7bK8drngH/NIlOa@phenom.ffwll.local> (raw)
In-Reply-To: <f58ab17e5c6c4a4e8b0d687b44618c51@AcuMS.aculab.com>
On Mon, Dec 12, 2022 at 09:38:12AM +0000, David Laight wrote:
> From: Andrzej Hajda <andrzej.hajda@intel.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
next prev parent reply other threads:[~2023-01-05 13:05 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-09 15:48 Andrzej Hajda
2022-12-09 15:48 ` [PATCH 2/5] drm/i915/display: kill fetch_and_zero usage Andrzej Hajda
2022-12-09 15:48 ` [PATCH 3/5] drm/i915/gt: " Andrzej Hajda
2022-12-12 9:14 ` [Intel-gfx] " Upadhyay, Tejas
2022-12-12 9:23 ` Andrzej Hajda
2022-12-09 15:48 ` [PATCH 4/5] drm/i915/gvt: " Andrzej Hajda
2022-12-09 15:48 ` [PATCH 5/5] drm/i915: kill fetch_and_zero Andrzej Hajda
2022-12-09 17:16 ` [PATCH 1/5] linux/minmax.h: add non-atomic version of xchg Arnd Bergmann
2022-12-13 9:28 ` [Intel-gfx] " Andrzej Hajda
2022-12-13 9:35 ` Arnd Bergmann
2022-12-09 18:56 ` Andy Shevchenko
2022-12-09 18:58 ` Andy Shevchenko
2022-12-13 10:09 ` [Intel-gfx] " Andrzej Hajda
2022-12-13 10:27 ` Andy Shevchenko
2022-12-12 9:38 ` David Laight
2023-01-05 13:04 ` Daniel Vetter [this message]
2023-01-05 13:28 ` [Intel-gfx] " Jani Nikula
2023-01-05 13:34 ` David Laight
2023-01-05 14:13 ` Daniel Vetter
2023-01-05 14:41 ` David Laight
2023-01-05 14:57 ` Daniel Vetter
2023-01-05 16:16 ` Mark Rutland
2023-01-05 14:13 ` Tvrtko Ursulin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y7bK8drngH/NIlOa@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=David.Laight@aculab.com \
--cc=akpm@linux-foundation.org \
--cc=andi.shyti@linux.intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=andrzej.hajda@intel.com \
--cc=arnd@arndb.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rodrigo.vivi@intel.com \
--cc=tvrtko.ursulin@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®