netdev - 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]

Message-ID: <0a62a2b6bff4fd01065e0de6a8703c96e344f1dc.camel@axis.com>
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?