Messages in this thread | | | Date | Thu, 7 Apr 2022 18:51:44 -0400 | From | Steven Rostedt <> | Subject | Re: [RFC][PATCH] timers: Add del_time_free() to be called before freeing timers |
| |
On Thu, 7 Apr 2022 14:58:02 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
> > Add a del_timer_free() that not only does a del_timer_sync() but will mark > > This limits the use case to situations where del_timer_sync() can actually > be called. There is, however, code where this is not possible. > Specifically, it doesn't work if the code triggered with the timer uses a > lock, and del_timer() is also called under that same lock. An example for > that is the code in sound/synth/emux/emux.c. How do you suggest to handle > that situation ?
Easy. Tell me how that situation is not a bug?
That code you point out at emux.c looks extremely buggy as well. In other words, if you can't call del_timer_free() for the reason you mention above, the code is very likely to have race conditions. I cannot think of a situation that it is safe to do this.
In fact, I think that just replacing that with del_timer_free() may be good enough. At least to show why it blows up later. I'm confused in what they are doing by taking that lock. I can see:
CPU1 CPU2 ---- ---- snd_emux_timer_callback() spin_lock(voice_lock) if (timer_active) del_timer() spin_unlock(voice_lock)
[..] do_again++ [..] if (do_again) { mod_timer() timer_active = 1; }
[..] free(emu);
BOOM!!
Hmm, perhaps I should change the code in __mod_timer() to:
if (WARN_ON(timer->flags & TIMER_FREED)) return;
To not rearm it.
-- Steve
| |