[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAKhg4t+dAj=gOSrTw-y0dx_2HDBgx+qY_gv2rEXPRKUiJFMJLA@mail.gmail.com>
Date: Wed, 29 Nov 2023 15:08:35 +0800
From: Liang Chen <liangchen.linux@...il.com>
To: Yunsheng Lin <linyunsheng@...wei.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, hawk@...nel.org, ilias.apalodimas@...aro.org,
netdev@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH net-next v4 4/4] skbuff: Optimization of SKB coalescing
for page pool
On Wed, Nov 29, 2023 at 11:56 AM Yunsheng Lin <linyunsheng@...wei.com> wrote:
>
> On 2023/11/29 11:12, Liang Chen wrote:
> >
> > +/**
> > + * skb_pp_get_frag_ref() - Increase fragment reference count of a page
> > + * @page: page of the fragment on which to increase a reference
> > + *
> > + * Increase fragment reference count (pp_ref_count) on a page, but if it is
> > + * not a page pool page, fallback to increase a reference(_refcount) on a
> > + * normal page.
> > + */
> > +static inline void skb_pp_get_frag_ref(struct page *page)
>
> Simiar comment for 'inline ' too.
>
Sure.
> Also, Is skb_pp_frag_ref() a better name than skb_pp_get_frag_ref()
> mirroring skb_frag_ref()?
>
Sure. skb_pp_frag_ref sounds better:)
> > +{
> > + struct page *head_page = compound_head(page);
> > +
> > + if (likely(skb_frag_is_pp_page(head_page)))
> > + atomic_long_inc(&head_page->pp_ref_count);
>
> As pp_ref_count is supposed to be a internal field to page_pool,
> I am not sure if it matters that much to manipulate pp_ref_count
> directly in skbuff core.
>
> Maybe we can provide a helper for that if it really matter in the
> future.
>
>
Sure. will provide a helper in page_pool/helpers.h
> > + else
> > + get_page(head_page);
>
> I suppose we can use page_ref_inc() as we have a compound_head()
> calling in the above.
>
Sure. Thanks!
> Other than the above nits, LGTM.
>
> Reviewed-by: Yunsheng Lin <linyunsheng@...wei.com>
Powered by blists - more mailing lists