Re: [PATCH 1/2] timer: introduce upper bound timers

[Thomas Gleixner <tglx@linutronix.de>]

Artem,

On Wed, Mar 23 2022 at 12:16, Artem Savkov wrote:
> Add TIMER_UPPER_BOUND flag which allows creation of timers that would
> expire at most at specified time or earlier.
>
> This was previously discussed here:
> https://lore.kernel.org/all/20210302001054.4qgrvnkltvkgikzr@treble/T/#u

please add the context to the changelog. A link is only supplemental
information and does not replace content.

>  static inline unsigned calc_index(unsigned long expires, unsigned lvl,
> -				  unsigned long *bucket_expiry)
> +				  unsigned long *bucket_expiry, bool upper_bound)
>  {
>  
>  	/*
> @@ -501,34 +501,39 @@ static inline unsigned calc_index(unsigned long expires, unsigned lvl,
>  	 * - Truncation of the expiry time in the outer wheel levels
>  	 *
>  	 * Round up with level granularity to prevent this.
> +	 * Do not perform round up in case of upper bound timer.
>  	 */
> -	expires = (expires + LVL_GRAN(lvl)) >> LVL_SHIFT(lvl);
> +	if (upper_bound)
> +		expires = expires >> LVL_SHIFT(lvl);
> +	else
> +		expires = (expires + LVL_GRAN(lvl)) >> LVL_SHIFT(lvl);

While this "works", I fundamentally hate this because it adds an extra
conditional into the common case. That affects every user of the timer
wheel. We went great length to optimize that code and I'm not really enthused
to sacrifice that just because of _one_ use case.

The resulting text bloat is +25% on x86/64 and a quick test case shows
that this is well measurable overhead. The first ones who will complain
about that are going to be - drumroll - the network people.

There must be smarter ways to solve that. Let me think about it.

Thanks,

        tglx