Messages in this thread | | | From | Linus Torvalds <> | Date | Mon, 14 Nov 2022 09:16:31 -0800 | Subject | Re: [PATCH v6 4/6] timers: Add timer_shutdown_sync() to be called before freeing timers |
| |
On Mon, Nov 14, 2022 at 7:42 AM Thomas Gleixner <tglx@linutronix.de> wrote: > > So if we want to make this solid and make the life of driver writers > easier, then we cannot issue a warning as I said in the original thread > already.
So I think that there are two issues at play:
(a) do we want to *find* problem places after the conversion
(b) do we want to make driver writing easier
and (a) argues for warning on timer re-arming, but (b) just says "don't warn, just ignore it, the driver is being shut down".
I'm personally ok with either of those approaches, and it's literally just a question of mindset.
> The semantics of timer_shutdown_sync() have to be: > > After return: > - the timer is not queued > - the timer callbacks is not running > - the timer cannot be enqueued again
Yes, but that last case is literally a "do we expect the *driver* to not enqueue it and warn if it tries, or do we just silently enforce it"?
I agree with all three points. I'm just not sure about who we expect to do the "don't enqueue again".
There's a big argument for "make it easy for driver writers" in just saying "make mod_timer() silently just ignore a re-arming". Making things easier for driver writers is a good thing.
But maybe it's a "you shouldn't have done that in the first place" thing, and merits a warning?
I have no strong opinions on that.
What I *do* still want to happen is for subsystems to be able to start doing the conversion one by one. Which is why I'd still prefer to have the new names available just so that we don't have to have one 50-patch series, but we can have subsystems apply the obvious cases.
And I'd still like the mindless "let's get the non-semantic changes out of the way" as one single patch, to get rid of mindless noise.
And honestly, for that to happen I'd be perfectly happy with something like
#define timer_shutdown(t) del_timer(t) #define timer_shutdown_sync(t) del_timer_sync(t)
(obviously with the patches that first remove the existing 'timer_shutdown()' uses first). That wouldn't introduce the *new* semantics, but it would at least allow the different subsystems to do the obvious cases, and let the networking people wonder about the much less obvious ones.
Linus
| |