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: <439590d4-0f05-4f5e-80ec-e7fdf214e307@gmail.com>
Date: Mon, 17 Jun 2024 15:16:56 +0100
From: Pavel Begunkov <asml.silence@...il.com>
To: Mina Almasry <almasrymina@...gle.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-alpha@...r.kernel.org, linux-mips@...r.kernel.org,
 linux-parisc@...r.kernel.org, sparclinux@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 linux-arch@...r.kernel.org, bpf@...r.kernel.org,
 linux-kselftest@...r.kernel.org, linux-media@...r.kernel.org,
 dri-devel@...ts.freedesktop.org
Cc: "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Donald Hunter <donald.hunter@...il.com>,
 Jonathan Corbet <corbet@....net>,
 Richard Henderson <richard.henderson@...aro.org>,
 Ivan Kokshaysky <ink@...assic.park.msu.ru>, Matt Turner
 <mattst88@...il.com>, Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
 "James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
 Helge Deller <deller@....de>, Andreas Larsson <andreas@...sler.com>,
 Sergey Shtylyov <s.shtylyov@....ru>, Jesper Dangaard Brouer
 <hawk@...nel.org>, Ilias Apalodimas <ilias.apalodimas@...aro.org>,
 Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Arnd Bergmann <arnd@...db.de>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
 Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman
 <eddyz87@...il.com>, Song Liu <song@...nel.org>,
 Yonghong Song <yonghong.song@...ux.dev>,
 John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
 Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Steffen Klassert
 <steffen.klassert@...unet.com>, Herbert Xu <herbert@...dor.apana.org.au>,
 David Ahern <dsahern@...nel.org>,
 Willem de Bruijn <willemdebruijn.kernel@...il.com>,
 Shuah Khan <shuah@...nel.org>, Sumit Semwal <sumit.semwal@...aro.org>,
 Christian König <christian.koenig@....com>,
 Bagas Sanjaya <bagasdotme@...il.com>, Christoph Hellwig <hch@...radead.org>,
 Nikolay Aleksandrov <razor@...ckwall.org>, David Wei <dw@...idwei.uk>,
 Jason Gunthorpe <jgg@...pe.ca>, Yunsheng Lin <linyunsheng@...wei.com>,
 Shailend Chand <shailend@...gle.com>,
 Harshitha Ramamurthy <hramamurthy@...gle.com>,
 Shakeel Butt <shakeel.butt@...ux.dev>, Jeroen de Borst
 <jeroendb@...gle.com>, Praveen Kaligineedi <pkaligineedi@...gle.com>,
 linux-mm@...ck.org, Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH net-next v12 06/13] page_pool: devmem support

On 6/13/24 02:35, Mina Almasry wrote:
> Convert netmem to be a union of struct page and struct netmem. Overload
> the LSB of struct netmem* to indicate that it's a net_iov, otherwise
> it's a page.
> 
> Currently these entries in struct page are rented by the page_pool and
> used exclusively by the net stack:
> 
> struct {
> 	unsigned long pp_magic;
> 	struct page_pool *pp;
> 	unsigned long _pp_mapping_pad;
> 	unsigned long dma_addr;
> 	atomic_long_t pp_ref_count;
> };
> 
> Mirror these (and only these) entries into struct net_iov and implement
> netmem helpers that can access these common fields regardless of
> whether the underlying type is page or net_iov.
> 
> Implement checks for net_iov in netmem helpers which delegate to mm
> APIs, to ensure net_iov are never passed to the mm stack.
> 
> Signed-off-by: Mina Almasry <almasrymina@...gle.com>

Apart from small comments below

Reviewed-by: Pavel Begunkov <asml.silence@...il.com>


> ---
>   include/net/netmem.h            | 137 ++++++++++++++++++++++++++++++--
>   include/net/page_pool/helpers.h |  25 +++---
>   net/core/devmem.c               |   3 +
>   net/core/page_pool.c            |  26 +++---
>   net/core/skbuff.c               |  22 +++--
>   5 files changed, 168 insertions(+), 45 deletions(-)
> 
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 664df8325ece5..35ad237fdf29e 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
...
> -/* Converting from page to netmem is always safe, because a page can always be
> - * a netmem.
> - */
>   static inline netmem_ref page_to_netmem(struct page *page)
>   {
>   	return (__force netmem_ref)page;
> @@ -68,17 +107,103 @@ static inline netmem_ref page_to_netmem(struct page *page)
>   
>   static inline int netmem_ref_count(netmem_ref netmem)
>   {
> +	/* The non-pp refcount of net_iov is always 1. On net_iov, we only
> +	 * support pp refcounting which uses the pp_ref_count field.
> +	 */
> +	if (netmem_is_net_iov(netmem))
> +		return 1;
> +
>   	return page_ref_count(netmem_to_page(netmem));
>   }
>   
>   static inline unsigned long netmem_to_pfn(netmem_ref netmem)
>   {
> +	if (netmem_is_net_iov(netmem))
> +		return 0;

IIRC 0 is a valid pfn. Not much of a concern since it's
used only for tracing, but might make sense to pass some
invalid pfn if there is one

> +
>   	return page_to_pfn(netmem_to_page(netmem));
>   }
>   
...
>   static inline netmem_ref netmem_compound_head(netmem_ref netmem)
>   {
> +	/* niov are never compounded */
> +	if (netmem_is_net_iov(netmem))
> +		return netmem;
> +
>   	return page_to_netmem(compound_head(netmem_to_page(netmem)));
>   }
>   
> +static inline void *netmem_address(netmem_ref netmem)

I don't think it's used anywhere, do I miss it?

> +{
> +	if (netmem_is_net_iov(netmem))
> +		return NULL;
> +
> +	return page_address(netmem_to_page(netmem));
> +}
> +
...
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index a5957d3359762..1152e3547795a 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -26,6 +26,8 @@
...
>   
>   /* If the page refcnt == 1, this will try to recycle the page.
> @@ -714,7 +713,7 @@ __page_pool_put_page(struct page_pool *pool, netmem_ref netmem,
>   	 * refcnt == 1 means page_pool owns page, and can recycle it.
>   	 *
>   	 * page is NOT reusable when allocated when system is under
> -	 * some pressure. (page_is_pfmemalloc)
> +	 * some pressure. (page_pool_page_is_pfmemalloc)

There is no page_pool_page_is_pfmemalloc()

>   	 */
>   	if (likely(__page_pool_page_can_be_recycled(netmem))) {
>   		/* Read barrier done in page_ref_count / READ_ONCE */
> @@ -727,6 +726,7 @@ __page_pool_put_page(struct page_pool *pool, netmem_ref netmem,
>   		/* Page found as candidate for recycling */
>   		return netmem;
>   	}

-- 
Pavel Begunkov

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ