Re: [PATCH net-next v6] atl1c: move tx cleanup processing out of interrupt

2021-04-16 Thread patchwork-bot+netdevbpf
Hello:

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

On Wed, 14 Apr 2021 22:09:20 +0300 you wrote:
> Tx queue cleanup happens in interrupt handler on same core as rx queue
> processing. Both can take considerable amount of processing in high
> packet-per-second scenarios.
> 
> Sending big amounts of packets can stall the rx processing which is
> unfair and also can lead to out-of-memory condition since
> __dev_kfree_skb_irq queues the skbs for later kfree in softirq which
> is not allowed to happen with heavy load in interrupt handler.
> 
> [...]

Here is the summary with links:
  - [net-next,v6] atl1c: move tx cleanup processing out of interrupt
https://git.kernel.org/netdev/net-next/c/a1150a04b7e8

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




[PATCH net-next v6] atl1c: move tx cleanup processing out of interrupt

2021-04-14 Thread Gatis Peisenieks
Tx queue cleanup happens in interrupt handler on same core as rx queue
processing. Both can take considerable amount of processing in high
packet-per-second scenarios.

Sending big amounts of packets can stall the rx processing which is
unfair and also can lead to out-of-memory condition since
__dev_kfree_skb_irq queues the skbs for later kfree in softirq which
is not allowed to happen with heavy load in interrupt handler.

This puts tx cleanup in its own napi and enables threaded napi to
allow the rx/tx queue processing to happen on different cores.

The ability to sustain equal amounts of tx/rx traffic increased:
from 280Kpps to 1130Kpps on Threadripper 3960X with upcoming
Mikrotik 10/25G NIC,
from 520Kpps to 850Kpps on Intel i3-3320 with Mikrotik RB44Ge adapter.

Signed-off-by: Gatis Peisenieks 
---
v6:
- sent with git send-email
v5:
- EXPORT_SYMBOL(dev_set_threaded) not needed, already there
v4:
- made scripts/checkpatch.pl happy
- moved the new intr_mask_lock to be besides the intr_mask it
  protects so they are more likely to be on same cacheline
v3:
- addressed comments from Eric Dumazet
- added EXPORT_SYMBOL for dev_set_threaded

 drivers/net/ethernet/atheros/atl1c/atl1c.h|  2 +
 .../net/ethernet/atheros/atl1c/atl1c_main.c   | 44 ++-
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c.h 
b/drivers/net/ethernet/atheros/atl1c/atl1c.h
index a0562a90fb6d..28ae5c16831e 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c.h
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c.h
@@ -367,6 +367,7 @@ struct atl1c_hw {
u16 phy_id1;
u16 phy_id2;
 
+   spinlock_t intr_mask_lock;  /* protect the intr_mask */
u32 intr_mask;
 
u8 preamble_len;
@@ -506,6 +507,7 @@ struct atl1c_adapter {
struct net_device   *netdev;
struct pci_dev  *pdev;
struct napi_struct  napi;
+   struct napi_struct  tx_napi;
struct page *rx_page;
unsigned intrx_page_offset;
unsigned intrx_frag_size;
diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c 
b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index d54375b255dc..1d17c24e6d75 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -813,6 +813,7 @@ static int atl1c_sw_init(struct atl1c_adapter *adapter)
atl1c_set_rxbufsize(adapter, adapter->netdev);
atomic_set(&adapter->irq_sem, 1);
spin_lock_init(&adapter->mdio_lock);
+   spin_lock_init(&adapter->hw.intr_mask_lock);
set_bit(__AT_DOWN, &adapter->flags);
 
return 0;
@@ -1530,20 +1531,19 @@ static inline void atl1c_clear_phy_int(struct 
atl1c_adapter *adapter)
spin_unlock(&adapter->mdio_lock);
 }
 
-static bool atl1c_clean_tx_irq(struct atl1c_adapter *adapter,
-   enum atl1c_trans_queue type)
+static int atl1c_clean_tx(struct napi_struct *napi, int budget)
 {
-   struct atl1c_tpd_ring *tpd_ring = &adapter->tpd_ring[type];
+   struct atl1c_adapter *adapter =
+   container_of(napi, struct atl1c_adapter, tx_napi);
+   struct atl1c_tpd_ring *tpd_ring = 
&adapter->tpd_ring[atl1c_trans_normal];
struct atl1c_buffer *buffer_info;
struct pci_dev *pdev = adapter->pdev;
u16 next_to_clean = atomic_read(&tpd_ring->next_to_clean);
u16 hw_next_to_clean;
-   u16 reg;
unsigned int total_bytes = 0, total_packets = 0;
+   unsigned long flags;
 
-   reg = type == atl1c_trans_high ? REG_TPD_PRI1_CIDX : REG_TPD_PRI0_CIDX;
-
-   AT_READ_REGW(&adapter->hw, reg, &hw_next_to_clean);
+   AT_READ_REGW(&adapter->hw, REG_TPD_PRI0_CIDX, &hw_next_to_clean);
 
while (next_to_clean != hw_next_to_clean) {
buffer_info = &tpd_ring->buffer_info[next_to_clean];
@@ -1564,7 +1564,15 @@ static bool atl1c_clean_tx_irq(struct atl1c_adapter 
*adapter,
netif_wake_queue(adapter->netdev);
}
 
-   return true;
+   if (total_packets < budget) {
+   napi_complete_done(napi, total_packets);
+   spin_lock_irqsave(&adapter->hw.intr_mask_lock, flags);
+   adapter->hw.intr_mask |= ISR_TX_PKT;
+   AT_WRITE_REG(&adapter->hw, REG_IMR, adapter->hw.intr_mask);
+   spin_unlock_irqrestore(&adapter->hw.intr_mask_lock, flags);
+   return total_packets;
+   }
+   return budget;
 }
 
 /**
@@ -1599,13 +1607,22 @@ static irqreturn_t atl1c_intr(int irq, void *data)
AT_WRITE_REG(hw, REG_ISR, status | ISR_DIS_INT);
if (status & ISR_RX_PKT) {
if (likely(napi_schedule_prep(&adapter->napi))) {
+   spin_lock(&hw->intr_mask_lock);
hw->intr_mask &= ~ISR_RX_PKT;
AT_WRITE_REG(hw, REG_IMR, hw->in