RE: [Intel-wired-lan] [PATCH iwl-next v2 4/4] igb: Get rid of spurious interrupts

Loktionov, Aleksandr
2025-02-21T12:46:01Z

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Kurt Kanzenbach
> Sent: Monday, February 17, 2025 12:31 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Sebastian
> Andrzej Siewior <bigeasy@linutronix.de>; Damato, Joe
> <jdamato@fastly.com>; Gerhard Engleder <gerhard@engleder-
> embedded.com>; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org;
> Kurt Kanzenbach <kurt@linutronix.de>
> Subject: [Intel-wired-lan] [PATCH iwl-next v2 4/4] igb: Get rid of spurious
> interrupts
>
> When running the igc with XDP/ZC in busy polling mode with deferral of hard
> interrupts, interrupts still happen from time to time. That is caused by the igb
> task watchdog which triggers Rx interrupts periodically.
>
> That mechanism has been introduced to overcome skb/memory allocation
> failures [1]. So the Rx clean functions stop processing the Rx ring in case of
> such failure. The task watchdog triggers Rx interrupts periodically in the hope
> that memory became available in the mean time.
>
> The current behavior is undesirable for real time applications, because the
> driver induced Rx interrupts trigger also the softirq processing. However, all
> real time packets should be processed by the application which uses the busy
> polling method.
>
> Therefore, only trigger the Rx interrupts in case of real allocation failures.
> Introduce a new flag for signaling that condition.
>
> Follow the same logic as in commit 8dcf2c212078 ("igc: Get rid of spurious
> interrupts").
>
> [1] -
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=3
> be507547e6177e5c808544bd6a2efa2c7f1d436
>
> Reviewed-by: Joe Damato <jdamato@fastly.com>
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> ---
> drivers/net/ethernet/intel/igb/igb.h | 3 ++-
> drivers/net/ethernet/intel/igb/igb_main.c | 29
> +++++++++++++++++++++++++---- drivers/net/ethernet/intel/igb/igb_xsk.c
> | 1 +
> 3 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb.h
> b/drivers/net/ethernet/intel/igb/igb.h
> index
> 79eca385a751bfdafdf384928b6cc1b350b22560..f34ead8243e9f0176a068
> 299138c5c16f7faab2e 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -391,7 +391,8 @@ enum e1000_ring_flags_t {
> IGB_RING_FLAG_RX_LB_VLAN_BSWAP,
> IGB_RING_FLAG_TX_CTX_IDX,
> IGB_RING_FLAG_TX_DETECT_HANG,
> - IGB_RING_FLAG_TX_DISABLED
> + IGB_RING_FLAG_TX_DISABLED,
> + IGB_RING_FLAG_RX_ALLOC_FAILED,
> };
>
> #define ring_uses_large_buffer(ring) \
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index
> 054376d648da883f35d1dee5f879487b8adfd540..25abe7d8ab400f63ca9b
> 4e87c9b5f2c15316485a 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -5755,11 +5755,29 @@ static void igb_watchdog_task(struct
> work_struct *work)
> if (adapter->flags & IGB_FLAG_HAS_MSIX) {
> u32 eics = 0;
>
> - for (i = 0; i < adapter->num_q_vectors; i++)
> - eics |= adapter->q_vector[i]->eims_value;
> - wr32(E1000_EICS, eics);
> + for (i = 0; i < adapter->num_q_vectors; i++) {
> + struct igb_q_vector *q_vector = adapter->q_vector[i];
> + struct igb_ring *rx_ring;
> +
> + if (!q_vector->rx.ring)
> + continue;
> +
> + rx_ring = adapter->rx_ring[q_vector->rx.ring-
> >queue_index];
> +
> + if (test_bit(IGB_RING_FLAG_RX_ALLOC_FAILED,
> &rx_ring->flags)) {
> + eics |= q_vector->eims_value;
> + clear_bit(IGB_RING_FLAG_RX_ALLOC_FAILED,
> &rx_ring->flags);
> + }
> + }
> + if (eics)
> + wr32(E1000_EICS, eics);
> } else {
> - wr32(E1000_ICS, E1000_ICS_RXDMT0);
> + struct igb_ring *rx_ring = adapter->rx_ring[0];
> +
> + if (test_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring-
> >flags)) {
> + clear_bit(IGB_RING_FLAG_RX_ALLOC_FAILED,
> &rx_ring->flags);
> + wr32(E1000_ICS, E1000_ICS_RXDMT0);
> + }
> }
>
> igb_spoof_check(adapter);
> @@ -9090,6 +9108,7 @@ static int igb_clean_rx_irq(struct igb_q_vector
> *q_vector, const int budget)
> if (!xdp_res && !skb) {
> rx_ring->rx_stats.alloc_failed++;
> rx_buffer->pagecnt_bias++;
> + set_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring-
> >flags);
> break;
> }
>
> @@ -9149,6 +9168,7 @@ static bool igb_alloc_mapped_page(struct igb_ring
> *rx_ring,
> page = dev_alloc_pages(igb_rx_pg_order(rx_ring));
> if (unlikely(!page)) {
> rx_ring->rx_stats.alloc_failed++;
> + set_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags);
> return false;
> }
>
> @@ -9165,6 +9185,7 @@ static bool igb_alloc_mapped_page(struct igb_ring
> *rx_ring,
> __free_pages(page, igb_rx_pg_order(rx_ring));
>
> rx_ring->rx_stats.alloc_failed++;
> + set_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags);
> return false;
> }
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_xsk.c
> b/drivers/net/ethernet/intel/igb/igb_xsk.c
> index
> a5ad090dfe94b6afc8194fe39d28cdd51c7067b0..47344ee1ed7f29bd68055
> 485702a87df3b8922e8 100644
> --- a/drivers/net/ethernet/intel/igb/igb_xsk.c
> +++ b/drivers/net/ethernet/intel/igb/igb_xsk.c
> @@ -417,6 +417,7 @@ int igb_clean_rx_irq_zc(struct igb_q_vector *q_vector,
> /* exit if we failed to retrieve a buffer */
> if (!skb) {
> rx_ring->rx_stats.alloc_failed++;
> + set_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring-
> >flags);
> break;
> }
>
>
> --
> 2.39.5