* [PATCH v2] net: stmmac: Improve Tx timer arm logic further
@ 2026-05-26 5:19 muhammad.nazim.amirul.nazle.asmade
0 siblings, 0 replies; 5+ messages in thread
From: muhammad.nazim.amirul.nazle.asmade @ 2026-05-26 5:19 UTC (permalink / raw)
To: netdev
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32,
alexandre.torgue, rmk+kernel, maxime.chevallier, linux-stm32,
linux-arm-kernel, linux-kernel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] net: stmmac: Improve Tx timer arm logic further
@ 2026-05-27 2:33 muhammad.nazim.amirul.nazle.asmade
2026-05-27 21:00 ` Jacob Keller
2026-05-28 1:58 ` Andrew Lunn
0 siblings, 2 replies; 5+ messages in thread
From: muhammad.nazim.amirul.nazle.asmade @ 2026-05-27 2:33 UTC (permalink / raw)
To: netdev
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32,
alexandre.torgue, rmk+kernel, maxime.chevallier, linux-stm32,
linux-arm-kernel, linux-kernel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net: stmmac: Improve Tx timer arm logic further
2026-05-27 2:33 [PATCH v2] net: stmmac: Improve Tx timer arm logic further muhammad.nazim.amirul.nazle.asmade
@ 2026-05-27 21:00 ` Jacob Keller
2026-05-28 1:58 ` Andrew Lunn
1 sibling, 0 replies; 5+ messages in thread
From: Jacob Keller @ 2026-05-27 21:00 UTC (permalink / raw)
To: muhammad.nazim.amirul.nazle.asmade, netdev
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32,
alexandre.torgue, rmk+kernel, maxime.chevallier, linux-stm32,
linux-arm-kernel, linux-kernel
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);
> + }
> }
>
> /**
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net: stmmac: Improve Tx timer arm logic further
2026-05-27 2:33 [PATCH v2] net: stmmac: Improve Tx timer arm logic further muhammad.nazim.amirul.nazle.asmade
2026-05-27 21:00 ` Jacob Keller
@ 2026-05-28 1:58 ` Andrew Lunn
2026-05-29 6:47 ` Nazle Asmade, Muhammad Nazim Amirul
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2026-05-28 1:58 UTC (permalink / raw)
To: muhammad.nazim.amirul.nazle.asmade
Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, alexandre.torgue, rmk+kernel, maxime.chevallier,
linux-stm32, linux-arm-kernel, linux-kernel
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net: stmmac: Improve Tx timer arm logic further
2026-05-28 1:58 ` Andrew Lunn
@ 2026-05-29 6:47 ` Nazle Asmade, Muhammad Nazim Amirul
0 siblings, 0 replies; 5+ messages in thread
From: Nazle Asmade, Muhammad Nazim Amirul @ 2026-05-29 6:47 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev@vger•kernel.org, andrew+netdev@lunn•ch,
davem@davemloft•net, edumazet@google•com, kuba@kernel•org,
pabeni@redhat•com, mcoquelin.stm32@gmail•com,
alexandre.torgue@foss•st.com, rmk+kernel@armlinux•org.uk,
maxime.chevallier@bootlin•com,
linux-stm32@st-md-mailman•stormreply.com,
linux-arm-kernel@lists•infradead.org,
linux-kernel@vger•kernel.org
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-29 6:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-27 2:33 [PATCH v2] net: stmmac: Improve Tx timer arm logic further muhammad.nazim.amirul.nazle.asmade
2026-05-27 21:00 ` Jacob Keller
2026-05-28 1:58 ` Andrew Lunn
2026-05-29 6:47 ` Nazle Asmade, Muhammad Nazim Amirul
-- strict thread matches above, loose matches on Subject: below --
2026-05-26 5:19 muhammad.nazim.amirul.nazle.asmade
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox