Re: [PATCH] net: stmmac: Improve Tx timer arm logic further

[Maxime Chevallier]

Hi,

On 5/25/26 08:16, muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx wrote:
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx>
>
> Currently hrtimer_start is called even if hrtimer is
> active. This is unnecessary and expensive in some targets.
> This patch avoids calling hrtimer_start unnecessarily.

This description is a bit lacking on details wrt. the expensiveness
and perf impacts, but I've tested with and without this patch on a
cyclone V with dwmac-socfpga, and I cansee a diff when sending
small UDP packets with :

iperf3 -c <srv> -u -b 0 -l 64

Before this patch, around 45200 pps sent,
after this patch, around 52300 pps sent !

Nice improvement :)

So from my perspective,

Tested-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
Reviewed-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>

Maxime


> Signed-off-by: Rohan G Thomas <rohan.g.thomas@xxxxxxxxx>
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 3591755ea30b..35da51c26248 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3341,12 +3341,14 @@ static void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue)
>   	 * Try to cancel any timer if napi is scheduled, timer will be armed
>   	 * again in the next scheduled napi.
>   	 */
> -	if (unlikely(!napi_is_scheduled(napi)))
> -		hrtimer_start(&tx_q->txtimer,
> -			      STMMAC_COAL_TIMER(tx_coal_timer),
> -			      HRTIMER_MODE_REL);
> -	else
> +	if (unlikely(!napi_is_scheduled(napi))) {
> +		if (unlikely(!(hrtimer_active(&tx_q->txtimer))))
> +			hrtimer_start(&tx_q->txtimer,
> +				      STMMAC_COAL_TIMER(tx_coal_timer),
> +				      HRTIMER_MODE_REL);
> +	} else {
>   		hrtimer_try_to_cancel(&tx_q->txtimer);
> +	}
>   }
>   
>   /**