[PATCH net-next v2 0/2] page_pool: bnxt_en: unlink old page pool in queue api using helper

[PATCH net-next v2 0/2] page_pool: bnxt_en: unlink old page pool in queue api using helper

[David Wei]

56ef27e3 unexported page_pool_unlink_napi() and renamed it to
page_pool_disable_direct_recycling(). This is because there was no
in-tree user of page_pool_unlink_napi().

Since then Rx queue API and an implementation in bnxt got merged. In the
bnxt implementation, it broadly follows the following steps: allocate
new queue memory + page pool, stop old rx queue, swap, then destroy old
queue memory + page pool.

The existing NAPI instance is re-used so when the old page pool that is
no longer used but still linked to this shared NAPI instance is
destroyed, it will trigger warnings.

In my initial patches I unlinked a page pool from a NAPI instance
directly. Instead, export page_pool_disable_direct_recycling() and call
that instead to avoid having a driver touch a core struct.

David Wei (2):
  page_pool: export page_pool_disable_direct_recycling()
  bnxt_en: unlink page pool when stopping Rx queue

 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 +-----
 include/net/page_pool/types.h             | 1 +
 net/core/page_pool.c                      | 3 ++-
 3 files changed, 4 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH net-next v2 1/2] page_pool: export page_pool_disable_direct_recycling()

[David Wei]

56ef27e3 unexported page_pool_unlink_napi() and renamed it to
page_pool_disable_direct_recycling(). This is because there was no
in-tree user of page_pool_unlink_napi().

Since then Rx queue API and an implementation in bnxt got merged. In the
bnxt implementation, it broadly follows the following steps: allocate
new queue memory + page pool, stop old rx queue, swap, then destroy old
queue memory + page pool.

The existing NAPI instance is re-used so when the old page pool that is
no longer used but still linked to this shared NAPI instance is
destroyed, it will trigger warnings.

In my initial patches I unlinked a page pool from a NAPI instance
directly. Instead, export page_pool_disable_direct_recycling() and call
that instead to avoid having a driver touch a core struct.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David Wei <dw@davidwei.uk>
---
 include/net/page_pool/types.h | 1 +
 net/core/page_pool.c          | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index 7e8477057f3d..9093a964fc33 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -229,6 +229,7 @@ struct page_pool *page_pool_create_percpu(const struct page_pool_params *params,
 struct xdp_mem_info;
 
 #ifdef CONFIG_PAGE_POOL
+void page_pool_disable_direct_recycling(struct page_pool *pool);
 void page_pool_destroy(struct page_pool *pool);
 void page_pool_use_xdp_mem(struct page_pool *pool, void (*disconnect)(void *),
 			   const struct xdp_mem_info *mem);
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 3927a0a7fa9a..5f1ed6f2ca8f 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -1014,7 +1014,7 @@ void page_pool_use_xdp_mem(struct page_pool *pool, void (*disconnect)(void *),
 	pool->xdp_mem_id = mem->id;
 }
 
-static void page_pool_disable_direct_recycling(struct page_pool *pool)
+void page_pool_disable_direct_recycling(struct page_pool *pool)
 {
 	/* Disable direct recycling based on pool->cpuid.
 	 * Paired with READ_ONCE() in page_pool_napi_local().
@@ -1032,6 +1032,7 @@ static void page_pool_disable_direct_recycling(struct page_pool *pool)
 
 	WRITE_ONCE(pool->p.napi, NULL);
 }
+EXPORT_SYMBOL(page_pool_disable_direct_recycling);
 
 void page_pool_destroy(struct page_pool *pool)
 {
-- 
2.43.0

[PATCH net-next v2 2/2] bnxt_en: unlink page pool when stopping Rx queue

[David Wei]

Have bnxt call page_pool_disable_direct_recycling() to unlink the old
page pool when resetting a queue prior to destroying it, instead of
touching a netdev core struct directly.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 1bd0c5973252..18ff747a57fb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -15049,11 +15049,6 @@ static void bnxt_queue_mem_free(struct net_device *dev, void *qmem)
 	bnxt_free_one_rx_ring(bp, rxr);
 	bnxt_free_one_rx_agg_ring(bp, rxr);
 
-	/* At this point, this NAPI instance has another page pool associated
-	 * with it. Disconnect here before freeing the old page pool to avoid
-	 * warnings.
-	 */
-	rxr->page_pool->p.napi = NULL;
 	page_pool_destroy(rxr->page_pool);
 	rxr->page_pool = NULL;
 
@@ -15173,6 +15168,7 @@ static int bnxt_queue_stop(struct net_device *dev, void *qmem, int idx)
 	bnxt_hwrm_rx_ring_free(bp, rxr, false);
 	bnxt_hwrm_rx_agg_ring_free(bp, rxr, false);
 	rxr->rx_next_cons = 0;
+	page_pool_disable_direct_recycling(rxr->page_pool);
 
 	memcpy(qmem, rxr, sizeof(*rxr));
 	bnxt_init_rx_ring_struct(bp, qmem);
-- 
2.43.0

Re: [PATCH net-next v2 0/2] page_pool: bnxt_en: unlink old page pool in queue api using helper

[Jakub Kicinski]

On Wed, 26 Jun 2024 20:01:58 -0700 David Wei wrote:
> In my initial patches I unlinked a page pool from a NAPI instance
> directly. Instead, export page_pool_disable_direct_recycling() and call
> that instead to avoid having a driver touch a core struct.

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH net-next v2 0/2] page_pool: bnxt_en: unlink old page pool in queue api using helper

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 26 Jun 2024 20:01:58 -0700 you wrote:
> 56ef27e3 unexported page_pool_unlink_napi() and renamed it to
> page_pool_disable_direct_recycling(). This is because there was no
> in-tree user of page_pool_unlink_napi().
> 
> Since then Rx queue API and an implementation in bnxt got merged. In the
> bnxt implementation, it broadly follows the following steps: allocate
> new queue memory + page pool, stop old rx queue, swap, then destroy old
> queue memory + page pool.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/2] page_pool: export page_pool_disable_direct_recycling()
    https://git.kernel.org/netdev/net-next/c/d7f39aee79f0
  - [net-next,v2,2/2] bnxt_en: unlink page pool when stopping Rx queue
    https://git.kernel.org/netdev/net-next/c/40eca00ae605

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html