* [PATCH net] net: page_pool: replace ASSERT_RTNL() in page_pool_init()
@ 2025-03-24 1:46 David Wei
2025-03-24 2:39 ` Stanislav Fomichev
2025-03-24 14:18 ` Andrew Lunn
0 siblings, 2 replies; 5+ messages in thread
From: David Wei @ 2025-03-24 1:46 UTC (permalink / raw)
To: netdev
Cc: Jakub Kicinski, Paolo Abeni, David S. Miller, Eric Dumazet,
Jesper Dangaard Brouer, Stanislav Fomichev
Replace a stray ASSERT_RTNL() in page_pool_init() with
netdev_assert_locked().
Fixes: 1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations")
Signed-off-by: David Wei <dw@davidwei.uk>
---
net/core/page_pool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index f5e908c9e7ad..2f469b02ea31 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -281,7 +281,7 @@ static int page_pool_init(struct page_pool *pool,
* configuration doesn't change while we're initializing
* the page_pool.
*/
- ASSERT_RTNL();
+ netdev_assert_locked(params->netdev);
rxq = __netif_get_rx_queue(pool->slow.netdev,
pool->slow.queue_idx);
pool->mp_priv = rxq->mp_params.mp_priv;
--
2.47.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] net: page_pool: replace ASSERT_RTNL() in page_pool_init()
2025-03-24 1:46 [PATCH net] net: page_pool: replace ASSERT_RTNL() in page_pool_init() David Wei
@ 2025-03-24 2:39 ` Stanislav Fomichev
2025-03-24 18:24 ` David Wei
2025-03-24 14:18 ` Andrew Lunn
1 sibling, 1 reply; 5+ messages in thread
From: Stanislav Fomichev @ 2025-03-24 2:39 UTC (permalink / raw)
To: David Wei
Cc: netdev, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Jesper Dangaard Brouer
On 03/23, David Wei wrote:
> Replace a stray ASSERT_RTNL() in page_pool_init() with
> netdev_assert_locked().
>
> Fixes: 1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations")
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
> net/core/page_pool.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index f5e908c9e7ad..2f469b02ea31 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -281,7 +281,7 @@ static int page_pool_init(struct page_pool *pool,
> * configuration doesn't change while we're initializing
> * the page_pool.
> */
> - ASSERT_RTNL();
> + netdev_assert_locked(params->netdev);
> rxq = __netif_get_rx_queue(pool->slow.netdev,
> pool->slow.queue_idx);
> pool->mp_priv = rxq->mp_params.mp_priv;
Not sure we can do this unconditionally. Since we still have plenty of
drivers that might not use netdev instance lock (yet). Probably should
be something like the following?
if (netdev_need_ops_lock(params->netdev))
netdev_assert_locked(params->netdev);
else
ASSERT_RTNL();
?
We should probably wait for https://lore.kernel.org/netdev/20250312223507.805719-11-kuba@kernel.org/T/#m2053ac617759a9005806e56a7df97c378b76ec77
to land and use netdev_ops_assert_locked instead?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] net: page_pool: replace ASSERT_RTNL() in page_pool_init()
2025-03-24 2:39 ` Stanislav Fomichev
@ 2025-03-24 18:24 ` David Wei
0 siblings, 0 replies; 5+ messages in thread
From: David Wei @ 2025-03-24 18:24 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Jesper Dangaard Brouer
On 2025-03-23 19:39, Stanislav Fomichev wrote:
> On 03/23, David Wei wrote:
>> Replace a stray ASSERT_RTNL() in page_pool_init() with
>> netdev_assert_locked().
>>
>> Fixes: 1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations")
>> Signed-off-by: David Wei <dw@davidwei.uk>
>> ---
>> net/core/page_pool.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index f5e908c9e7ad..2f469b02ea31 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -281,7 +281,7 @@ static int page_pool_init(struct page_pool *pool,
>> * configuration doesn't change while we're initializing
>> * the page_pool.
>> */
>> - ASSERT_RTNL();
>> + netdev_assert_locked(params->netdev);
>> rxq = __netif_get_rx_queue(pool->slow.netdev,
>> pool->slow.queue_idx);
>> pool->mp_priv = rxq->mp_params.mp_priv;
>
> Not sure we can do this unconditionally. Since we still have plenty of
> drivers that might not use netdev instance lock (yet). Probably should
> be something like the following?
>
> if (netdev_need_ops_lock(params->netdev))
> netdev_assert_locked(params->netdev);
> else
> ASSERT_RTNL();
>
> ?
>
> We should probably wait for https://lore.kernel.org/netdev/20250312223507.805719-11-kuba@kernel.org/T/#m2053ac617759a9005806e56a7df97c378b76ec77
> to land and use netdev_ops_assert_locked instead?
Ah I didn't realise that migration hasn't entirely happened. Yes, I'll
wait for netdev_ops_assert_locked() first.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: page_pool: replace ASSERT_RTNL() in page_pool_init()
2025-03-24 1:46 [PATCH net] net: page_pool: replace ASSERT_RTNL() in page_pool_init() David Wei
2025-03-24 2:39 ` Stanislav Fomichev
@ 2025-03-24 14:18 ` Andrew Lunn
2025-03-24 18:25 ` David Wei
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2025-03-24 14:18 UTC (permalink / raw)
To: David Wei
Cc: netdev, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Jesper Dangaard Brouer, Stanislav Fomichev
On Sun, Mar 23, 2025 at 06:46:39PM -0700, David Wei wrote:
> Replace a stray ASSERT_RTNL() in page_pool_init() with
> netdev_assert_locked().
>
> Fixes: 1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations")
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
> net/core/page_pool.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index f5e908c9e7ad..2f469b02ea31 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -281,7 +281,7 @@ static int page_pool_init(struct page_pool *pool,
> * configuration doesn't change while we're initializing
> * the page_pool.
> */
> - ASSERT_RTNL();
> + netdev_assert_locked(params->netdev);
Adding a bit more context:
if (pool->slow.flags & PP_FLAG_ALLOW_UNREADABLE_NETMEM) {
/* We rely on rtnl_lock()ing to make sure netdev_rx_queue
* configuration doesn't change while we're initializing
* the page_pool.
*/
ASSERT_RTNL();
If ASSERT_RTNL() is now wrong, you also need to update the comment.
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] net: page_pool: replace ASSERT_RTNL() in page_pool_init()
2025-03-24 14:18 ` Andrew Lunn
@ 2025-03-24 18:25 ` David Wei
0 siblings, 0 replies; 5+ messages in thread
From: David Wei @ 2025-03-24 18:25 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Jesper Dangaard Brouer, Stanislav Fomichev
On 2025-03-24 07:18, Andrew Lunn wrote:
> On Sun, Mar 23, 2025 at 06:46:39PM -0700, David Wei wrote:
>> Replace a stray ASSERT_RTNL() in page_pool_init() with
>> netdev_assert_locked().
>>
>> Fixes: 1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations")
>> Signed-off-by: David Wei <dw@davidwei.uk>
>> ---
>> net/core/page_pool.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index f5e908c9e7ad..2f469b02ea31 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -281,7 +281,7 @@ static int page_pool_init(struct page_pool *pool,
>> * configuration doesn't change while we're initializing
>> * the page_pool.
>> */
>> - ASSERT_RTNL();
>> + netdev_assert_locked(params->netdev);
>
> Adding a bit more context:
>
> if (pool->slow.flags & PP_FLAG_ALLOW_UNREADABLE_NETMEM) {
> /* We rely on rtnl_lock()ing to make sure netdev_rx_queue
> * configuration doesn't change while we're initializing
> * the page_pool.
> */
> ASSERT_RTNL();
>
> If ASSERT_RTNL() is now wrong, you also need to update the comment.
Thanks, I'll update the docs too after switching to
netdev_ops_assert_locked().
>
> Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-24 18:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-24 1:46 [PATCH net] net: page_pool: replace ASSERT_RTNL() in page_pool_init() David Wei
2025-03-24 2:39 ` Stanislav Fomichev
2025-03-24 18:24 ` David Wei
2025-03-24 14:18 ` Andrew Lunn
2025-03-24 18:25 ` David Wei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).