RE: [EXTERNAL] [PATCH net-next v2 2/3] net: atlantic: free RX pages of consumed but not refilled buffers

[Sukhdeep Soni [C]]

On 24 july 2026, Yangyu Chen wrote:

> aq_ring_rx_deinit() only walks [sw_head, sw_tail), the region posted to
> hardware. Since the page reuse strategy was added, a cleaned RX buffer
> keeps its page (and its DMA mapping) in the ring for reuse, and refill
> is batched: aq_ring_rx_fill() returns early until AQ_CFG_RX_REFILL_THRES
> slots are free. Slots that were consumed but not yet reposted therefore
> sit in the complementary [sw_tail, sw_head) gap with a live page, and
> the deinit walk never visits them: up to a refill batch worth of pages
> and DMA mappings leak on every interface down.
> 
> Walk the whole ring instead and release whatever is still there. Also
> bail out if the buffer ring is already gone: a partial
> aq_ptp_ring_alloc() failure frees the ring but leaves aq_nic set, so
> aq_ptp_ring_deinit() still gets here on the unwind path.
> 
> Fixes: 46f4c29d9de6 ("net: aquantia: optimize rx performance by page reuse strategy")
> Cc: mailto:stable@xxxxxxxxxxxxxxx # v5.2+
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Yangyu Chen <mailto:cyy@xxxxxxxxxxxx>
> ---
>
> Notes:
>    Without this fix, the following page_pool conversion turns the missed
>    pages into fragments that page_pool_destroy() waits for forever.
>    Reproduced on an AQC100 with the conversion applied and this fix
>    reverted -- ordinary small received frames (<= 256 byte header-only
>    packets, e.g. ping replies or pure TCP ACKs) are enough to populate
>    the [sw_tail, sw_head) gap:
>    
>      ping -c 200 -i 0.005 <peer>%enp99s0
>      ip link set enp99s0 down
>    
>    One short ping flow left three of the eight RX rings' pools with
>    stranded fragments, and page_pool_release_retry() warns for each of
>    them every 60 seconds, indefinitely:
>    
>      [278084.929092] page_pool_release_retry() stalled pool shutdown: id 123, 1 inflight 60 sec
>      [278084.961064] page_pool_release_retry() stalled pool shutdown: id 126, 1 inflight 60 sec
>      [278084.961087] page_pool_release_retry() stalled pool shutdown: id 125, 6 inflight 60 sec
>      [278145.346737] page_pool_release_retry() stalled pool shutdown: id 123, 1 inflight 120 sec
>      [278145.378745] page_pool_release_retry() stalled pool shutdown: id 125, 6 inflight 120 sec
>      [278145.378759] page_pool_release_retry() stalled pool shutdown: id 126, 1 inflight 120 sec
>    
>    With this patch the whole ring is walked at deinit, the pages are
>    released, and the pools drain immediately. On the current code the
>    same gap leaks the pages and their DMA mappings silently.
>    
>    Applies and was build- and runtime-tested independently of the rest
>    of this series, against the current page scheme.
>    
>    New in v2: split out of the page_pool conversion and made a
>    standalone fix for the existing page reuse scheme (Mina); Cc stable
>    with the affected range.
>
> .../net/ethernet/aquantia/atlantic/aq_ring.c  | 22 +++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> index 81685a4dc5a6..e1193c6719d9 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> @@ -950,15 +950,29 @@ int aq_ring_rx_fill(struct aq_ring_s *self)
> 
> void aq_ring_rx_deinit(struct aq_ring_s *self)
> {
> -	if (!self)
> +	unsigned int i;
> +
> +	if (!self || !self->buff_ring)
> 		return;
> 
> -	for (; self->sw_head != self->sw_tail;
> -		self->sw_head = aq_ring_next_dx(self, self->sw_head)) {
> -		struct aq_ring_buff_s *buff = &self->buff_ring[self->sw_head];
> +	/* Release every page still owned by the ring.
> +	 *
> +	 * Walking [sw_head, sw_tail) is not enough: refill is batched
> +	 * (aq_ring_rx_fill() waits for AQ_CFG_RX_REFILL_THRES free slots),
> +	 * so slots that were cleaned but not yet reposted accumulate in the
> +	 * [sw_tail, sw_head) gap, and they keep their page for reuse. Walk
> +	 * the whole ring and release whatever is left.
> +	 */
> +	for (i = 0; i < self->size; i++) {
> +		struct aq_ring_buff_s *buff = &self->buff_ring[i];
> +
> +		if (!buff->rxdata.page)
> +			continue;
> 
> 		aq_free_rxpage(&buff->rxdata, aq_nic_get_dev(self->aq_nic));
> 	}
> +
> +	self->sw_head = self->sw_tail;
> }
> 
> void aq_ring_free(struct aq_ring_s *self)
> -- 
> 2.47.3

Reviewed-by: Sukhdeep Singh <sukhdeeps@xxxxxxxxxxx>