From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 44F49194C73; Wed, 10 Jul 2024 16:49:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720630143; cv=none; b=YD3lAAgDBKpuSJh4FPdaUHZeUP8bMn34H6FQmmmHWuKiCg3QYTC3qbcpRHHO2KV69AWXvoAp5yVoBJWCIv2l37pCwdMthDRUCsShZFEtKggyz7vjm4Kp3TyL3+yd5VidK1ry//W3DsPZ+fc8518dSKJ5JyLJDC1vUf2V5R3SXUU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720630143; c=relaxed/simple; bh=eS/RMGP4VxRIFbhmXWkCrFvxioDBvmLiKkBEAODdwGU=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=pCQ/7SSA2jPsUE9Nfax8t0rFtnFMzk6fWwhRqHFXxXe80js3uuqxMmIx7qjPIBXJvi1KybvFljiZsftFo/KWB5XYWPMWFnZwzrK6VEY0S709HDCs//wv87SQAKEbZtYt0L5QIam5uBo4tW70RAFEh1rPiNYyvrW7tJH9hIQSvb0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GeE9psM3; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="GeE9psM3" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 24C51C32781; Wed, 10 Jul 2024 16:49:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1720630142; bh=eS/RMGP4VxRIFbhmXWkCrFvxioDBvmLiKkBEAODdwGU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GeE9psM35dHGvAUo79TtOnu4kq5cY9EUrGF2ghylZhRExsJfxInrnrbzIGYTRdEyP JuNUksWFVoX99WDJK0OYBlsaXy6q6YA4gtH9ruZmomvqVAnZKrQ5V0cyaDEK1nrfmj TYzIOxglnzs0uEzQL7xFLg0IgYd2T3xSZ3m9tEEYvl2XCdOAvnosGa5j4fJc/Yy95m Fc5KKtoP2fmuVTazyDqaAh0RTuL+2gSo3Q0QXNcq8mXWHgnju0cDM5kOaxJGJVc4MU IX9XFPAHEsGhG08l+UfVEIDuPwq1AH3mfKgmvCy5HzZT6Og5qbJjr0Ohj8oZTpGXx8 z5HtdpFuugVEQ== Date: Wed, 10 Jul 2024 09:49:00 -0700 From: Jakub Kicinski To: Mina Almasry Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-alpha@vger.kernel.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, sparclinux@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-kselftest@vger.kernel.org, bpf@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, Donald Hunter , "David S. Miller" , Eric Dumazet , Paolo Abeni , Jonathan Corbet , Richard Henderson , Ivan Kokshaysky , Matt Turner , Thomas Bogendoerfer , "James E.J. Bottomley" , Helge Deller , Andreas Larsson , Jesper Dangaard Brouer , Ilias Apalodimas , Steven Rostedt , Masami Hiramatsu , Mathieu Desnoyers , Arnd Bergmann , Steffen Klassert , Herbert Xu , David Ahern , Willem de Bruijn , Shuah Khan , Sumit Semwal , "Christian =?UTF-8?B?S8O2bmln?=" , Bagas Sanjaya , Christoph Hellwig , Nikolay Aleksandrov , Taehee Yoo , Pavel Begunkov , David Wei , Jason Gunthorpe , Yunsheng Lin , Shailend Chand , Harshitha Ramamurthy , Shakeel Butt , Jeroen de Borst , Praveen Kaligineedi , linux-mm@kvack.org, Matthew Wilcox Subject: Re: [PATCH net-next v16 05/13] page_pool: devmem support Message-ID: <20240710094900.0f808684@kernel.org> In-Reply-To: <20240710001749.1388631-6-almasrymina@google.com> References: <20240710001749.1388631-1-almasrymina@google.com> <20240710001749.1388631-6-almasrymina@google.com> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 10 Jul 2024 00:17:38 +0000 Mina Almasry wrote: > @@ -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)); > } How can this work if we had to revert the patch which made all of the networking stack take pp-aware refs? Maybe we should add the refcount, and let it be bumped, but WARN() if the net_iov is released with refcount other than 1? Or we need a very solid explanation why the conversion had to be reverted and this is fine. > static inline unsigned long netmem_to_pfn(netmem_ref netmem) > { > + if (netmem_is_net_iov(netmem)) > + return 0; > + > return page_to_pfn(netmem_to_page(netmem)); > } Can we move this out and rename it to netmem_pfn_trace() ? Silently returning 0 is not generally okay, but since it's only for tracing we don't care. > +static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem) > +{ > + return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV); > +} > + > +static inline unsigned long netmem_get_pp_magic(netmem_ref netmem) > +{ > + return __netmem_clear_lsb(netmem)->pp_magic; > +} > + > +static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic) > +{ > + __netmem_clear_lsb(netmem)->pp_magic |= pp_magic; > +} > + > +static inline void netmem_clear_pp_magic(netmem_ref netmem) > +{ > + __netmem_clear_lsb(netmem)->pp_magic = 0; > +} > + > +static inline struct page_pool *netmem_get_pp(netmem_ref netmem) > +{ > + return __netmem_clear_lsb(netmem)->pp; > +} > + > +static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool) > +{ > + __netmem_clear_lsb(netmem)->pp = pool; > +} Why is all this stuff in the main header? It's really low level. Please put helpers which are only used by the core in a header under net/core/, like net/core/dev.h