Date:   Tue, 1 Mar 2022 18:58:02 +0100
From:   Greg KH <greg@...ah.com>
To:     Jakob Koschel <jakobkoschel@...il.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Christian König <christian.koenig@....com>,
        alsa-devel@...a-project.org, linux-aspeed@...ts.ozlabs.org,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        linux-iio@...r.kernel.org, nouveau@...ts.freedesktop.org,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Cristiano Giuffrida <c.giuffrida@...nl>,
        amd-gfx list <amd-gfx@...ts.freedesktop.org>,
        samba-technical@...ts.samba.org,
        linux1394-devel@...ts.sourceforge.net, drbd-dev@...ts.linbit.com,
        linux-arch <linux-arch@...r.kernel.org>,
        CIFS <linux-cifs@...r.kernel.org>,
        KVM list <kvm@...r.kernel.org>,
        linux-scsi <linux-scsi@...r.kernel.org>,
        linux-rdma <linux-rdma@...r.kernel.org>,
        linux-staging@...ts.linux.dev, "Bos, H.J." <h.j.bos@...nl>,
        Jason Gunthorpe <jgg@...pe.ca>,
        intel-wired-lan@...ts.osuosl.org,
        kgdb-bugreport@...ts.sourceforge.net,
        bcm-kernel-feedback-list@...adcom.com,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Kees Cook <keescook@...omium.org>,
        Arnd Bergman <arnd@...db.de>,
        Linux PM <linux-pm@...r.kernel.org>,
        intel-gfx <intel-gfx@...ts.freedesktop.org>,
        Brian Johannesmeyer <bjohannesmeyer@...il.com>,
        Nathan Chancellor <nathan@...nel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        v9fs-developer@...ts.sourceforge.net,
        linux-tegra <linux-tegra@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-sgx@...r.kernel.org,
        linux-block <linux-block@...r.kernel.org>,
        Netdev <netdev@...r.kernel.org>, linux-usb@...r.kernel.org,
        linux-wireless <linux-wireless@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux F2FS Dev Mailing List 
        <linux-f2fs-devel@...ts.sourceforge.net>,
        tipc-discussion@...ts.sourceforge.net,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        dma <dmaengine@...r.kernel.org>,
        linux-mediatek@...ts.infradead.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Mike Rapoport <rppt@...nel.org>
Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body
 as a ptr

On Tue, Mar 01, 2022 at 06:40:04PM +0100, Jakob Koschel wrote:
> 
> 
> > On 1. Mar 2022, at 18:36, Greg KH <greg@...ah.com> wrote:
> > 
> > On Tue, Mar 01, 2022 at 12:28:15PM +0100, Jakob Koschel wrote:
> >> 
> >> 
> >>> On 1. Mar 2022, at 01:41, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> >>> 
> >>> On Mon, Feb 28, 2022 at 1:47 PM Jakob Koschel <jakobkoschel@...il.com> wrote:
> >>>> 
> >>>> The goal of this is to get compiler warnings right? This would indeed be great.
> >>> 
> >>> Yes, so I don't mind having a one-time patch that has been gathered
> >>> using some automated checker tool, but I don't think that works from a
> >>> long-term maintenance perspective.
> >>> 
> >>> So if we have the basic rule being "don't use the loop iterator after
> >>> the loop has finished, because it can cause all kinds of subtle
> >>> issues", then in _addition_ to fixing the existing code paths that
> >>> have this issue, I really would want to (a) get a compiler warning for
> >>> future cases and (b) make it not actually _work_ for future cases.
> >>> 
> >>> Because otherwise it will just happen again.
> >>> 
> >>>> Changing the list_for_each_entry() macro first will break all of those cases
> >>>> (e.g. the ones using 'list_entry_is_head()).
> >>> 
> >>> So I have no problems with breaking cases that we basically already
> >>> have a patch for due to  your automated tool. There were certainly
> >>> more than a handful, but it didn't look _too_ bad to just make the
> >>> rule be "don't use the iterator after the loop".
> >>> 
> >>> Of course, that's just based on that patch of yours. Maybe there are a
> >>> ton of other cases that your patch didn't change, because they didn't
> >>> match your trigger case, so I may just be overly optimistic here.
> >> 
> >> Based on the coccinelle script there are ~480 cases that need fixing
> >> in total. I'll now finish all of them and then split them by
> >> submodules as Greg suggested and repost a patch set per submodule.
> >> Sounds good?
> > 
> > Sounds good to me!
> > 
> > If you need help carving these up and maintaining them over time as
> > different subsystem maintainers accept/ignore them, just let me know.
> > Doing large patchsets like this can be tough without a lot of
> > experience.
> 
> Very much appreciated!
> 
> There will probably be some cases that do not match one of the pattern
> we already discussed and need separate attention.
> 
> I was planning to start with one subsystem and adjust the coming ones
> according to the feedback gather there instead of posting all of them
> in one go.

That seems wise.  Feel free to use USB as a testing ground for this if
you want to :)

thanks,

greg k-h

Powered by blists - more mailing lists