Re: [PATCH net-next] bnxt: Detach page from page pool before sending up the stack

On Fri, Jan 10, 2020 at 09:14:42AM -0800, Jonathan Lemon wrote:

Sure, on our FB systems, the reproduction is simple:

- Start an XDP program (which uses the page pool).
  In our case, it's droplet.
- Send normal traffic, so pages are released to the system.
- Restart droplet (or do something which causes the
  driver to re-initialize)  This frees and reallocates the
  rings and page pool.

After a bit, observe incessant warnings about being unable
to release the page pool due to pages inflight:

Jan  4 03:19:17 twtraffic0235.06.atn5.facebook.com kernel: [1941287.997351]
page_pool_release_retry() stalled pool shutdown -1783494730 inflight 1451031
sec
Jan  4 03:19:24 twtraffic0235.06.atn5.facebook.com kernel: [1941295.031546]
page_pool_release_retry() stalled pool shutdown -1962791719 inflight 1451040
sec
Jan  4 03:19:38 twtraffic0235.06.atn5.facebook.com kernel: [1941308.131831]
page_pool_release_retry() stalled pool shutdown -1978246510 inflight 1451052
sec
Jan  4 03:19:57 twtraffic0235.06.atn5.facebook.com kernel: [1941327.842882]
page_pool_release_retry() stalled pool shutdown -1893208809 inflight 1451072
sec
Jan  4 03:19:58 twtraffic0235.06.atn5.facebook.com kernel: [1941328.332587]
page_pool_release_retry() stalled pool shutdown -1822809109 inflight 1451070
sec
Jan  4 03:20:10 twtraffic0235.06.atn5.facebook.com kernel: [1941340.994990]
page_pool_release_retry() stalled pool shutdown -2064970262 inflight 1451087
sec

Also note that if the count is negative, this triggers a
kernel stack trace, which has a deleterious affect on the
performance of the system.

Thanks.  I actually tried something similar this morning before you sent
this and was unable to reproduce at first.

I thought about it more and realized that I needed to send packets
larger than BNXT_RX_COPY_THRESH.  I see it now and will have more
feedback in a few mins.

On 9 Jan 2020, at 18:19, Andy Gospodarek wrote:

quoted

On Thu, Jan 09, 2020 at 11:35:42AM -0800, Jonathan Lemon wrote:

quoted

When running in XDP mode, pages come from the page pool, and should
be freed back to the same pool or specifically detached.  Currently,
when the driver re-initializes, the page pool destruction is delayed
forever since it thinks there are oustanding pages.

If you can please share a reproduction script/steps that would be
helpful for me.

Since this is an XDP_PASS case I can easily create a program that does
that, so no need to share that program -- just the sequence to remove
the program, shutdown the driver, whatever is done and the error you
see.

Thanks!

quoted

Fixes: 322b87ca55f2 ("bnxt_en: add page_pool support")
Signed-off-by: Jonathan Lemon <redacted>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c

b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 39d4309b17fb..33eb8cd6551e 100644

--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c

@@ -944,6 +944,7 @@ static struct sk_buff *bnxt_rx_page_skb(struct

bnxt *bp,
 	dma_addr -= bp->rx_dma_offset;
 	dma_unmap_page_attrs(&bp->pdev->dev, dma_addr, PAGE_SIZE,
bp->rx_dir,
 			     DMA_ATTR_WEAK_ORDERING);
+	page_pool_release_page(rxr->page_pool, page);

 	if (unlikely(!payload))
 		payload = eth_get_headlen(bp->dev, data_ptr, len);
-- 
2.17.1