On Sat, Nov 25, 2023 at 8:16 PM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote:
>
> On 2023/11/24 15:34, Liang Chen wrote:
>
> ...
>
> > --- a/include/net/page_pool/helpers.h
> > +++ b/include/net/page_pool/helpers.h
> > @@ -402,4 +402,26 @@ static inline void page_pool_nid_changed(struct page_pool *pool, int new_nid)
> > page_pool_update_nid(pool, new_nid);
> > }
> >
> > +static inline bool page_pool_is_pp_page(struct page *page)
> > +{
>
> We have a page->pp_magic checking in napi_pp_put_page() in skbuff.c already,
> it seems better to move it to skbuff.c or skbuff.h and use it for
> napi_pp_put_page() too, as we seem to have chosen to demux the page_pool
> owned page and non-page_pool owned page handling in the skbuff core.
>
> If we move it to skbuff.c or skbuff.h, we might need a better prefix than
> page_pool_* too.
>
How about keeping the 'page_pool_is_pp_page' function in 'helper.h'
and letting 'skbbuff.c' use it? It seems like the function's logic is
better suited to be internal to the page pool, and it might be needed
outside of 'skbuff.c' in the future.
