2021-08-06 01:49:59

by Grygorii Strashko

[permalink] [raw]
Subject: [PATCH net-next 0/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion

hi

The intention of this series is to fully enable hard irqs deferral feature
(hrtimers based HW IRQ coalescing) from Eric Dumazet [1] for TI K3 CPSW driver
by using napi_complete_done() in TX completion path, so the combination of
parameters (/sys/class/net/ethX/):
napi_defer_hard_irqs
gro_flush_timeout
can be used for hard irqs deferral.

The Patch 1 is required before enabling hard irqs deferral feature to avoid
"Unbalanced enable" issue if gro_flush_timeout is configured while
(napi_defer_hard_irqs == 0).

It's a bit sad that it can not be configured per RX/TX separately.

[1] https://lore.kernel.org/netdev/[email protected]/
Grygorii Strashko (1):
net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX
completion

Vignesh Raghavendra (1):
net: ti: am65-cpsw-nuss: fix RX IRQ state after .ndo_stop()

drivers/net/ethernet/ti/am65-cpsw-nuss.c | 23 ++++++++++++++++-------
drivers/net/ethernet/ti/am65-cpsw-nuss.h | 2 ++
2 files changed, 18 insertions(+), 7 deletions(-)

--
2.17.1


2021-08-06 01:51:11

by Grygorii Strashko

[permalink] [raw]
Subject: [PATCH net-next 1/2] net: ti: am65-cpsw-nuss: fix RX IRQ state after .ndo_stop()

From: Vignesh Raghavendra <[email protected]>

On TI K3 am64x platform the issue with RX IRQ is observed - it's become
disabled forever after .ndo_stop(). The K3 CPSW driver manipulates RX IRQ
by using standard Linux enable_irq()/disable_irq_nosync() API as there is
no IRQ enable/disable options in CPSW HW itself, as result during
.ndo_stop() following sequence happens

phy_stop()
teardown TX/RX channels
wait for TX tdown complete
napi_disable(TX)
clean up TX channels

(a)

napi_disable(RX)

At point (a) it's not possible to predict if RX IRQ was triggered or not.
if RX IRQ was triggered then it also not possible to definitely say if RX
NAPI was run or only scheduled and immediately canceled by
napi_disable(RX). Actually the last case causes RX IRQ to be permanently
disabled.

Another observed issue is that RX IRQ enable counter become unbalanced if
(gro_flush_timeout =! 0) while (napi_defer_hard_irqs == 0):

Unbalanced enable for IRQ 44
WARNING: CPU: 0 PID: 10 at ../kernel/irq/manage.c:776 __enable_irq+0x38/0x80
__enable_irq+0x38/0x80
enable_irq+0x54/0xb0
am65_cpsw_nuss_rx_poll+0x2f4/0x368
__napi_poll+0x34/0x1b8
net_rx_action+0xe4/0x220
_stext+0x11c/0x284
run_ksoftirqd+0x4c/0x60

To avoid above issues introduce flag indicating if RX was actually disabled
before enabling it in am65_cpsw_nuss_rx_poll() and restore RX IRQ state in
.ndo_open()

Fixes: 4f7cce272403 ("net: ethernet: ti: am65-cpsw: add support for am64x cpsw3g")
Signed-off-by: Vignesh Raghavendra <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 +++++++++++--
drivers/net/ethernet/ti/am65-cpsw-nuss.h | 2 ++
2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 629c267d5254..08399f572091 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -519,6 +519,10 @@ static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common,
}

napi_enable(&common->napi_rx);
+ if (common->rx_irq_disabled) {
+ common->rx_irq_disabled = false;
+ enable_irq(common->rx_chns.irq);
+ }

dev_dbg(common->dev, "cpsw_nuss started\n");
return 0;
@@ -872,8 +876,12 @@ static int am65_cpsw_nuss_rx_poll(struct napi_struct *napi_rx, int budget)

dev_dbg(common->dev, "%s num_rx:%d %d\n", __func__, num_rx, budget);

- if (num_rx < budget && napi_complete_done(napi_rx, num_rx))
- enable_irq(common->rx_chns.irq);
+ if (num_rx < budget && napi_complete_done(napi_rx, num_rx)) {
+ if (common->rx_irq_disabled) {
+ common->rx_irq_disabled = false;
+ enable_irq(common->rx_chns.irq);
+ }
+ }

return num_rx;
}
@@ -1091,6 +1099,7 @@ static irqreturn_t am65_cpsw_nuss_rx_irq(int irq, void *dev_id)
{
struct am65_cpsw_common *common = dev_id;

+ common->rx_irq_disabled = true;
disable_irq_nosync(irq);
napi_schedule(&common->napi_rx);

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
index 5d93e346f05e..048ed10143c1 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
@@ -126,6 +126,8 @@ struct am65_cpsw_common {
struct am65_cpsw_rx_chn rx_chns;
struct napi_struct napi_rx;

+ bool rx_irq_disabled;
+
u32 nuss_ver;
u32 cpsw_ver;
unsigned long bus_freq;
--
2.17.1

2021-08-06 01:51:11

by Grygorii Strashko

[permalink] [raw]
Subject: [PATCH net-next 2/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion

This patch enables support for hard irqs deferral feature from Eric Dumazet
[1] for TI K3 CPSW driver by using napi_complete_done() in TX completion
path.

Depending on gro_flush_timeout and napi_defer_hard_irqs at gives up to 30%
CPU utilization reduction:

gro_flush_timeout=50000
napi_defer_hard_irqs=2

netperf -l 10 -H 192.168.1.1 -t UDP_STREAM -c -C -- -m 1470
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.1.1 () port 0 AF_INET
Socket Message Elapsed Messages CPU Service
Size Size Time Okay Errors Throughput Util Demand
bytes bytes secs # # 10^6bits/sec % SS us/KB

before:
212992 1470 10.00 809632 0 952.0 42.98 14.792
212992 10.00 809630 952.0 50.66 8.719

after:
212992 1470 10.00 813686 0 956.8 32.14 11.009
212992 10.00 813686 956.8 50.05 8.570

[1] https://lore.kernel.org/netdev/[email protected]/

Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 08399f572091..b18343f9791d 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1086,13 +1086,13 @@ static int am65_cpsw_nuss_tx_poll(struct napi_struct *napi_tx, int budget)
else
num_tx = am65_cpsw_nuss_tx_compl_packets(tx_chn->common, tx_chn->id, budget);

- num_tx = min(num_tx, budget);
- if (num_tx < budget) {
- napi_complete(napi_tx);
+ if (num_tx >= budget)
+ return budget;
+
+ if (napi_complete_done(napi_tx, num_tx))
enable_irq(tx_chn->irq);
- }

- return num_tx;
+ return 0;
}

static irqreturn_t am65_cpsw_nuss_rx_irq(int irq, void *dev_id)
--
2.17.1

2021-08-06 10:11:29

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net-next 0/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 6 Aug 2021 01:55:30 +0300 you wrote:
> hi
>
> The intention of this series is to fully enable hard irqs deferral feature
> (hrtimers based HW IRQ coalescing) from Eric Dumazet [1] for TI K3 CPSW driver
> by using napi_complete_done() in TX completion path, so the combination of
> parameters (/sys/class/net/ethX/):
> napi_defer_hard_irqs
> gro_flush_timeout
> can be used for hard irqs deferral.
>
> [...]

Here is the summary with links:
- [net-next,1/2] net: ti: am65-cpsw-nuss: fix RX IRQ state after .ndo_stop()
https://git.kernel.org/netdev/net-next/c/47bfc4d128de
- [net-next,2/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion
https://git.kernel.org/netdev/net-next/c/3bacbe04251b

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html