* [PATCH net-next] bnxt: Detach page from page pool before sending up the stack
@ 2020-01-09 19:35 Jonathan Lemon
2020-01-10 2:19 ` Andy Gospodarek
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jonathan Lemon @ 2020-01-09 19:35 UTC (permalink / raw)
To: netdev, gospo, michael.chan; +Cc: davem, kernel-team
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.
Fixes: 322b87ca55f2 ("bnxt_en: add page_pool support")
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] bnxt: Detach page from page pool before sending up the stack
2020-01-09 19:35 [PATCH net-next] bnxt: Detach page from page pool before sending up the stack Jonathan Lemon
@ 2020-01-10 2:19 ` Andy Gospodarek
[not found] ` <402D3501-9EED-49F3-A5C8-003F78DD0D59@gmail.com>
2020-01-10 19:19 ` Andy Gospodarek
2020-01-11 7:04 ` David Miller
2 siblings, 1 reply; 5+ messages in thread
From: Andy Gospodarek @ 2020-01-10 2:19 UTC (permalink / raw)
To: Jonathan Lemon; +Cc: netdev, michael.chan, davem, kernel-team
On Thu, Jan 09, 2020 at 11:35:42AM -0800, Jonathan Lemon wrote:
> 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!
>
> Fixes: 322b87ca55f2 ("bnxt_en: add page_pool support")
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] bnxt: Detach page from page pool before sending up the stack
[not found] ` <402D3501-9EED-49F3-A5C8-003F78DD0D59@gmail.com>
@ 2020-01-10 18:42 ` Andy Gospodarek
0 siblings, 0 replies; 5+ messages in thread
From: Andy Gospodarek @ 2020-01-10 18:42 UTC (permalink / raw)
To: Jonathan Lemon; +Cc: Andy Gospodarek, netdev, michael.chan, davem, kernel-team
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:
>
> > On Thu, Jan 09, 2020 at 11:35:42AM -0800, Jonathan Lemon wrote:
> > > 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!
> >
> > >
> > > Fixes: 322b87ca55f2 ("bnxt_en: add page_pool support")
> > > Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> > > ---
> > > 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
> > >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] bnxt: Detach page from page pool before sending up the stack
2020-01-09 19:35 [PATCH net-next] bnxt: Detach page from page pool before sending up the stack Jonathan Lemon
2020-01-10 2:19 ` Andy Gospodarek
@ 2020-01-10 19:19 ` Andy Gospodarek
2020-01-11 7:04 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: Andy Gospodarek @ 2020-01-10 19:19 UTC (permalink / raw)
To: Jonathan Lemon; +Cc: netdev, michael.chan, davem, kernel-team
On Thu, Jan 09, 2020 at 11:35:42AM -0800, Jonathan Lemon wrote:
> 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.
Looks good. Thanks for the patch!
> Fixes: 322b87ca55f2 ("bnxt_en: add page_pool support")
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] bnxt: Detach page from page pool before sending up the stack
2020-01-09 19:35 [PATCH net-next] bnxt: Detach page from page pool before sending up the stack Jonathan Lemon
2020-01-10 2:19 ` Andy Gospodarek
2020-01-10 19:19 ` Andy Gospodarek
@ 2020-01-11 7:04 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2020-01-11 7:04 UTC (permalink / raw)
To: jonathan.lemon; +Cc: netdev, gospo, michael.chan, kernel-team
From: Jonathan Lemon <jonathan.lemon@gmail.com>
Date: Thu, 9 Jan 2020 11:35:42 -0800
> 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.
>
> Fixes: 322b87ca55f2 ("bnxt_en: add page_pool support")
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Applied, thank you.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-11 7:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-09 19:35 [PATCH net-next] bnxt: Detach page from page pool before sending up the stack Jonathan Lemon
2020-01-10 2:19 ` Andy Gospodarek
[not found] ` <402D3501-9EED-49F3-A5C8-003F78DD0D59@gmail.com>
2020-01-10 18:42 ` Andy Gospodarek
2020-01-10 19:19 ` Andy Gospodarek
2020-01-11 7:04 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.