Add patch fixing regression from stmmac TX timer.
Refer to the single patch for extensive details on the problem.
This should restore original performance before 4.19 kernel.
Fixes: #11676
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
From b5532bdc6d09e6e789417f0c7a0b665b57b0e7be Mon Sep 17 00:00:00 2001
From: Christian Marangi <ansuelsmth@gmail.com>
Date: Mon, 18 Sep 2023 14:21:56 +0200
-Subject: [PATCH 1/3] net: introduce napi_is_scheduled helper
+Subject: [PATCH 1/4] net: introduce napi_is_scheduled helper
We currently have napi_if_scheduled_mark_missed that can be used to
check if napi is scheduled but that does more thing than simply checking
+++ /dev/null
-From 26e872db0f14c09ab57486b538ecc3a24c579df2 Mon Sep 17 00:00:00 2001
-From: Christian Marangi <ansuelsmth@gmail.com>
-Date: Mon, 18 Sep 2023 14:39:01 +0200
-Subject: [PATCH 2/3] net: stmmac: improve TX timer arm logic
-
-There is currently a problem with the TX timer getting armed multiple
-unnecessary times causing big performance regression on some device that
-suffer from heavy handling of hrtimer rearm.
-
-The use of the TX timer is an old implementation that predates the napi
-implementation and the interrupt enable/disable handling.
-
-Due to stmmac being a very old code, the TX timer was never evaluated
-again with this new implementation and was kept there causing
-performance regression. The performance regression started to appear
-with kernel version 4.19 with 8fce33317023 ("net: stmmac: Rework coalesce
-timer and fix multi-queue races") where the timer was reduced to 1ms
-causing it to be armed 40 times more than before.
-
-Decreasing the timer made the problem more present and caused the
-regression in the other of 600-700mbps on some device (regression where
-this was notice is ipq806x).
-
-The problem is in the fact that handling the hrtimer on some target is
-expensive and recent kernel made the timer armed much more times.
-A solution that was proposed was reverting the hrtimer change and use
-mod_timer but such solution would still hide the real problem in the
-current implementation.
-
-To fix the regression, apply some additional logic and skip arming the
-timer when not needed.
-
-Arm the timer ONLY if a napi is not already scheduled. Running the timer
-is redundant since the same function (stmmac_tx_clean) will run in the
-napi TX poll. Also try to cancel any timer if a napi is scheduled to
-prevent redundant run of TX call.
-
-With the following new logic the original performance are restored while
-keeping using the hrtimer.
-
-Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
----
- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++++++++---
- 1 file changed, 15 insertions(+), 3 deletions(-)
-
---- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
-+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
-@@ -2974,13 +2974,25 @@ static void stmmac_tx_timer_arm(struct s
- {
- 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);
- }
-
- /**
--- /dev/null
+From fb04db35447d1e8ff557c8e57139164cecab7de5 Mon Sep 17 00:00:00 2001
+From: Christian Marangi <ansuelsmth@gmail.com>
+Date: Wed, 27 Sep 2023 15:38:31 +0200
+Subject: [PATCH 2/4] net: stmmac: move TX timer arm after DMA enable
+
+Move TX timer arm call after DMA interrupt is enabled again.
+
+The TX timer arm function changed logic and now is skipped if a napi is
+already scheduled. By moving the TX timer arm call after DMA is enabled,
+we permit to correctly skip if a DMA interrupt has been fired and a napi
+has been scheduled again.
+
+Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
+---
+ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 19 +++++++++++++++----
+ 1 file changed, 15 insertions(+), 4 deletions(-)
+
+--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
++++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+@@ -2529,7 +2529,8 @@ static void stmmac_bump_dma_threshold(st
+ * @queue: TX queue index
+ * Description: it reclaims the transmit resources after transmission completes.
+ */
+-static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue)
++static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue,
++ bool *pending_packets)
+ {
+ struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
+ unsigned int bytes_compl = 0, pkts_compl = 0;
+@@ -2692,7 +2693,7 @@ static int stmmac_tx_clean(struct stmmac
+
+ /* We still have pending packets, let's call for a new scheduling */
+ if (tx_q->dirty_tx != tx_q->cur_tx)
+- stmmac_tx_timer_arm(priv, queue);
++ *pending_packets = true;
+
+ __netif_tx_unlock_bh(netdev_get_tx_queue(priv->dev, queue));
+
+@@ -5474,12 +5475,13 @@ static int stmmac_napi_poll_tx(struct na
+ struct stmmac_channel *ch =
+ container_of(napi, struct stmmac_channel, tx_napi);
+ struct stmmac_priv *priv = ch->priv_data;
++ bool pending_packets = false;
+ u32 chan = ch->index;
+ int work_done;
+
+ priv->xstats.napi_poll++;
+
+- work_done = stmmac_tx_clean(priv, budget, chan);
++ work_done = stmmac_tx_clean(priv, budget, chan, &pending_packets);
+ work_done = min(work_done, budget);
+
+ if (work_done < budget && napi_complete_done(napi, work_done)) {
+@@ -5490,6 +5492,10 @@ static int stmmac_napi_poll_tx(struct na
+ spin_unlock_irqrestore(&ch->lock, flags);
+ }
+
++ /* TX still have packet to handle, check if we need to arm tx timer */
++ if (pending_packets)
++ stmmac_tx_timer_arm(priv, chan);
++
+ return work_done;
+ }
+
+@@ -5499,11 +5505,12 @@ static int stmmac_napi_poll_rxtx(struct
+ container_of(napi, struct stmmac_channel, rxtx_napi);
+ struct stmmac_priv *priv = ch->priv_data;
+ int rx_done, tx_done, rxtx_done;
++ bool tx_pending_packets = false;
+ u32 chan = ch->index;
+
+ priv->xstats.napi_poll++;
+
+- tx_done = stmmac_tx_clean(priv, budget, chan);
++ tx_done = stmmac_tx_clean(priv, budget, chan, &tx_pending_packets);
+ tx_done = min(tx_done, budget);
+
+ rx_done = stmmac_rx_zc(priv, budget, chan);
+@@ -5528,6 +5535,10 @@ static int stmmac_napi_poll_rxtx(struct
+ spin_unlock_irqrestore(&ch->lock, flags);
+ }
+
++ /* TX still have packet to handle, check if we need to arm tx timer */
++ if (tx_pending_packets)
++ stmmac_tx_timer_arm(priv, chan);
++
+ return min(rxtx_done, budget - 1);
+ }
+
--- /dev/null
+From cd40cd8b1ca4a6f531c6c3fd78b306e5014f9c04 Mon Sep 17 00:00:00 2001
+From: Christian Marangi <ansuelsmth@gmail.com>
+Date: Mon, 18 Sep 2023 14:39:01 +0200
+Subject: [PATCH 3/4] net: stmmac: improve TX timer arm logic
+
+There is currently a problem with the TX timer getting armed multiple
+unnecessary times causing big performance regression on some device that
+suffer from heavy handling of hrtimer rearm.
+
+The use of the TX timer is an old implementation that predates the napi
+implementation and the interrupt enable/disable handling.
+
+Due to stmmac being a very old code, the TX timer was never evaluated
+again with this new implementation and was kept there causing
+performance regression. The performance regression started to appear
+with kernel version 4.19 with 8fce33317023 ("net: stmmac: Rework coalesce
+timer and fix multi-queue races") where the timer was reduced to 1ms
+causing it to be armed 40 times more than before.
+
+Decreasing the timer made the problem more present and caused the
+regression in the other of 600-700mbps on some device (regression where
+this was notice is ipq806x).
+
+The problem is in the fact that handling the hrtimer on some target is
+expensive and recent kernel made the timer armed much more times.
+A solution that was proposed was reverting the hrtimer change and use
+mod_timer but such solution would still hide the real problem in the
+current implementation.
+
+To fix the regression, apply some additional logic and skip arming the
+timer when not needed.
+
+Arm the timer ONLY if a napi is not already scheduled. Running the timer
+is redundant since the same function (stmmac_tx_clean) will run in the
+napi TX poll. Also try to cancel any timer if a napi is scheduled to
+prevent redundant run of TX call.
+
+With the following new logic the original performance are restored while
+keeping using the hrtimer.
+
+Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
+---
+ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++++++++---
+ 1 file changed, 15 insertions(+), 3 deletions(-)
+
+--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
++++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+@@ -2975,13 +2975,25 @@ static void stmmac_tx_timer_arm(struct s
+ {
+ 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);
+ }
+
+ /**
+++ /dev/null
-From 13e24b0c4318b8b4d0a0fde7e5e7ccb982821c63 Mon Sep 17 00:00:00 2001
-From: Christian Marangi <ansuelsmth@gmail.com>
-Date: Mon, 18 Sep 2023 15:11:13 +0200
-Subject: [PATCH 3/3] net: stmmac: increase TX coalesce timer to 5ms
-
-Commit 8fce33317023 ("net: stmmac: Rework coalesce timer and fix
-multi-queue races") decreased the TX coalesce timer from 40ms to 1ms.
-
-This caused some performance regression on some target (regression was
-reported at least on ipq806x) in the order of 600mbps dropping from
-gigabit handling to only 200mbps.
-
-The problem was identified in the TX timer getting armed too much time.
-While this was fixed and improved in another commit, performance can be
-improved even further by increasing the timer delay a bit moving from
-1ms to 5ms.
-
-The value is a good balance between battery saving by prevending too
-much interrupt to be generated and permitting good performance for
-internet oriented devices.
-
-Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
----
- drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
---- a/drivers/net/ethernet/stmicro/stmmac/common.h
-+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
-@@ -286,7 +286,7 @@ struct stmmac_safety_stats {
- #define MIN_DMA_RIWT 0x10
- #define DEF_DMA_RIWT 0xa0
- /* Tx coalesce parameters */
--#define STMMAC_COAL_TX_TIMER 1000
-+#define STMMAC_COAL_TX_TIMER 5000
- #define STMMAC_MAX_COAL_TX_TICK 100000
- #define STMMAC_TX_MAX_FRAMES 256
- #define STMMAC_TX_FRAMES 25
--- /dev/null
+From 95281ab33fbaa1e974bceb20cfb0f5c92934f2b3 Mon Sep 17 00:00:00 2001
+From: Christian Marangi <ansuelsmth@gmail.com>
+Date: Mon, 18 Sep 2023 15:11:13 +0200
+Subject: [PATCH 4/4] net: stmmac: increase TX coalesce timer to 5ms
+
+Commit 8fce33317023 ("net: stmmac: Rework coalesce timer and fix
+multi-queue races") decreased the TX coalesce timer from 40ms to 1ms.
+
+This caused some performance regression on some target (regression was
+reported at least on ipq806x) in the order of 600mbps dropping from
+gigabit handling to only 200mbps.
+
+The problem was identified in the TX timer getting armed too much time.
+While this was fixed and improved in another commit, performance can be
+improved even further by increasing the timer delay a bit moving from
+1ms to 5ms.
+
+The value is a good balance between battery saving by prevending too
+much interrupt to be generated and permitting good performance for
+internet oriented devices.
+
+Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
+---
+ drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+--- a/drivers/net/ethernet/stmicro/stmmac/common.h
++++ b/drivers/net/ethernet/stmicro/stmmac/common.h
+@@ -286,7 +286,7 @@ struct stmmac_safety_stats {
+ #define MIN_DMA_RIWT 0x10
+ #define DEF_DMA_RIWT 0xa0
+ /* Tx coalesce parameters */
+-#define STMMAC_COAL_TX_TIMER 1000
++#define STMMAC_COAL_TX_TIMER 5000
+ #define STMMAC_MAX_COAL_TX_TICK 100000
+ #define STMMAC_TX_MAX_FRAMES 256
+ #define STMMAC_TX_FRAMES 25
projects / openwrt / openwrt.git / commitdiff