Date:   Sat, 5 Nov 2022 11:28:33 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Anna-Maria Gleixner <anna-maria@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>, rcu@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-s390@...r.kernel.org,
        linux-sh@...r.kernel.org, linux-edac@...r.kernel.org,
        cgroups@...r.kernel.org, linux-block@...r.kernel.org,
        linux-acpi@...r.kernel.org,
        linux-atm-general@...ts.sourceforge.net, netdev@...r.kernel.org,
        linux-pm@...r.kernel.org, drbd-dev@...ts.linbit.com,
        linux-bluetooth@...r.kernel.org,
        openipmi-developer@...ts.sourceforge.net,
        linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linaro-mm-sig@...ts.linaro.org, intel-gfx@...ts.freedesktop.org,
        linux-input@...r.kernel.org, linux-parisc@...r.kernel.org,
        linux-leds@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
        linux-usb@...r.kernel.org, linux-wireless@...r.kernel.org,
        linux-scsi@...r.kernel.org, linux-staging@...ts.linux.dev,
        linux-ext4@...r.kernel.org, linux-nilfs@...r.kernel.org,
        bridge@...ts.linux-foundation.org, netfilter-devel@...r.kernel.org,
        coreteam@...filter.org, lvs-devel@...r.kernel.org,
        linux-afs@...ts.infradead.org, linux-nfs@...r.kernel.org,
        tipc-discussion@...ts.sourceforge.net, alsa-devel@...a-project.org,
        Julia Lawall <Julia.Lawall@...ia.fr>
Subject: Re: [PATCH v4a 00/38] timers: Use timer_shutdown*() before freeing timers

On Sat, Nov 5, 2022 at 11:04 AM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> Here's the changes I made after running the script

Please. No.

What part of "I don't want extra crud" was I unclear on?

I'm not interested in converting everything. That's clearly a 6.,2
issue, possibly even longer considering how complicated the networking
side has been.

I'm not AT ALL interested in "oh, I then added my own small cleanups
on top to random files because I happened to notice them".

Repeat after me: "If the script didn't catch them, they weren't
trivially obvious".

And it does seem that right now the script itself is a bit too
generous, which is why it didn't notice that sometimes there wasn't a
kfree after all because of a goto around it. So clearly that "..."
doesn't really work, I think it accepts "_any_ path leads to the
second situation" rather than "_all_ paths lead to the second
situation".

But yeah, my coccinelle-foo is very weak too, and maybe there's no
pattern for "no flow control".

I would also like the coccinelle script to notice the "timer is used
afterwards", so that it does *not* modify that case that does

                del_timer(&dch->timer);
                dch->timer.function = NULL;

since now the timer is modified in between the del_timer() and the kfree.

Again, that timer modification is then made pointless by changing the
del_timer() to a "timer_shutdown()", but at that point it is no longer
a "so obvious non-semantic change that it should be scripted". At that
point it's a manual thing.

So I think the "..." in your script should be "no flow control, and no
access to the timer", but do not know how to do that in coccinelle.

Julia?

And this thread has way too many participants, I suspect some email
systems will just mark it as spam as a result. Which is partly *why* I
would like to get rid of noisy changes that really don't matter - but
I would like it to be truly mindlessly obvious that there are *zero*
questions about it, and absolutely no manual intervention because the
patch is so strict that it's just unquestionably correct.

              Linus

Powered by blists - more mailing lists