From: David Laight <David.Laight@ACULAB.COM>
To: 'Linus Torvalds' <torvalds@linux-foundation.org>,
James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
"nouveau@lists.freedesktop.org" <nouveau@lists.freedesktop.org>,
"Rasmus Villemoes" <linux@rasmusvillemoes.dk>,
dri-devel <dri-devel@lists.freedesktop.org>,
"Cristiano Giuffrida" <c.giuffrida@vu.nl>,
"Bos, H.J." <h.j.bos@vu.nl>,
"samba-technical@lists.samba.org"
<samba-technical@lists.samba.org>,
"linux1394-devel@lists.sourceforge.net"
<linux1394-devel@lists.sourceforge.net>,
"drbd-dev@lists.linbit.com" <drbd-dev@lists.linbit.com>,
linux-arch <linux-arch@vger.kernel.org>,
CIFS <linux-cifs@vger.kernel.org>,
"KVM list" <kvm@vger.kernel.org>,
linux-scsi <linux-scsi@vger.kernel.org>,
linux-rdma <linux-rdma@vger.kernel.org>,
"linux-staging@lists.linux.dev" <linux-staging@lists.linux.dev>,
"amd-gfx list" <amd-gfx@lists.freedesktop.org>,
"Jason Gunthorpe" <jgg@ziepe.ca>,
"intel-wired-lan@lists.osuosl.org"
<intel-wired-lan@lists.osuosl.org>,
"kgdb-bugreport@lists.sourceforge.net"
<kgdb-bugreport@lists.sourceforge.net>,
"bcm-kernel-feedback-list@broadcom.com"
<bcm-kernel-feedback-list@broadcom.com>,
"Dan Carpenter" <dan.carpenter@oracle.com>,
"Linux Media Mailing List" <linux-media@vger.kernel.org>,
"Kees Cook" <keescook@chromium.org>,
"Arnd Bergman" <arnd@arndb.de>,
"Linux PM" <linux-pm@vger.kernel.org>,
intel-gfx <intel-gfx@lists.freedesktop.org>,
"Brian Johannesmeyer" <bjohannesmeyer@gmail.com>,
"Nathan Chancellor" <nathan@kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
"Christophe JAILLET" <christophe.jaillet@wanadoo.fr>,
"Jakob Koschel" <jakobkoschel@gmail.com>,
"v9fs-developer@lists.sourceforge.net"
<v9fs-developer@lists.sourceforge.net>,
linux-tegra <linux-tegra@vger.kernel.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
"linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
linux-block <linux-block@vger.kernel.org>,
Netdev <netdev@vger.kernel.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
linux-wireless <linux-wireless@vger.kernel.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Linux F2FS Dev Mailing List"
<linux-f2fs-devel@lists.sourceforge.net>,
"tipc-discussion@lists.sourceforge.net"
<tipc-discussion@lists.sourceforge.net>,
"Linux Crypto Mailing List" <linux-crypto@vger.kernel.org>,
dma <dmaengine@vger.kernel.org>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"Andrew Morton" <akpm@linux-foundation.org>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
"Christian König" <christian.koenig@amd.com>,
"Mike Rapoport" <rppt@kernel.org>
Subject: RE: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
Date: Tue, 1 Mar 2022 22:58:11 +0000 [thread overview]
Message-ID: <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> (raw)
In-Reply-To: <CAHk-=wiT5HX6Kp0Qv4ZYK_rkq9t5fZ5zZ7vzvi6pub9kgp=72g@mail.gmail.com>
From: Linus Torvalds
> Sent: 01 March 2022 19:07
> On Mon, Feb 28, 2022 at 2:29 PM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >
> > However, if the desire is really to poison the loop variable then we
> > can do
> >
> > #define list_for_each_entry(pos, head, member) \
> > for (pos = list_first_entry(head, typeof(*pos), member); \
> > !list_entry_is_head(pos, head, member) && ((pos = NULL) == NULL; \
> > pos = list_next_entry(pos, member))
> >
> > Which would at least set pos to NULL when the loop completes.
>
> That would actually have been excellent if we had done that
> originally. It would not only avoid the stale and incorrectly typed
> head entry left-over turd, it would also have made it very easy to
> test for "did I find an entry in the loop".
>
> But I don't much like it in the situation we are now.
>
> Why? Mainly because it basically changes the semantics of the loop
> _without_ any warnings about it. And we don't actually get the
> advantage of the nicer semantics, because we can't actually make code
> do
>
> list_for_each_entry(entry, ....) {
> ..
> }
> if (!entry)
> return -ESRCH;
> .. use the entry we found ..
>
> because that would be a disaster for back-porting, plus it would be a
> flag-day issue (ie we'd have to change the semantics of the loop at
> the same time we change every single user).
>
> So instead of that simple "if (!entry)", we'd effectively have to
> continue to use something that still works with the old world order
> (ie that "if (list_entry_is_head())" model).
>
> So we couldn't really take _advantage_ of the nicer semantics, and
> we'd not even get a warning if somebody does it wrong - the code would
> just silently do the wrong thing.
>
> IOW: I don't think you are wrong about that patch: it would solve the
> problem that Jakob wants to solve, and it would have absolutely been
> much better if we had done this from the beginning. But I think that
> in our current situation, it's actually a really fragile solution to
> the "don't do that then" problem we have.
Can it be resolved by making:
#define list_entry_is_head(pos, head, member) ((pos) == NULL)
and double-checking that it isn't used anywhere else (except in
the list macros themselves).
The odd ones I just found are fs/locks.c mm/page_reporting.c
security/apparmor/apparmorfs.c (3 times)
net/xfrm/xfrm_ipcomp.c#L244 is buggy.
(There is a WARN_ON() then it just carries on regardless!)
There are only about 25 uses of list_entry_is_head().
There are a lot more places where these lists seem to be scanned by hand.
I bet a few of those aren't actually right either.
(Oh at 3am this morning I thought it was a different list type
that could have much the same problem!)
Another plausible solution is a variant of list_foreach_entry()
that does set the 'entry' to NULL at the end.
Then code can be moved over in stages.
I'd reorder the arguments as well as changing the name!
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2022-03-01 22:58 UTC|newest]
Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-28 11:08 [PATCH 0/6] Remove usage of list iterator past the loop body Jakob Koschel
2022-02-28 11:08 ` [PATCH 1/6] drivers: usb: remove " Jakob Koschel
2022-02-28 11:24 ` Dan Carpenter
2022-02-28 12:03 ` Jakob Koschel
2022-02-28 13:18 ` Dan Carpenter
2022-02-28 18:20 ` Joe Perches
2022-03-01 5:52 ` Dan Carpenter
2022-02-28 11:08 ` [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr Jakob Koschel
2022-02-28 11:20 ` Greg KH
2022-02-28 12:06 ` Jakob Koschel
2022-03-01 17:37 ` Greg KH
2022-02-28 12:19 ` Christian König
2022-02-28 19:56 ` Linus Torvalds
2022-02-28 20:03 ` Linus Torvalds
2022-02-28 20:10 ` Linus Torvalds
2022-02-28 20:14 ` Linus Torvalds
2022-02-28 20:53 ` Segher Boessenkool
2022-02-28 20:16 ` Matthew Wilcox
2022-02-28 20:27 ` Johannes Berg
2022-02-28 20:41 ` Linus Torvalds
2022-02-28 20:37 ` Linus Torvalds
2022-02-28 23:26 ` Matthew Wilcox
2022-03-01 0:45 ` Linus Torvalds
2022-03-01 0:57 ` Linus Torvalds
2022-03-01 18:14 ` Kees Cook
2022-03-01 18:47 ` Linus Torvalds
2022-03-01 19:01 ` Matthew Wilcox
2022-03-01 3:03 ` David Laight
2022-02-28 21:47 ` Jakob Koschel
2022-03-01 0:41 ` Linus Torvalds
2022-03-01 6:32 ` Jakub Kicinski
2022-03-01 11:28 ` Jakob Koschel
2022-03-01 17:36 ` Greg KH
2022-03-01 17:40 ` Jakob Koschel
2022-03-01 17:58 ` Greg KH
2022-03-01 18:21 ` Kees Cook
2022-03-02 9:31 ` Xiaomeng Tong
2022-03-02 14:04 ` David Laight
2022-03-03 2:27 ` Xiaomeng Tong
2022-03-03 4:58 ` David Laight
2022-03-03 7:26 ` Xiaomeng Tong
2022-03-03 9:30 ` David Laight
2022-03-03 12:37 ` Xiaomeng Tong
2022-03-03 12:18 ` [Kgdb-bugreport] " Daniel Thompson
2022-03-04 6:59 ` Xiaomeng Tong
2022-03-03 7:32 ` Jakob Koschel
2022-03-03 8:30 ` Xiaomeng Tong
2022-03-03 8:38 ` Xiaomeng Tong
2022-02-28 20:07 ` Christian König
2022-02-28 20:42 ` James Bottomley
2022-02-28 20:56 ` Christian König
2022-02-28 21:13 ` James Bottomley
2022-03-01 7:03 ` Christian König
2022-02-28 22:05 ` Jakob Koschel
2022-02-28 21:18 ` Jeffrey Walton
2022-02-28 21:59 ` Mike Rapoport
2022-02-28 22:28 ` James Bottomley
2022-02-28 22:50 ` Barnabás Pőcze
2022-03-01 0:30 ` Segher Boessenkool
2022-03-01 0:54 ` Linus Torvalds
2022-03-01 19:06 ` Linus Torvalds
2022-03-01 19:42 ` Linus Torvalds
2022-03-01 22:58 ` David Laight [this message]
2022-03-01 23:03 ` Linus Torvalds
2022-03-01 23:19 ` David Laight
2022-03-01 23:55 ` Linus Torvalds
2022-03-02 9:29 ` Rasmus Villemoes
2022-03-02 20:07 ` Kees Cook
2022-03-02 20:18 ` Linus Torvalds
2022-03-02 20:59 ` Kees Cook
2022-03-03 8:37 ` Dan Carpenter
2022-03-03 10:56 ` Dan Carpenter
2022-03-01 2:15 ` David Laight
2022-02-28 13:13 ` Dan Carpenter
2022-02-28 11:08 ` [PATCH 3/6] treewide: fix incorrect use to determine if list is empty Jakob Koschel
2022-02-28 11:38 ` Dan Carpenter
2022-02-28 11:08 ` [PATCH 4/6] drivers: remove unnecessary use of list iterator variable Jakob Koschel
2022-02-28 11:08 ` [PATCH 5/6] treewide: remove dereference of list iterator after loop body Jakob Koschel
2022-02-28 11:08 ` [PATCH 6/6] treewide: remove check of list iterator against head past the " Jakob Koschel
2022-02-28 11:22 ` Dominique Martinet
2022-02-28 13:12 ` Dan Carpenter
2022-03-01 20:36 ` Linus Torvalds
2022-03-02 17:14 ` [Intel-gfx] " Tvrtko Ursulin
2022-03-07 15:00 ` [PATCH 0/6] Remove usage of list iterator " Dan Carpenter
2022-03-07 15:26 ` David Laight
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=7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=akpm@linux-foundation.org \
--cc=alsa-devel@alsa-project.org \
--cc=amd-gfx@lists.freedesktop.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=arnd@arndb.de \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=bjohannesmeyer@gmail.com \
--cc=c.giuffrida@vu.nl \
--cc=christian.koenig@amd.com \
--cc=christophe.jaillet@wanadoo.fr \
--cc=dan.carpenter@oracle.com \
--cc=dmaengine@vger.kernel.org \
--cc=drbd-dev@lists.linbit.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gustavo@embeddedor.com \
--cc=h.j.bos@vu.nl \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jakobkoschel@gmail.com \
--cc=jgg@ziepe.ca \
--cc=keescook@chromium.org \
--cc=kgdb-bugreport@lists.sourceforge.net \
--cc=kvm@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=linux-sgx@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=linux-tegra@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linux1394-devel@lists.sourceforge.net \
--cc=linux@rasmusvillemoes.dk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=nathan@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=rppt@kernel.org \
--cc=samba-technical@lists.samba.org \
--cc=tglx@linutronix.de \
--cc=tipc-discussion@lists.sourceforge.net \
--cc=torvalds@linux-foundation.org \
--cc=v9fs-developer@lists.sourceforge.net \
/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;
as well as URLs for NNTP newsgroup(s).