netdev - Re: [PATCH net-next] skbuff: Optimize SKB coalescing for page pool case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKhg4t+hoOiVWMbBiD7HCu_Z5pSdCsZrev2FMEKhbWvzgHCarw@mail.gmail.com>
Date: Mon, 3 Jul 2023 17:12:46 +0800
From: Liang Chen <liangchen.linux@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: ilias.apalodimas@...aro.org, hawk@...nel.org, davem@...emloft.net, 
	edumazet@...gle.com, pabeni@...hat.com, linyunsheng@...wei.com, 
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] skbuff: Optimize SKB coalescing for page pool case

On Sat, Jul 1, 2023 at 7:07 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Wed, 28 Jun 2023 20:11:50 +0800 Liang Chen wrote:
> > +static inline void page_pool_page_ref(struct page *page)
> > +{
> > +     struct page *head_page = compound_head(page);
> > +
> > +     if (page_pool_is_pp_page(head_page) &&
> > +                     page_pool_is_pp_page_frag(head_page))
> > +             atomic_long_inc(&head_page->pp_frag_count);
> > +     else
> > +             get_page(head_page);
>
> AFAICT this is not correct, you cannot take an extra *pp* reference
> on a pp page, unless it is a frag. If the @to skb is a pp skb (and it
> must be, if we get here) we will have two skbs with pp_recycle = 1.
> Which means if they both get freed at the same time they will both
> try to return / release the page.
>
> I haven't looked at Yunsheng's v5 yet, but that's why I was asking
> to rename the pp_frag_count to pp_ref_count. pp_frag_count is a true
> refcount (rather than skb->pp_recycle acting as a poor man's single
> shot refcount), so in case of frag'd pages / after Yunsheng's work
> we will be able to take new refs.
>
> Long story short please wait until Yunsheng's changes are finalized.

Sure, thanks for the information. I will wait until Yunsheng's changes
are finalized before proposing v2.

As for the "pp" reference, it has the test
page_pool_is_pp_page_frag(head_page) there. So for a non-frag pp page,
it will be a get_page call.

Thanks,
Liang