netdev - Re: [RFC 04/12] net: page_pool: merge page_pool_release_page() with page_pool_return

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]

Date: Mon, 10 Jul 2023 18:07:02 +0200
From: Jesper Dangaard Brouer <jbrouer@...hat.com>
To: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Cc: brouer@...hat.com, almasrymina@...gle.com, hawk@...nel.org,
 ilias.apalodimas@...aro.org, edumazet@...gle.com, dsahern@...il.com,
 michael.chan@...adcom.com, willemb@...gle.com
Subject: Re: [RFC 04/12] net: page_pool: merge page_pool_release_page() with
 page_pool_return_page()



On 07/07/2023 20.39, Jakub Kicinski wrote:
> Now that page_pool_release_page() is not exported we can
> merge it with page_pool_return_page(). I believe that
> the "Do not replace this with page_pool_return_page()"
> comment was there in case page_pool_return_page() was
> not inlined, to avoid two function calls.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>

I forgot the exact reason, but the "avoid two function calls" argument
makes sense.  As this is no-longer an issues, I'm okay with this change.

Acked-by: Jesper Dangaard Brouer <hawk@...nel.org>

> ---
>   net/core/page_pool.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 2c7cf5f2bcb8..7ca456bfab71 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -492,7 +492,7 @@ static s32 page_pool_inflight(struct page_pool *pool)
>    * a regular page (that will eventually be returned to the normal
>    * page-allocator via put_page).
>    */
> -static void page_pool_release_page(struct page_pool *pool, struct page *page)
> +static void page_pool_return_page(struct page_pool *pool, struct page *page)
>   {
>   	dma_addr_t dma;
>   	int count;
> @@ -518,12 +518,6 @@ static void page_pool_release_page(struct page_pool *pool, struct page *page)
>   	 */
>   	count = atomic_inc_return_relaxed(&pool->pages_state_release_cnt);
>   	trace_page_pool_state_release(pool, page, count);
> -}
> -
> -/* Return a page to the page allocator, cleaning up our state */
> -static void page_pool_return_page(struct page_pool *pool, struct page *page)
> -{
> -	page_pool_release_page(pool, page);
>   
>   	put_page(page);
>   	/* An optimization would be to call __free_pages(page, pool->p.order)
> @@ -615,9 +609,7 @@ __page_pool_put_page(struct page_pool *pool, struct page *page,
>   	 * will be invoking put_page.
>   	 */
>   	recycle_stat_inc(pool, released_refcnt);
> -	/* Do not replace this with page_pool_return_page() */
> -	page_pool_release_page(pool, page);
> -	put_page(page);
> +	page_pool_return_page(pool, page);
>   
>   	return NULL;
>   }