Re: [PATCH 00/17] timers: Complete the timer_*() API renames

Ingo Molnar
2025-04-16T09:04:09Z

* Thomas Gleixner <tglx@linutronix.de> wrote:

> While I appreciate proper namespace prefixes, this series is just a
> mechanical conversion without any additional value.

Yes, intentionally so, as mentioned I didn't even rename any of the
APIs beyond the trivial conversion, because I wanted to gather feedback
first:

> > I didn't want to make bigger, discretionary changes in the first
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > iteration, but I can easily propagate any such suggestions into
^^^^^^^^^
> > future versions of this series.

https://lore.kernel.org/all/Z_zk94RFo2bK85iJ@gmail.com/

Can you please acknowledge this clearly declared, intentionally limited
scope of the -v1 series, instead of deriding it as 'mindless'? :-)

> [...] Some of the conversions like try_to_del_timer_sync() are
> obviously fine and can't provide moar than a namespace consolidation.
>
> But if you look at the actual functions and their usage all over the
> place then you can see that there is way more cleanup and consolidation
> potential especially for those functions which add or modify timers.
>
> First of all the question is whether add() and mod() are really valuable
> distinctions. I'm not convinced at all. Back then, when we introduced
> hrtimers, we came to the conclusion that hrtimer_start() is sufficient.

I didn't want to mix namespace cleanups with functional cleanups, but
sure, I'd be glad to do that too.

> But that aside there is a major cleanup potential for this stuff. The
> vast majority of add/mod_timer() sites uses:
>
> - Precomputed timeout values derived from a timeout provided in SE
> units
>
> - Instant conversions of SE unit based timeouts to jiffies
>
> msec/usec/sec_to_jiffies()
>
> - All variants of HZ, HZ * N, HZ / N ....
>
> This is lots of duplicated and copy and pasted code. So instead of
> blindly renaming things, we can be smarter and provide sensible
> functions:
>
> mod_timer() takes an absolute expiry value, but most places use
>
> mod_timer(t, jiffies + $timeout);
>
> So the obvious first step is to provide:
>
> timer_start_rel(t, $timeout);
>
> which does the addition of jiffies under the hood.
>
> And because $timeout is some of the above calculations, we can be
> smart and provide:
>
> timer_start_rel_secs(t, timeout_in_seconds);
> timer_start_rel_msecs(t, timeout_in_milliseconds);
> timer_start_rel_usecs(t, timeout_in_microseconds);
>
> This all can be sensibly converted with coccinelle, which even can
> handle the cases where $timeout is calculated from HZ / N.
>
> I have a pile of half finished coccinelle scripts somewhere, which do
> exactly such a conversion. I just ran out of time to play with that, as
> I ran into a few things which need more thoughts about proper
> interfaces. I'm happy to share them.

Please do!

See why I sent the simple approach first? :-)

> Converting the whole timer arming to use SE unit based timeouts makes
> a lot of sense in general and also paves the way to boot-time
> controlled HZ, which is something distros and others are asking for
> since years (of course nobody wants to sit down and do the actual
> work as usual...)
>
> That said, I'm fine to convert the obvious things, like
> try_timer_del*(), where there is no other consolidation value, but
> for everything else we better sit down and think about proper
> interfaces and large scale consolidation.

No arguments from me! :-)

Thanks,

Ingo