Re: [PATCH net 1/2] net: macb: reprogram TBQP after shuffling the TX ring on link-up
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: 2026-07-06 15:04:25
Also in:
linux-rt-devel, lkml, stable
On 2026-07-06 16:02:14 [+0200], Christian Taedcke via B4 Relay wrote:
From: Christian Taedcke <redacted> gem_shuffle_tx_one_ring() rotates the software TX ring so that the tail sits at index 0 and resets queue->tx_tail to 0, but it never reprograms the hardware transmit buffer queue pointer (TBQP). Other paths that reset tx_tail to the ring base (macb_init_buffers() and macb_tx_error_task()) also reprogram TBQP to queue->tx_ring_dma; this path does not, leaving TBQP pointing at a stale descriptor. gem_shuffle_tx_rings() runs on every link-up from macb_mac_link_up(). After a few link up/down flaps that leave un-completed descriptors in the ring, the stale TBQP keeps pointing at a descriptor whose used bit is set. When TX is re-enabled on link-up, the GEM reads that used descriptor and raises TXUBR. macb_interrupt() schedules the TX NAPI, macb_tx_poll() makes no progress (work_done == 0) and macb_tx_restart() re-issues TSTART, which makes the controller read the same used descriptor again and re-assert TXUBR. As the MAC interrupt is level-triggered, it never deasserts and one CPU is pegged at 100% in the threaded handler, eventually triggering "sched: RT throttling activated" and a dead network interface.
But this should also happen with !RT at which point the interrupt runs at 100% CPU and the softirq has hardly an chance to make progress, no?
Fix it by reprogramming TBQP to the ring base on every path of
gem_shuffle_tx_one_ring() that resets tx_tail to 0, mirroring
macb_tx_error_task(). The early return for an already-aligned tail is
left untouched as TBQP is already consistent there. This is safe
because the shuffle runs from macb_mac_link_up() while TE is still
disabled, so the transmitter is halted.
Fixes: 881a0263d502 ("net: macb: Shuffle the tx ring before enabling tx")This is v7.0-rc4. So that RT tree of yours has some backports or did you run into this while trying to reproduce it upstream?
quoted hunk ↗ jump to hunk
Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Christian Taedcke <redacted> --- drivers/net/ethernet/cadence/macb_main.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index fd282a1700fb..b11cb8f068b7 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c@@ -820,7 +820,7 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue) if (!count) { queue->tx_head = 0; queue->tx_tail = 0; - goto unlock; + goto reset_hw_ptr;
This update is even needed for count == 0 case? I kind of do understand that you need to updated if you shuffled the descriptors around.
quoted hunk ↗ jump to hunk
} shift = tail % ring_size;@@ -869,6 +869,13 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue) /* Make descriptor updates visible to hardware */ wmb(); +reset_hw_ptr: + /* tx_tail was reset to the ring base, so TBQP must be reprogrammed + * to match; otherwise it keeps pointing at a stale descriptor. Safe + * to write directly here as TX is still disabled (called from + * macb_mac_link_up() before TE is set). + */ + queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma)); unlock: spin_unlock_irqrestore(&queue->tx_ptr_lock, flags); }
Sebastian