lkml.org 
[lkml]   [2022]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2022-11-14 18:18    [W:0.690 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site