On 2023/12/20 11:01, Mina Almasry wrote: ... >>>> Perhaps we should aim to not export netmem_to_page(), >>>> prevent modules from accessing it directly. >>> >>> +1. >> > > I looked into this, but it turns out it's a slightly bigger change > that needs some refactoring to make it work. There are few places > where I believe I need to add netmem_to_page() that are exposed to the > drivers via inline helpers, these are: > > - skb_frag_page(), which returns NULL if the netmem is not a page, but > needs to do a netmem_to_page() to return the page otherwise. Is it possible to introduce something like skb_frag_netmem() for netmem? so that we can keep most existing users of skb_frag_page() unchanged and avoid adding additional checking overhead for existing users. > - The helpers inside skb_add_rx_frag(), which needs to do a > netmem_to_page() to set skb->pfmemalloc. Similar as above, perhaps introduce something like skb_add_rx_netmem_frag()? > - Some of the page_pool APIs are exposed to the drivers as static > inline helpers, and if I want the page_pool to use netmem internally > the page_pool needs to do a netmem_to_page() in these helpers. > > The refactor is not an issue, but I was wondering if not exporting > netmem_to_page() was worth moving the code around. I was thinking in > the interim until netmem is adopted and has actual driver users we may > prefer to just add a comment on the netmem_to_page() helper that says > 'try not to use this directly and use the netmem helpers instead'. >