linux-kernel - Re: [net-next PATCH 2/3] net: stmmac: improve TX timer arm logic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 29 Sep 2023 12:38:48 +0000
From:   Vincent Whitchurch <Vincent.Whitchurch@...s.com>
To:     "ansuelsmth@...il.com" <ansuelsmth@...il.com>,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "liuhangbin@...il.com" <liuhangbin@...il.com>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "joabreu@...opsys.com" <joabreu@...opsys.com>,
        "rajur@...lsio.com" <rajur@...lsio.com>,
        Vincent Whitchurch <Vincent.Whitchurch@...s.com>,
        "horms@...nel.org" <horms@...nel.org>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "kvalo@...nel.org" <kvalo@...nel.org>,
        "alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
        "mcoquelin.stm32@...il.com" <mcoquelin.stm32@...il.com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "pkshih@...ltek.com" <pkshih@...ltek.com>
Subject: Re: [net-next PATCH 2/3] net: stmmac: improve TX timer arm logic

On Fri, 2023-09-22 at 13:12 +0200, Christian Marangi wrote:
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 9201ed778ebc..14bf6fae6662 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2994,13 +2994,25 @@ static void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue)
>  {
>  	struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
>  	u32 tx_coal_timer = priv->tx_coal_timer[queue];
> +	struct stmmac_channel *ch;
> +	struct napi_struct *napi;
>  
> 
>  	if (!tx_coal_timer)
>  		return;
>  
> 
> -	hrtimer_start(&tx_q->txtimer,
> -		      STMMAC_COAL_TIMER(tx_coal_timer),
> -		      HRTIMER_MODE_REL);
> +	ch = &priv->channel[tx_q->queue_index];
> +	napi = tx_q->xsk_pool ? &ch->rxtx_napi : &ch->tx_napi;
> +
> +	/* Arm timer only if napi is not already scheduled.
> +	 * 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
> +		hrtimer_try_to_cancel(&tx_q->txtimer);

When this function is called from within the napi poll function
(stmmac_tx_clean()), NAPI_STATE_SCHED will always be set and so after
this patch the "We still have pending packets, let's call for a new
scheduling" logic will never start the timer.  Was that really
intentional?