Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Artem Savkov <asavkov@redhat.com>, jpoimboe@redhat.com,
        netdev@vger.kernel.org
Cc:     davem@davemloft.net, yoshfuji@linux-ipv6.org, dsahern@kernel.org,
        linux-kernel@vger.kernel.org, Artem Savkov <asavkov@redhat.com>,
        Anna-Maria Gleixner <anna-maria@linutronix.de>
Subject: Re: [PATCH 1/2] timer: introduce upper bound timers
In-Reply-To: <20220323111642.2517885-2-asavkov@redhat.com>
References: <20220323111642.2517885-1-asavkov@redhat.com>
 <20220323111642.2517885-2-asavkov@redhat.com>
Date:   Thu, 24 Mar 2022 13:28:43 +0100
Message-ID: <87tubn8rgk.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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