On Thu, Dec 21, 2023 at 3:32 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote: > > 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. > In my experience most current skb_frag_page() users need specifically the struct page*. Example is illegal_highdma() which PageHighMem(skb_frag_page()) But RFC v5 adds skb_frag_netmem() for callsites that want a netmem and don't care about specifically a page: https://patchwork.kernel.org/project/netdevbpf/patch/20231218024024.3516870-10-almasrymina@xxxxxxxxxx/ > > - 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()? > Yes, v3 of this series adds skb_add_rx_frag_netmem(): https://patchwork.kernel.org/project/netdevbpf/patch/20231220214505.2303297-4-almasrymina@xxxxxxxxxx/ > > - 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'. > > > -- Thanks, Mina
