Re: [RFC][PATCH v2 00/31] timers: Use del_timer_shutdown() before freeing timers
From: Guenter Roeck
Date: Thu Oct 27 2022 - 19:25:03 EST
On Thu, Oct 27, 2022 at 06:58:59PM -0400, Steven Rostedt wrote:
> On Thu, 27 Oct 2022 15:24:04 -0700
> Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> > On Thu, Oct 27, 2022 at 11:05:25AM -0400, Steven Rostedt wrote:
> > >
> > > Back in April, I posted an RFC patch set to help mitigate a common issue
> > > where a timer gets armed just before it is freed, and when the timer
> > > goes off, it crashes in the timer code without any evidence of who the
> > > culprit was. I got side tracked and never finished up on that patch set.
> > > Since this type of crash is still our #1 crash we are seeing in the field,
> > > it has become a priority again to finish it.
> > >
> > > This is v2 of that patch set. Thomas Gleixner posted an untested version
> > > that makes timer->function NULL as the flag that it is shutdown. I took that
> > > code, tested it (fixed it up), added more comments, and changed the
> > > name to del_timer_shutdown() as Linus had asked. I also converted it to use
> > > WARN_ON_ONCE() instead of just WARN_ON() as Linus asked for that too.
> > >
> >
> > Here are various warnings and crashes. Complete logs are at
> >
> > https://kerneltests.org/builders
> >
> > in the "testing" column of the qemu test results.
> >
> > This is with the published patch set plus the fixups in
> > timer_fixup_init() and timer_fixup_free().
> >
> > Guenter
> >
> > ---
> > Block code:
> >
> > WARNING: CPU: 0 PID: 8 at lib/debugobjects.c:502 debug_print_object+0x66/0x7a
> > ODEBUG: free active (active state 0) object type: timer_list hint: blk_rq_timed_out_timer+0x0/0xc
>
> This is probably just missing a switch to shutdown.
>
Wild shot, and I don't really know what I am doing,
but I'll try
iff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index e71b3b43927c..6c1cb4a219f0 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -767,6 +767,8 @@ static void blk_release_queue(struct kobject *kobj)
might_sleep();
+ del_timer_shutdown(&q->timeout);
+
percpu_ref_exit(&q->q_usage_counter);
if (q->poll_stat)
Guenter