[PATCH net-next] amd-xgbe: schedule NAPI on Rx Buffer Unavailable to prevent RX stalls

[PATCH net-next] amd-xgbe: schedule NAPI on Rx Buffer Unavailable to prevent RX stalls

[Raju Rangoju]

When Rx Buffer Unavailable (RBU) interrupt is asserted, the device can
stall under load and suffer prolonged receive starvation if polling is
not initiated. Treat RBU as a wakeup source and schedule the appropriate
NAPI instance (per-channel or global) to promptly recover from buffer
shortages and refill descriptors.

Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 73 +++++++++++++++++-------
 1 file changed, 53 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 0653e69f0ef7..ec030198b710 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -367,10 +367,11 @@ static irqreturn_t xgbe_ecc_isr(int irq, void *data)
 static void xgbe_isr_bh_work(struct work_struct *work)
 {
 	struct xgbe_prv_data *pdata = from_work(pdata, work, dev_bh_work);
+	unsigned int mac_isr, mac_tssr, mac_mdioisr;
 	struct xgbe_hw_if *hw_if = &pdata->hw_if;
-	struct xgbe_channel *channel;
+	bool per_ch_irq, ti, ri, rbu, fbe;
 	unsigned int dma_isr, dma_ch_isr;
-	unsigned int mac_isr, mac_tssr, mac_mdioisr;
+	struct xgbe_channel *channel;
 	unsigned int i;
 
 	/* The DMA interrupt status register also reports MAC and MTL
@@ -384,43 +385,75 @@ static void xgbe_isr_bh_work(struct work_struct *work)
 	netif_dbg(pdata, intr, pdata->netdev, "DMA_ISR=%#010x\n", dma_isr);
 
 	for (i = 0; i < pdata->channel_count; i++) {
+		bool schedule_napi = false;
+		struct napi_struct *napi;
+
 		if (!(dma_isr & (1 << i)))
 			continue;
 
 		channel = pdata->channel[i];
 
 		dma_ch_isr = XGMAC_DMA_IOREAD(channel, DMA_CH_SR);
+		/* Precompute flags once */
+		ti  = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, TI);
+		ri  = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, RI);
+		rbu = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, RBU);
+		fbe = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, FBE);
+
 		netif_dbg(pdata, intr, pdata->netdev, "DMA_CH%u_ISR=%#010x\n",
 			  i, dma_ch_isr);
 
-		/* The TI or RI interrupt bits may still be set even if using
-		 * per channel DMA interrupts. Check to be sure those are not
-		 * enabled before using the private data napi structure.
+		per_ch_irq = pdata->per_channel_irq;
+
+		/*
+		 * Decide which NAPI to use and whether to schedule:
+		 * - When not using per-channel IRQs: schedule on global NAPI
+		 *   if TI or RI are set.
+		 * - RBU should also trigger NAPI (either per-channel or global)
+		 *   to allow refill.
 		 */
-		if (!pdata->per_channel_irq &&
-		    (XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, TI) ||
-		     XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, RI))) {
-			if (napi_schedule_prep(&pdata->napi)) {
-				/* Disable Tx and Rx interrupts */
-				xgbe_disable_rx_tx_ints(pdata);
+		if (!per_ch_irq && (ti || ri))
+			schedule_napi = true;
 
-				/* Turn on polling */
-				__napi_schedule(&pdata->napi);
+		if (rbu) {
+			schedule_napi = true;
+			pdata->ext_stats.rx_buffer_unavailable++;
+			netif_dbg(pdata, intr, pdata->netdev,
+				  "RBU on DMA_CH %u, scheduling %s NAPI\n",
+				  i, per_ch_irq ? "per-channel" : "global");
+		}
+
+		napi = per_ch_irq ? &channel->napi : &pdata->napi;
+
+		if (schedule_napi && napi_schedule_prep(napi)) {
+			/* Disable interrupts appropriately before polling */
+			if (per_ch_irq) {
+				if (pdata->channel_irq_mode)
+					xgbe_disable_rx_tx_int(pdata, channel);
+				else
+					disable_irq_nosync(channel->dma_irq);
+			} else {
+				xgbe_disable_rx_tx_ints(pdata);
 			}
+
+			/* Turn on polling */
+			__napi_schedule(napi);
 		} else {
-			/* Don't clear Rx/Tx status if doing per channel DMA
-			 * interrupts, these will be cleared by the ISR for
-			 * per channel DMA interrupts.
+			/*
+			 * Don't clear Rx/Tx status if doing per-channel DMA
+			 * interrupts; those bits will be serviced/cleared by
+			 * the per-channel ISR/NAPI. In non-per-channel mode
+			 * when we're not scheduling NAPI here, ensure we don't
+			 * accidentally clear TI/RI in HW: zero them in the
+			 * local copy so that the eventual write-back does not
+			 * clear TI/RI.
 			 */
 			XGMAC_SET_BITS(dma_ch_isr, DMA_CH_SR, TI, 0);
 			XGMAC_SET_BITS(dma_ch_isr, DMA_CH_SR, RI, 0);
 		}
 
-		if (XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, RBU))
-			pdata->ext_stats.rx_buffer_unavailable++;
-
 		/* Restart the device on a Fatal Bus Error */
-		if (XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, FBE))
+		if (fbe)
 			schedule_work(&pdata->restart_work);
 
 		/* Clear interrupt signals */
-- 
2.34.1

Re: [PATCH net-next] amd-xgbe: schedule NAPI on Rx Buffer Unavailable to prevent RX stalls

[Jakub Kicinski]

On Mon, 24 Nov 2025 15:41:11 +0530 Raju Rangoju wrote:
> When Rx Buffer Unavailable (RBU) interrupt is asserted, the device can
> stall under load and suffer prolonged receive starvation if polling is
> not initiated. Treat RBU as a wakeup source and schedule the appropriate
> NAPI instance (per-channel or global) to promptly recover from buffer
> shortages and refill descriptors.

You need to say more.. Under heavy load network devices will routinely
run out of Rx buffers, it's expected if Rx processing is slower than
the network. What hw condition and scenario exactly are you describing
here?

>  		dma_ch_isr = XGMAC_DMA_IOREAD(channel, DMA_CH_SR);
> +		/* Precompute flags once */
> +		ti  = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, TI);
> +		ri  = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, RI);
> +		rbu = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, RBU);
> +		fbe = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, FBE);

Please split this into two patches, one pure refactoring with no
functional changes and second one changing RBU handling.

> +		if (rbu) {
> +			schedule_napi = true;
> +			pdata->ext_stats.rx_buffer_unavailable++;
> +			netif_dbg(pdata, intr, pdata->netdev,
> +				  "RBU on DMA_CH %u, scheduling %s NAPI\n",
> +				  i, per_ch_irq ? "per-channel" : "global");

I guess it's just _dbg() but as a general rule when the system is under
overload printing stuff (potentially over UART) is the last thing you
should be doing. How is this print useful to you?
-- 
pw-bot: cr

Re: [PATCH net-next] amd-xgbe: schedule NAPI on Rx Buffer Unavailable to prevent RX stalls

[Rangoju, Raju]

On 11/27/2025 8:43 AM, Jakub Kicinski wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Mon, 24 Nov 2025 15:41:11 +0530 Raju Rangoju wrote:
>> When Rx Buffer Unavailable (RBU) interrupt is asserted, the device can
>> stall under load and suffer prolonged receive starvation if polling is
>> not initiated. Treat RBU as a wakeup source and schedule the appropriate
>> NAPI instance (per-channel or global) to promptly recover from buffer
>> shortages and refill descriptors.
> 
> You need to say more.. Under heavy load network devices will routinely
> run out of Rx buffers, it's expected if Rx processing is slower than
> the network. What hw condition and scenario exactly are you describing
> here?

During the bi-directional traffic device is running out of RX buffers, 
it could be because of slower rx processing. HW notifies this via Rx 
Buffer Unavailable (RBU) interrupt. What is being described above is 
that, driver should treat RBU interrupt as source to trigger the NAPI 
poll immediately, rather than waiting for regular rx interrupts to 
process the rx buffers.

> 
>>                dma_ch_isr = XGMAC_DMA_IOREAD(channel, DMA_CH_SR);
>> +             /* Precompute flags once */
>> +             ti  = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, TI);
>> +             ri  = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, RI);
>> +             rbu = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, RBU);
>> +             fbe = !!XGMAC_GET_BITS(dma_ch_isr, DMA_CH_SR, FBE);
> 
> Please split this into two patches, one pure refactoring with no
> functional changes and second one changing RBU handling.

Sure, that makes sense.

> 
>> +             if (rbu) {
>> +                     schedule_napi = true;
>> +                     pdata->ext_stats.rx_buffer_unavailable++;
>> +                     netif_dbg(pdata, intr, pdata->netdev,
>> +                               "RBU on DMA_CH %u, scheduling %s NAPI\n",
>> +                               i, per_ch_irq ? "per-channel" : "global");
> 
> I guess it's just _dbg() but as a general rule when the system is under
> overload printing stuff (potentially over UART) is the last thing you
> should be doing. How is this print useful to you?

Wanted to let the user know about RBU. But this can be dropped to avoid 
additional load on the UART.

> --
> pw-bot: cr

Re: [PATCH net-next] amd-xgbe: schedule NAPI on Rx Buffer Unavailable to prevent RX stalls

[Jakub Kicinski]

On Fri, 28 Nov 2025 11:20:09 +0530 Rangoju, Raju wrote:
> > On Mon, 24 Nov 2025 15:41:11 +0530 Raju Rangoju wrote:  
> >> When Rx Buffer Unavailable (RBU) interrupt is asserted, the device can
> >> stall under load and suffer prolonged receive starvation if polling is
> >> not initiated. Treat RBU as a wakeup source and schedule the appropriate
> >> NAPI instance (per-channel or global) to promptly recover from buffer
> >> shortages and refill descriptors.  
> > 
> > You need to say more.. Under heavy load network devices will routinely
> > run out of Rx buffers, it's expected if Rx processing is slower than
> > the network. What hw condition and scenario exactly are you describing
> > here?  
> 
> During the bi-directional traffic device is running out of RX buffers, 
> it could be because of slower rx processing. HW notifies this via Rx 
> Buffer Unavailable (RBU) interrupt. What is being described above is 
> that, driver should treat RBU interrupt as source to trigger the NAPI 
> poll immediately, rather than waiting for regular rx interrupts to 
> process the rx buffers.

Ack, all I'm saying is that the commit message seems to be overselling
the impact of this change. Patch is very very unlikely to make anything
more "prompt". 99% of the time if Rx buffers are not refilled we are
either in OOM or Rx overload, so either we won't be able to alloc the
buffers, or NAPI is already scheduled. But of course trying to schedule
the NAPI does seem like the more correct reaction, in case we missed an
IRQ or such. Maybe rephrase a little.. unless there's some magic here
im not aware of

Re: [PATCH net-next] amd-xgbe: schedule NAPI on Rx Buffer Unavailable to prevent RX stalls

[Rangoju, Raju]

On 11/29/2025 12:08 AM, Jakub Kicinski wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Fri, 28 Nov 2025 11:20:09 +0530 Rangoju, Raju wrote:
>>> On Mon, 24 Nov 2025 15:41:11 +0530 Raju Rangoju wrote:
>>>> When Rx Buffer Unavailable (RBU) interrupt is asserted, the device can
>>>> stall under load and suffer prolonged receive starvation if polling is
>>>> not initiated. Treat RBU as a wakeup source and schedule the appropriate
>>>> NAPI instance (per-channel or global) to promptly recover from buffer
>>>> shortages and refill descriptors.
>>>
>>> You need to say more.. Under heavy load network devices will routinely
>>> run out of Rx buffers, it's expected if Rx processing is slower than
>>> the network. What hw condition and scenario exactly are you describing
>>> here?
>>
>> During the bi-directional traffic device is running out of RX buffers,
>> it could be because of slower rx processing. HW notifies this via Rx
>> Buffer Unavailable (RBU) interrupt. What is being described above is
>> that, driver should treat RBU interrupt as source to trigger the NAPI
>> poll immediately, rather than waiting for regular rx interrupts to
>> process the rx buffers.
> 
> Ack, all I'm saying is that the commit message seems to be overselling
> the impact of this change. Patch is very very unlikely to make anything
> more "prompt". 99% of the time if Rx buffers are not refilled we are
> either in OOM or Rx overload, so either we won't be able to alloc the
> buffers, or NAPI is already scheduled. But of course trying to schedule
> the NAPI does seem like the more correct reaction, in case we missed an
> IRQ or such. Maybe rephrase a little.. unless there's some magic here
> im not aware of

There's no magic as such. The patch is all about trying to schedule the 
NAPI when an RBU interrupt is received. I'll rephrase the message. Thx.