netdev - Re: [PATCH net-next v4 4/4] skbuff: Optimization of SKB coalescing for page pool

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: <664e753b-23ba-56b7-a2b7-a2bd83260887@huawei.com>
Date: Wed, 29 Nov 2023 11:56:40 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Liang Chen <liangchen.linux@...il.com>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<hawk@...nel.org>, <ilias.apalodimas@...aro.org>
CC: <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 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.

Also, Is skb_pp_frag_ref() a better name than skb_pp_get_frag_ref()
mirroring skb_frag_ref()?

> +{
> +	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.


> +	else
> +		get_page(head_page);

I suppose we can use page_ref_inc() as we have a compound_head()
calling in the above.

Other than the above nits, LGTM.

Reviewed-by: Yunsheng Lin <linyunsheng@...wei.com>