All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Stephen Boyd <sboyd@kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Anna-Maria Gleixner <anna-maria@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Julia Lawall <Julia.Lawall@inria.fr>
Subject: Re: [PATCH v4a 00/38] timers: Use timer_shutdown*() before freeing timers
Date: Sat, 5 Nov 2022 14:43:03 -0400	[thread overview]
Message-ID: <20221105144303.3552bf85@rorschach.local.home> (raw)
In-Reply-To: <CAHk-=wjnASLkTdPd+wxto2RBQH+S9MUm4FrNPWvU87opFG5SKQ@mail.gmail.com>

On Sat, 5 Nov 2022 11:28:33 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sat, Nov 5, 2022 at 11:04 AM Steven Rostedt <rostedt@goodmis.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?

The first one was a false change. That is, the script *did* catch it,
when it should not have. So I reverted the change. The coccinelle
documentation even states to look over the changes to see if there are
false positives.

The second change is that it frees three timers all for the same
object. If you want, I could run the script 2 more times on the same
file, and it will catch it then.

Would you be happier if I just ran it three times on that file? I can do
that, and it will produce the same result.

> 
> 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".

Of the two clean ups, one was a false positive, so I had to revert it.
The other, just needs me to run the script more than once. I can do
that, and then I only have the false positive case to clean up.

> 
> 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

I was told to make sure the cover letter had all the required mailing lists :-p

I removed them for this 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.

OK, I'll wait on Julia for an answer on this.

-- Steve

  reply	other threads:[~2022-11-05 18:43 UTC|newest]

Thread overview: 170+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-05  6:00 [PATCH v4a 00/38] timers: Use timer_shutdown*() before freeing timers Steven Rostedt
2022-11-05  6:00 ` [Bridge] " Steven Rostedt
2022-11-05  6:00 ` Steven Rostedt
2022-11-05  6:00 ` [Intel-wired-lan] " Steven Rostedt
2022-11-05  6:00 ` [Intel-gfx] " Steven Rostedt
2022-11-05  6:00 ` Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 01/38] SUNRPC/xprt: Use del_timer_sync() instead of del_singleshot_timer_sync() Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 02/38] ARM: spear: Do not use timer namespace for timer_shutdown() function Steven Rostedt
2022-11-05  6:00   ` Steven Rostedt
2022-11-05 14:05   ` Guenter Roeck
2022-11-05 14:05     ` Guenter Roeck
2022-11-07  5:46     ` Viresh Kumar
2022-11-07  5:46       ` Viresh Kumar
2022-11-05 18:12   ` Arnd Bergmann
2022-11-05 18:12     ` Arnd Bergmann
2022-11-05  6:00 ` [PATCH v4a 03/38] clocksource/drivers/arm_arch_timer: " Steven Rostedt
2022-11-05  6:00   ` Steven Rostedt
2022-11-05  9:38   ` Marc Zyngier
2022-11-05  9:38     ` Marc Zyngier
2022-11-05 14:07   ` Guenter Roeck
2022-11-05 14:07     ` Guenter Roeck
2022-11-05 14:42     ` Steven Rostedt
2022-11-05 14:42       ` Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 04/38] clocksource/drivers/sp804: " Steven Rostedt
2022-11-05 14:06   ` Guenter Roeck
2022-11-05  6:00 ` [PATCH v4a 05/38] timers: Add timer_shutdown_sync() and timer_shutdown() to be called before freeing timers Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 06/38] timers: sh: Use timer_shutdown_sync() before freeing timer Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 07/38] timers: block: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 08/38] timers: ACPI: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 09/38] timers: atm: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 10/38] timers: Bluetooth: " Steven Rostedt
2022-11-05  7:14   ` timers: Use timer_shutdown*() before freeing timers bluez.test.bot
2022-11-18  3:47   ` bluez.test.bot
2022-11-18  4:33   ` bluez.test.bot
2022-11-18  5:32   ` bluez.test.bot
2022-11-18  6:34   ` bluez.test.bot
2022-11-18  7:33   ` bluez.test.bot
2022-11-18  8:33   ` bluez.test.bot
2022-11-18  9:30   ` bluez.test.bot
2022-11-19  3:56   ` bluez.test.bot
2022-11-19  5:03   ` bluez.test.bot
2022-11-05  6:00 ` [PATCH v4a 11/38] timers: drm: Use timer_shutdown_sync() before freeing timer Steven Rostedt
2022-11-05  6:00   ` [Intel-gfx] " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 12/38] timers: HID: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 14/38] timers: mISDN: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 15/38] timers: leds: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 16/38] timers: media: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 17/38] timers: net: " Steven Rostedt
2022-11-05  6:00   ` [Bridge] " Steven Rostedt
2022-11-05  6:00   ` Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 18/38] timers: usb: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 19/38] timers: nfc: pn533: " Steven Rostedt
2022-11-07 10:13   ` Krzysztof Kozlowski
2022-11-05  6:00 ` [PATCH v4a 20/38] timers: pcmcia: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 21/38] timers: scsi: Use timer_shutdown_sync() and timer_shutdown() " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 22/38] timers: tty: Use timer_shutdown_sync() " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 23/38] timers: ext4: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 24/38] timers: fs/nilfs2: " Steven Rostedt
2022-11-05  6:00   ` Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 25/38] timers: ALSA: " Steven Rostedt
2022-11-05  6:00   ` Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 26/38] timers: jbd2: Use timer_shutdown() " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 27/38] timers: sched/psi: Use timer_shutdown_sync() " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 28/38] timers: workqueue: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 29/38] random: use timer_shutdown_sync() for on stack timers Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 30/38] timers: dma-buf: Use " Steven Rostedt
2022-11-07 14:12   ` Christian König
2022-11-07 14:12     ` Christian König
2022-11-05  6:00 ` [PATCH v4a 31/38] timers: drm: " Steven Rostedt
2022-11-05  6:00   ` [Intel-gfx] " Steven Rostedt
2022-11-05  9:54   ` Noralf Trønnes
2022-11-05  9:54     ` [Intel-gfx] " Noralf Trønnes
2022-11-05  9:54     ` Noralf Trønnes
2022-11-05  6:00 ` [PATCH v4a 32/38] timers: media: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 33/38] timers: s390/cmm: Use timer_shutdown_sync() before a module is released Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 34/38] timers: atm: " Steven Rostedt
2022-11-05  6:00 ` [PATCH v4a 35/38] timers: hangcheck: " Steven Rostedt
2022-11-05  6:01 ` [PATCH v4a 36/38] timers: ipmi: " Steven Rostedt
2022-11-05  6:01 ` [PATCH v4a 38/38] timers: PM: Use timer_shutdown_sync() Steven Rostedt
2022-11-05 14:18 ` [PATCH v4a 00/38] timers: Use timer_shutdown*() before freeing timers Guenter Roeck
2022-11-05 14:18   ` [Bridge] " Guenter Roeck
2022-11-05 14:18   ` Guenter Roeck
2022-11-05 14:18   ` [Intel-wired-lan] " Guenter Roeck
2022-11-05 14:18   ` [Intel-gfx] " Guenter Roeck
2022-11-05 14:18   ` Guenter Roeck
2022-11-05 14:47   ` Steven Rostedt
2022-11-05 14:47     ` [Bridge] " Steven Rostedt
2022-11-05 14:47     ` Steven Rostedt
2022-11-05 14:47     ` [Intel-wired-lan] " Steven Rostedt
2022-11-05 14:47     ` [Intel-gfx] " Steven Rostedt
2022-11-05 14:47     ` Steven Rostedt
2022-11-05 15:59 ` Linus Torvalds
2022-11-05 15:59   ` [Bridge] " Linus Torvalds
2022-11-05 15:59   ` Linus Torvalds
2022-11-05 15:59   ` Linus Torvalds
2022-11-05 15:59   ` [Intel-wired-lan] " Linus Torvalds
2022-11-05 15:59   ` [Intel-gfx] " Linus Torvalds
2022-11-05 16:36   ` Steven Rostedt
2022-11-05 16:36     ` [Bridge] " Steven Rostedt
2022-11-05 16:36     ` Steven Rostedt
2022-11-05 16:36     ` [Intel-wired-lan] " Steven Rostedt
2022-11-05 16:36     ` [Intel-gfx] " Steven Rostedt
2022-11-05 16:36     ` Steven Rostedt
2022-11-05 16:53     ` Steven Rostedt
2022-11-05 16:53       ` [Bridge] " Steven Rostedt
2022-11-05 16:53       ` Steven Rostedt
2022-11-05 16:53       ` [Intel-wired-lan] " Steven Rostedt
2022-11-05 16:53       ` [Intel-gfx] " Steven Rostedt
2022-11-05 16:53       ` Steven Rostedt
2022-11-05 18:03     ` Steven Rostedt
2022-11-05 18:03       ` [Bridge] " Steven Rostedt
2022-11-05 18:03       ` Steven Rostedt
2022-11-05 18:03       ` [Intel-wired-lan] " Steven Rostedt
2022-11-05 18:03       ` [Intel-gfx] " Steven Rostedt
2022-11-05 18:03       ` Steven Rostedt
2022-11-05 18:05       ` Steven Rostedt
2022-11-05 18:05         ` [Bridge] " Steven Rostedt
2022-11-05 18:05         ` Steven Rostedt
2022-11-05 18:05         ` [Intel-wired-lan] " Steven Rostedt
2022-11-05 18:05         ` [Intel-gfx] " Steven Rostedt
2022-11-05 18:05         ` Steven Rostedt
2022-11-05 18:28       ` Linus Torvalds
2022-11-05 18:28         ` [Bridge] " Linus Torvalds
2022-11-05 18:28         ` Linus Torvalds
2022-11-05 18:28         ` Linus Torvalds
2022-11-05 18:28         ` [Intel-wired-lan] " Linus Torvalds
2022-11-05 18:28         ` [Intel-gfx] " Linus Torvalds
2022-11-05 18:43         ` Steven Rostedt [this message]
2022-11-05 23:08           ` Julia Lawall
2022-11-06  0:36             ` Steven Rostedt
2022-11-06  2:38               ` Julia Lawall
2022-11-06  3:05               ` Julia Lawall
2022-11-06  3:12                 ` Steven Rostedt
2022-11-06  3:15                   ` Julia Lawall
2022-11-05 21:03     ` Jason A. Donenfeld
2022-11-05 21:03       ` [Bridge] " Jason A. Donenfeld
2022-11-05 21:03       ` Jason A. Donenfeld
2022-11-05 21:03       ` [Intel-wired-lan] " Jason A. Donenfeld
2022-11-05 21:03       ` [Intel-gfx] " Jason A. Donenfeld
2022-11-05 21:03       ` Jason A. Donenfeld
2022-11-05 21:13       ` Linus Torvalds
2022-11-05 21:13         ` [Bridge] " Linus Torvalds
2022-11-05 21:13         ` Linus Torvalds
2022-11-05 21:13         ` [Intel-gfx] " Linus Torvalds
2022-11-05 21:13         ` [Intel-wired-lan] " Linus Torvalds
2022-11-05 21:13         ` Linus Torvalds
2022-11-05 21:45         ` Steven Rostedt
2022-11-05 21:45           ` [Bridge] " Steven Rostedt
2022-11-05 21:45           ` Steven Rostedt
2022-11-05 21:45           ` [Intel-wired-lan] " Steven Rostedt
2022-11-05 21:45           ` [Intel-gfx] " Steven Rostedt
2022-11-05 21:45           ` Steven Rostedt
2022-11-05 21:47         ` Steven Rostedt
2022-11-05 21:47           ` [Bridge] " Steven Rostedt
2022-11-05 21:47           ` Steven Rostedt
2022-11-05 21:47           ` [Intel-wired-lan] " Steven Rostedt
2022-11-05 21:47           ` [Intel-gfx] " Steven Rostedt
2022-11-05 21:47           ` Steven Rostedt
2022-11-05 17:53   ` Steven Rostedt
2022-11-05 17:53     ` [Bridge] " Steven Rostedt
2022-11-05 17:53     ` Steven Rostedt
2022-11-05 17:53     ` [Intel-wired-lan] " Steven Rostedt
2022-11-05 17:53     ` [Intel-gfx] " Steven Rostedt
2022-11-05 17:53     ` Steven Rostedt
2022-11-05 19:31 ` Guenter Roeck
2022-11-05 19:31   ` [Bridge] " Guenter Roeck
2022-11-05 19:31   ` Guenter Roeck
2022-11-05 19:31   ` [Intel-wired-lan] " Guenter Roeck
2022-11-05 19:31   ` [Intel-gfx] " Guenter Roeck
2022-11-05 19:31   ` Guenter Roeck

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=20221105144303.3552bf85@rorschach.local.home \
    --to=rostedt@goodmis.org \
    --cc=Julia.Lawall@inria.fr \
    --cc=akpm@linux-foundation.org \
    --cc=anna-maria@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=sboyd@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.