Re: [RFC net-next v1 5/6] net: page_pool: convert refcounting helpers to nmdesc

On 8/13/25 17:55, Mina Almasry wrote:

On Wed, Aug 13, 2025 at 2:10 AM Pavel Begunkov [off-list ref] wrote:

quoted

...>>   static inline bool page_pool_unref_and_test(netmem_ref netmem)

quoted

quoted

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 24c591ab38ae..e084dad11506 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c

@@ -440,14 +440,9 @@ void mp_dmabuf_devmem_destroy(struct page_pool *pool)

   bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem)
   {
-       long refcount = atomic_long_read(netmem_get_pp_ref_count_ref(netmem));
-
          if (WARN_ON_ONCE(!netmem_is_net_iov(netmem)))
                  return false;

-       if (WARN_ON_ONCE(refcount != 1))
-               return false;
-

Rest of the patch looks good to me, but this comes across as a
completely unrelated clean up/change or something? Lets keep the
WARN_ON_ONCE?

I was killing netmem_get_pp_ref_count_ref(), which is why it's here.
It checks an assumption that's guaranteed by page pools and shared
with non-mp pools, so not like devmem needs it, and it'd not catch
any recycling problems either. Regardless, I can leave the warning.

Ack. I also agree the WARN_ON_ONCE is not necessary, even the one
above it for the net_iov check is not necessary.

But since devmem was the first memory provider I'm paranoid that I got
something wrong in the general memory provider infra or in the devmem
implementation specifically; I think some paranoid WARN_ON_ONCEs are
justified, maybe. I'd rather debug the warning firing rather than a
very subtle refcounting issue or provider mixup or something at a
later point. I'm still surprised there aren't many bug reports about
any memory providers. They probably aren't used much in production
yet.

I think after 2025 or 2026 LTS it's probably time to clean up these
unnecessary WARN_ONs, but until then, let's keep them in if you don't
mind.

No problem, thanks for the review!

-- 
Pavel Begunkov