On 7/14/25 20:37, Mina Almasry wrote:
On Mon, Jul 14, 2025 at 5:01 AM Byungchul Park <byungc...@sk.com> wrote:
...>> +static inline struct netmem_desc *pp_page_to_nmdesc(struct page *page)
+{ + DEBUG_NET_WARN_ON_ONCE(!page_pool_page_is_pp(page)); + + /* XXX: How to extract netmem_desc from page must be changed, + * once netmem_desc no longer overlays on page and will be + * allocated through slab. + */ + return (struct netmem_desc *)page; +} +Same thing. Do not create a generic looking pp_page_to_nmdesc helper which does not check that the page is the correct type. The DEBUG_NET... is not good enough. You don't need to add a generic helper here. There is only one call site. Open code this in the callsite. The one callsite is marked as unsafe, only called by code that knows that the netmem is specifically a pp page. Open code this in the unsafe callsite, instead of creating a generic looking unsafe helper and not even documenting it's unsafe./** * __netmem_get_pp - unsafely get pointer to the &page_pool backing @netmem * @netmem: netmem reference to get the pointer from @@ -280,7 +291,7 @@ static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem) */ static inline struct page_pool *__netmem_get_pp(netmem_ref netmem) { - return __netmem_to_page(netmem)->pp; + return pp_page_to_nmdesc(__netmem_to_page(netmem))->pp; }This makes me very sad. Casting from netmem -> page -> nmdesc...
The function is not used, and I don't think the series adds any new users? It can be killed then. It's a horrible function anyway, would be much better to have a variant taking struct page * if necessary.
Instead, we should be able to go from netmem directly to nmdesc. I would suggest rename __netmem_clear_lsb to netmem_to_nmdesc and have it return netmem_desc instead of net_iov. Then use it here.
Glad you liked the diff I suggested :) In either case, seems like it's not strictly necessary for this iteration as __netmem_get_pp() should be killed, and the rest of patches work directly with pages.
We could have an unsafe version of netmem_to_nmdesc which converts the netmem to netmem_desc without clearing the lsb and mark it unsafe.
-- Pavel Begunkov
