1. Patchew
  2. linux
  3. View series

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

muhammad.nazim.amirul.nazle.asmade@altera.com posted 1 patch 1 week, 5 days ago
There is a newer version of this series
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions()
[PATCH v2] net: stmmac: Improve Tx timer arm logic further
Posted by muhammad.nazim.amirul.nazle.asmade@altera.com 1 week, 5 days ago 
From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

Calling hrtimer_start() on an already-active txtimer is unnecessary
and expensive. Skip the restart if the timer is already active by
adding an hrtimer_active() check before hrtimer_start().

This avoids redundant timer restarts under burst traffic and ensures
NAPI is scheduled within tx_coal_timer microseconds of the first
packet rather than having the window reset on every packet.

There is no race concern: hrtimer_start() is internally serialized and
safe to call on an active timer. In the event of a race between
hrtimer_active() and hrtimer_start(), the worst case is calling
hrtimer_start() on an already-active timer, which is identical to the
pre-patch behaviour. The meaning of tx_coal_timer is unchanged.

Performance on Cyclone V with dwmac-socfpga (iperf3 -u -b 0 -l 64):
  Before: ~45200 pps
  After:  ~52300 pps (~15% improvement)

Additionally, ~10% improvement in UDP throughput observed on Agilex5,
with hrtimer CPU usage reduced from ~8% to ~0.6%.

Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
Changes in v2:
 - Expanded commit message to address race condition concern and clarify
   tx_coal_timer semantics (Andrew Lunn)
 - Added performance numbers to commit message (Andrew Lunn)
 - Added Agilex5 performance data with hrtimer CPU usage improvement
 - Added Tested-by and Reviewed-by from Maxime Chevallier
---
 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);
+	}
 }
 
 /**
-- 
2.43.7
Re: [PATCH v2] net: stmmac: Improve Tx timer arm logic further
Posted by Andrew Lunn 1 week, 4 days ago 
> pre-patch behaviour. The meaning of tx_coal_timer is unchanged.

> 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

With this code, the timer is always tx_coal_timer in the future.


> +	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);

If the timer is not active, it is set to tx_coal_timer in the
future. However, if the timer is active, meaning it is already
counting down, it is left alone, so is less than tx_coal_timer in the
future.

Do i have this right?

Doesn't that change the meaning of the timer. It now actually goes off
sooner?

This is somewhat academic. The point of coalescence is to reduce
overheads. The increase in performance shows that this change does
reduce overheads.

	Andrew
Re: [PATCH v2] net: stmmac: Improve Tx timer arm logic further
Posted by Nazle Asmade, Muhammad Nazim Amirul 1 week, 3 days ago 
On 28/5/2026 9:58 am, Andrew Lunn wrote:
>> pre-patch behaviour. The meaning of tx_coal_timer is unchanged.
> 
>> 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
> 
> With this code, the timer is always tx_coal_timer in the future.
> 
> 
>> +	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);
> 
> If the timer is not active, it is set to tx_coal_timer in the
> future. However, if the timer is active, meaning it is already
> counting down, it is left alone, so is less than tx_coal_timer in the
> future.
> 
> Do i have this right?
> 
> Doesn't that change the meaning of the timer. It now actually goes off
> sooner?
> 
> This is somewhat academic. The point of coalescence is to reduce
> overheads. The increase in performance shows that this change does
> reduce overheads.
> 
> 	Andrew
Hi Andrew,

Yes, you have it right. I have updated the commit message in v3 to 
correctly describe this behaviour change.

Thanks, Nazim
Re: [PATCH v2] net: stmmac: Improve Tx timer arm logic further
Posted by Jacob Keller 1 week, 4 days ago 
On 5/26/2026 7:33 PM, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> 
> Calling hrtimer_start() on an already-active txtimer is unnecessary
> and expensive. Skip the restart if the timer is already active by
> adding an hrtimer_active() check before hrtimer_start().
> 
> This avoids redundant timer restarts under burst traffic and ensures
> NAPI is scheduled within tx_coal_timer microseconds of the first
> packet rather than having the window reset on every packet.
> 
> There is no race concern: hrtimer_start() is internally serialized and
> safe to call on an active timer. In the event of a race between
> hrtimer_active() and hrtimer_start(), the worst case is calling
> hrtimer_start() on an already-active timer, which is identical to the
> pre-patch behaviour. The meaning of tx_coal_timer is unchanged.
> 
> Performance on Cyclone V with dwmac-socfpga (iperf3 -u -b 0 -l 64):
>   Before: ~45200 pps
>   After:  ~52300 pps (~15% improvement)
> 
> Additionally, ~10% improvement in UDP throughput observed on Agilex5,
> with hrtimer CPU usage reduced from ~8% to ~0.6%.
> 

Nice improvements! Appreciate seeing the data here.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
> Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> ---
> Changes in v2:
>  - Expanded commit message to address race condition concern and clarify
>    tx_coal_timer semantics (Andrew Lunn)
>  - Added performance numbers to commit message (Andrew Lunn)
>  - Added Agilex5 performance data with hrtimer CPU usage improvement
>  - Added Tested-by and Reviewed-by from Maxime Chevallier
> ---
>  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);
> +	}
>  }
>  
>  /**

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.