From: Yunsheng Lin <linyunsheng@xxxxxxxxxx> Date: Mon, 6 Mar 2023 09:09:31 +0800 > On 2023/3/3 21:26, Alexander Lobakin wrote: >> From: Yunsheng Lin <linyunsheng@xxxxxxxxxx> >> Date: Fri, 3 Mar 2023 20:44:24 +0800 >> >>> On 2023/3/3 19:22, Alexander Lobakin wrote: >>>> From: Yunsheng Lin <linyunsheng@xxxxxxxxxx> >>>> Date: Thu, 2 Mar 2023 10:30:13 +0800 >> >> [...] >> >>>> And they are fixed :D >>>> No drivers currently which use Page Pool mix PP pages with non-PP. And >>> >>> The wireless adapter which use Page Pool *does* mix PP pages with >>> non-PP, see below discussion: >>> >>> https://lore.kernel.org/netdev/156f3e120bd0757133cb6bc11b76889637b5e0a6.camel@xxxxxxxxx/ >> >> Ah right, I remember that (also was fixed). >> Not that I think it is correct to mix them -- for my PoV, a driver >> shoule either give *all* its Rx buffers as PP-backed or not use PP at all. >> >> [...] >> >>>> As Jesper already pointed out, not having a quick way to check whether >>>> we have to check ::pp_magic at all can decrease performance. So it's >>>> rather a shortcut. >>> >>> When we are freeing a page by updating the _refcount, I think >>> we are already touching the cache of ::pp_magic. >> >> But no page freeing happens before checking for skb->pp_recycle, neither >> in skb_pp_recycle() (skb_free_head() etc.)[0] nor in skb_frag_unref()[1]. > > If we move to per page marker, we probably do not need checking > skb->pp_recycle. > > Note both page_pool_return_skb_page() and skb_free_frag() can > reuse the cache line triggered by per page marker checking if > the per page marker is in the 'struct page'. Ah, from that perspective. Yes, you're probably right, but would need to be tested anyway. I don't see any open problems with the PP recycling right now on the lists, but someone may try to change it one day. Anyway, this flag is only to do a quick test. We do have sk_buff::pfmemalloc, but this flag doesn't mean every page from this skb was pfmemalloced. > >> >>> >>> Anyway, I am not sure checking ::pp_magic is correct when a >>> page will be passing between different subsystem and back to >>> the network stack eventually, checking ::pp_magic may not be >>> correct if this happens. >>> >>> Another way is to use the bottom two bits in bv_page, see: >>> https://www.spinics.net/lists/netdev/msg874099.html >>> >>>> >>>>> >>>>>> >>>>>> /* Allow SKB to reuse area used by xdp_frame */ >>>>>> xdp_scrub_frame(xdpf); >>>>>> >>>> >>>> Thanks, >>>> Olek >>>> . >>>> >> >> [0] https://elixir.bootlin.com/linux/latest/source/net/core/skbuff.c#L808 >> [1] >> https://elixir.bootlin.com/linux/latest/source/include/linux/skbuff.h#L3385 >> >> Thanks, >> Olek >> . >> Thanks, Olek