From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67D32C54F30 for ; Mon, 26 May 2025 01:37:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DCF2B6B007B; Sun, 25 May 2025 21:37:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D804E6B0082; Sun, 25 May 2025 21:37:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C6FE96B0083; Sun, 25 May 2025 21:37:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A7D916B007B for ; Sun, 25 May 2025 21:37:55 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 8F5E016189D for ; Mon, 26 May 2025 01:37:53 +0000 (UTC) X-FDA: 83483347626.09.82C60EA Received: from invmail4.hynix.com (exvmail4.skhynix.com [166.125.252.92]) by imf22.hostedemail.com (Postfix) with ESMTP id 1C219C0004 for ; Mon, 26 May 2025 01:37:50 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of byungchul@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=byungchul@sk.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1748223471; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bV5YEHUGhfseoI3ksLDo3GuSIdPB9HBzzHs3Em39u3o=; b=Jf30WAa9MNParwBQbl3tV3MFx9Fx8I3acgOIUCE/zgid4isJJZBUxT3UNo9nj3MN/nZpPx KhOtNiivgfkePODI5sKOYL42dl6T6CeyW3YE7tanCDaA0YVifBCJD0alB0p7J1xLfViD9t 63oWeAqMtF5GgGTfsIVw7AaoUsnQkbY= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of byungchul@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=byungchul@sk.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1748223471; a=rsa-sha256; cv=none; b=7aMlNWNiJnDod4wZkOFBO4+Pc+eey0tJT5xB+f/mniCW41kCOpbhcXUEM6ArJziJBDJQt6 20+iz/vlblNehp4VUlIV31vDhs1JcvPkU8ZoUffiSP+i/cCsqYd2uqPkVBwFMvletLVfAe P+q/F46fNjDm4Qsj2aEO4gHEa7uhvzg= X-AuditID: a67dfc5b-681ff7000002311f-23-6833c5ed816f Date: Mon, 26 May 2025 10:37:44 +0900 From: Byungchul Park To: Mina Almasry Cc: willy@infradead.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, kernel_team@skhynix.com, kuba@kernel.org, ilias.apalodimas@linaro.org, harry.yoo@oracle.com, hawk@kernel.org, akpm@linux-foundation.org, davem@davemloft.net, john.fastabend@gmail.com, andrew+netdev@lunn.ch, asml.silence@gmail.com, toke@redhat.com, tariqt@nvidia.com, edumazet@google.com, pabeni@redhat.com, saeedm@nvidia.com, leon@kernel.org, ast@kernel.org, daniel@iogearbox.net, david@redhat.com, lorenzo.stoakes@oracle.com, Liam.Howlett@oracle.com, vbabka@suse.cz, rppt@kernel.org, surenb@google.com, mhocko@suse.com, horms@kernel.org, linux-rdma@vger.kernel.org, bpf@vger.kernel.org, vishal.moola@gmail.com Subject: Re: [PATCH 18/18] mm, netmem: remove the page pool members in struct page Message-ID: <20250526013744.GD74632@system.software.com> References: <20250523032609.16334-1-byungchul@sk.com> <20250523032609.16334-19-byungchul@sk.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-Brightmail-Tracker: H4sIAAAAAAAAA02Sf0yMcRzHfZ/nueeebh1P59e3H0pniSKR2WdGzvzzbJYx/KFsdatn7rjS 7pI7P7aoaaJUGJ3DCf2S3XZd3YWFin6wnFCHKFHZUn7EVVdDVzP999r783m/358/PgwpKRT4 MMqkFF6dJFdJaRElGvAsXD7wKEIR7ipYDAZTOQ23RrRQ3GUTgKGsCsHP0bdCGKpvoOH6NScJ hmcZFPwyuUjoedwthM6iXgruZVpJ6D7TSEN2xhgJx20lBNircgRwznWTBGtalxBe3DHQ8L78 jwB6a7MpaNKXUtCZI4PHxnngfPIFQb3JSoDz9GUazrYaafiY0Ymgta6bgkvHchCYahwCGBsx 0LJAzlL6muCq9e+EnNF8gKsoCeGyHK0kZy47SXPmH/lCrqPtHs01XhyjuGrbEMFlpw/S3Pee NxT3teYVzZksryjuqbFeyA2Z/bey0aJ1CbxKmcqrV0TGiRQt9yvo5PNh2pO5w3QaehSYhTwY zK7GZ4sHBP/45XCf0M0UG4TzmysJN9NsMHY4Rkk3z2GX4hs1eZP7JNspwC2GvW6eze7ALz8U THrFLOAO0zjKQiJGwpYi3G8pIKYGXrip4BM1ZQ7G41daJ0KZCfbFxb+ZKTkAp1dempQ92G3Y WSRzy3PZRfhBVQPhjsSshcF1w6+FUzd744clDioXeemnNeinNej/N+inNRgRVYYkyqTURLlS tTpMoUtSasPi9yea0cTnFB0dj7GhH/bttYhlkNRTHCeNUEgE8lSNLrEWYYaUzhH7GcIVEnGC XHeIV++PVR9Q8Zpa5MtQ0vniVc6DCRJ2jzyF38fzybz635RgPHzSkF/jpjL7Olu095qG5vFl 5apdzxcFv/gg8/P3fpNZtzO0r/Jwig8R/ylAezvUcqTrgmu3b2BMs89djf3USjrg15Z5X+Py Ykc8549uXj+s8/vGW6PTP0dtgMwZC6L2zUo+ro25ao9vnxkpO7Z9xpITg+0bq2v2LizVtQUG Wdfq+1lXvb+U0ijkK0NItUb+F/ggX2Q1AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA02SfUzMcRzHfX9P9+vm7Nc5fCePZ57a5JmPMfIHvvOHsRmWTd3qN3d0lTt3 ykQpw03JYXRddp5Ksd12UsduV65nTCnZUSpFWKXoWQ2dZvrvtfd779dfb56Wn2Gn85qoo6Iu ShWp5KSMdMf6pCWdpSvVy9yXFWC1P+Dg/mAsZDc7WbDm5iPoHaqXQE9JOQe3b/bTYK1KZqDP /pOGT2UtEmjKamPAdbaAhpaLFRykJA/TcNp5j4LizEoWqvNTWbjy8y4NBQnNEqh9YuWg8cFv Fto8KQxUWnIYaEoNhjLbVOh/3oGgxF5AQf+FTA4u19g4aE1uQlBT3MJARmIqArvby8LwoJUL VpK8nLcUeWx5LyE2h4E8vBdITN4amjhyz3PE8cMsIQ1vXBypuD7MkMfOHoqkJH3jyPdP7xjS 5a7jyO0v3RSx59Ux5IWtRLLTP0S6IUKM1BhF3dKNYVL1y8KHXMzVoNjzaQNcAiqda0J+PBZW 4dcDnyU+ZoT52PzsEeVjTliIvd4h2scKYTG+477E+pgWmlj80nrIx5OF3fj1h/S/W5kAuME+ gkxIysuFHITb89KpscIfV6Z/ZMbGC/HIjZpRKT/KATj7Fz8Wz8ZJjzL+xn7CLtyfFeyLpwjz cFF+OZWGJlnGiSzjRJb/Iss4kQ0xuUihiTJqVZrI1UH6w+q4KE1sUHi01oFGz5EVP3LJiXpr t3mQwCPlRFmYcqVazqqM+jitB2GeVipkM6zL1HJZhCruuKiLDtUZIkW9BwXwjHKabPteMUwu HFQdFQ+LYoyo+9dSvN/0BGSaoE05HT0xbc9657p6177WmY2JinNV2Uf8R9aWmdacKmyfHR/t H1Kt7TwUvsi9et0xV9WJW7Hm6lmtDTO6Q9FW44Gia1uKSId1gUH61Gx437wgPt5R+LXPu2bt RVrcP6fNUyukmjedmR8aM+nVRjy4uWDTdUOXqruVORkiXdHYZ1IyerVqeSCt06v+AH/V+y0Y AwAA X-CFilter-Loop: Reflected X-Rspam-User: X-Stat-Signature: 3iz55yqnem7ms69b1hec5mjsmtwuyhxg X-Rspamd-Queue-Id: 1C219C0004 X-Rspamd-Server: rspam11 X-HE-Tag: 1748223470-968795 X-HE-Meta: U2FsdGVkX1+L5Ni5PvF7MWR8ykrVHuShexr+iqqa+6A3lF410nxtW4DnT858XX2C14cJ8T/Hf6RUAVkeWTOryyUwQfbXg5bFv9t6KDPW6qKwcwqXeRpoU3t7YUrobXRD8nB7DGCvUb1BTjM2vC268Oi0E4vO448Cb5w+H18VxgIrSsSW9B21PNw9Mn6l082BZ78kG1pq3OKeqV+9vMlDwnM8mouXoOA3mZrxyr9Rwb/M8oFl+Uh6zn17dLBLCTEbc5+a/OrL6YgCxc/uMrsr8wHKjiBkcxm2b0NkGNIgnenj5omPS4NiJL+k1Dzd+qD4m/SVxqH27IvJe8g2c02K9w0vjRNr9tLfi5IJcV/J7EO6YSbkve2hrk6EkemftJ/1Hgoy7hMO3rovn++frKyvyVJcj00Sa3iE2e6+hwBRw/mk1rMPu2FNp50mMnO1AjEtIFqXK/r/GJLqdVkBBQlm2Fc9RRXQooGQqv6Zsz48ufKV7lzgOgphIy/U6UVxdasNoEnq1hkZLdHBJbiaMXynW1bEYhM8e/YZlEDxp92Njd0VYhc9nknAt9mnFBVSLbPM/SjKeLsU6r2b0oLiG9Ncnc5bqgSEAyG69lG2SgZ+ORwmyip+9SwArj+ItPke0Mj0fM3RxZHpB+Oecqsr2342P834UsuuUxlMvuE8tmf1wm7lxwfOrjMfyWZS0VLMGGXQuFmNyYvUhcxIhxet7LRZcd13M93a6W+4DdmzGDI9fluGD+8Tj2RuyGCQevAGScd/0lAw3tJCB+K9yIIycVGEairoeIdlzzjA4UnocXIredt1faVm8GOz7QOxfB9FR9vTg3ikVdPfa2xN5SDtttIcD/T51zzP+hmCMcHHWQt80vpBkyqFi6yk7fWL0GvSZJo/9zHq/X17PN76xRzZe5SXVFHS7piBJe45ouftyvVbU8busa50s8CNX+JdDUUA+Vu18e1xS2sGJyChTgdfmlp uD8zfPSh DhFXHwZTrF7M78dhRrr46o7jAYJsA0n1cKiIcmw/zVCgLOZs8r2Lf1nwBLLD0QD1+H+dvZzi/e0kITTI5zPpkOsoxZ6vCsZ7vzCDpWk1RCcuQLdA= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, May 23, 2025 at 10:55:54AM -0700, Mina Almasry wrote: > On Thu, May 22, 2025 at 8:26 PM Byungchul Park wrote: > > > > Now that all the users of the page pool members in struct page have been > > gone, the members can be removed from struct page. > > > > However, since struct netmem_desc might still use the space in struct > > page, the size of struct netmem_desc should be checked, until struct > > netmem_desc has its own instance from slab, to avoid conficting with > > other members within struct page. > > > > Remove the page pool members in struct page and add a static checker for > > the size. > > > > Signed-off-by: Byungchul Park > > --- > > include/linux/mm_types.h | 11 ----------- > > include/net/netmem.h | 28 +++++----------------------- > > 2 files changed, 5 insertions(+), 34 deletions(-) > > > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > > index 873e820e1521..5a7864eb9d76 100644 > > --- a/include/linux/mm_types.h > > +++ b/include/linux/mm_types.h > > @@ -119,17 +119,6 @@ struct page { > > */ > > unsigned long private; > > }; > > - struct { /* page_pool used by netstack */ > > - unsigned long _pp_mapping_pad; > > - /** > > - * @pp_magic: magic value to avoid recycling non > > - * page_pool allocated pages. > > - */ > > - unsigned long pp_magic; > > - struct page_pool *pp; > > - unsigned long dma_addr; > > - atomic_long_t pp_ref_count; > > - }; > > struct { /* Tail pages of compound page */ > > unsigned long compound_head; /* Bit zero is set */ > > }; > > diff --git a/include/net/netmem.h b/include/net/netmem.h > > index c63a7e20f5f3..257c22398d7a 100644 > > --- a/include/net/netmem.h > > +++ b/include/net/netmem.h > > @@ -77,30 +77,12 @@ struct net_iov_area { > > unsigned long base_virtual; > > }; > > > > -/* These fields in struct page are used by the page_pool and net stack: > > - * > > - * struct { > > - * unsigned long _pp_mapping_pad; > > - * unsigned long pp_magic; > > - * struct page_pool *pp; > > - * unsigned long dma_addr; > > - * atomic_long_t pp_ref_count; > > - * }; > > - * > > - * We mirror the page_pool fields here so the page_pool can access these fields > > - * without worrying whether the underlying fields belong to a page or net_iov. > > - * > > - * The non-net stack fields of struct page are private to the mm stack and must > > - * never be mirrored to net_iov. > > +/* XXX: The page pool fields in struct page have been removed but they > > + * might still use the space in struct page. Thus, the size of struct > > + * netmem_desc should be under control until struct netmem_desc has its > > + * own instance from slab. > > */ > > -#define NET_IOV_ASSERT_OFFSET(pg, iov) \ > > - static_assert(offsetof(struct page, pg) == \ > > - offsetof(struct net_iov, iov)) > > -NET_IOV_ASSERT_OFFSET(pp_magic, pp_magic); > > -NET_IOV_ASSERT_OFFSET(pp, pp); > > -NET_IOV_ASSERT_OFFSET(dma_addr, dma_addr); > > -NET_IOV_ASSERT_OFFSET(pp_ref_count, pp_ref_count); > > -#undef NET_IOV_ASSERT_OFFSET > > +static_assert(sizeof(struct netmem_desc) <= offsetof(struct page, _refcount)); > > > > Removing these asserts is actually a bit dangerous. Functions like > netmem_or_pp_magic() rely on the fact that the offsets are the same > between struct page and struct net_iov to access these fields without Worth noting this patch removes the page pool fields from struct page. However, yes, I will keep necessary assertions with some changes applied so that it can work even after removing the page pool fields like: NET_IOV_ASSERT_OFFSET(lru, pp_magic); NET_IOV_ASSERT_OFFSET(mapping, _pp_mapping_pad); > worrying about the type of the netmem. What we do in these helpers is > we we clear the least significant bit of the netmem, and then access > the field. This works only because we verified at build time that the > offset is the same. > > I think we have 3 options here: > > 1. Keep the asserts as-is, then in the follow up patch where we remove > netmem_desc from struct page, we update the asserts to make sure > struct page and struct net_iov can grab the netmem_desc in a uniform Ah. It's worth noting that I'm removing the page pool fields all the way from strcut page, instead of placing a place-holder that I did in RFC as Matthew requested. > way. > > 2. We remove the asserts, but all the helpers that rely on > __netmem_clear_lsb need to be modified to do custom handling of > net_iov vs page. Something like: > > static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic) > { > if (netmem_is_net_iov(netmem) > netmem_to_net_iov(netmem)->pp_magic |= pp_magic; > else > netmem_to_page(netmem)->pp_magic |= pp_magic; struct page should not have pp_magic field once the page pool fields are gone. Byungchul > } > > Option #2 requires extra checks, which may affect the performance > reported by page_pool_bench_simple that I pointed you to before. > > 3. We could swap out all the individual asserts for one assert, if > both page and net_iov have a netmem_desc subfield. This will also need > to be reworked when netmem_desc is eventually moved out of struct page > and is slab allocated: > > NET_IOV_ASSERT_OFFSET(netmem_desc, netmem_desc); > > -- > Thanks, > Mina