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 AFB55C77B7C for ; Mon, 23 Jun 2025 17:29:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3BE736B00B2; Mon, 23 Jun 2025 13:29:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3968D6B00BB; Mon, 23 Jun 2025 13:29:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2D3116B00C6; Mon, 23 Jun 2025 13:29:14 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 1E0626B00B2 for ; Mon, 23 Jun 2025 13:29:14 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id CDBEE1D827D for ; Mon, 23 Jun 2025 17:29:13 +0000 (UTC) X-FDA: 83587351386.22.5BD3F5D Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by imf19.hostedemail.com (Postfix) with ESMTP id DF2521A000B for ; Mon, 23 Jun 2025 17:29:11 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=PuXx7wB7; spf=pass (imf19.hostedemail.com: domain of almasrymina@google.com designates 209.85.214.177 as permitted sender) smtp.mailfrom=almasrymina@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1750699752; 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:dkim-signature; bh=ih7l7aGR3GQBtt1mBj++NsVHfE7pPQqbo93fSE/REN0=; b=VKim5W7MlnhO5wf6kvptKx9xr82B8kuHc1y2y4Oq9GVVegYaFPWIlXLiEAEce1dwONPOWI vh/ccOfO1l2w/0tDFdY8a9ZNTyBr9PGQF5O58LVgGLWtHGp2I6c3A8FoyR7XsxBVBytJJq QTVwNRNPe0ig/jzQAVfoL9b4VDUR4lk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750699752; a=rsa-sha256; cv=none; b=LbVuthFQ3BgRwxGjdITOm0b9QD70/6o/5Cr1a1wMqg7blpGxczuycz5NSLIWB2cQB9arXy wE6ARUmuGHm9KPYhd/YRt490a3ByxtL8IwW4jDahcuN6KgkA8h4LprJhbQQt56QUmoaxPQ dxYe7JWU4uwE7r8Fx8R/jTp+ISp5vbE= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=PuXx7wB7; spf=pass (imf19.hostedemail.com: domain of almasrymina@google.com designates 209.85.214.177 as permitted sender) smtp.mailfrom=almasrymina@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-237f18108d2so13655ad.0 for ; Mon, 23 Jun 2025 10:29:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1750699751; x=1751304551; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ih7l7aGR3GQBtt1mBj++NsVHfE7pPQqbo93fSE/REN0=; b=PuXx7wB7uX1EUpe0HQUW+ATiXrBEohuEFgsG+lQSWx6Ak4TERx223QE/B1D8PrNK8u 4YmDiNAihqsKb2NqK6bNE26+6UwiCIi4bX7Az+EihIao3TvsfIG5ck0GbkEdRCWap1M8 53k1p1Me0iP8dZf75bs9qLeCwUpcmnXPSOuqTFFf3rFhWXAmwKz5d+D6BQ4nIoy+DNho GCJCSEDM9MYpsy4R1aegXXz58vSetB6R4VaaxKcpvVmdoyn7ihn6rvlwr3c82MJRuBvx pZ1ExU6aQBksbQ4Nbcu8niJX7qk26kREMuAxDgAj0HKst0STOiDlgRVnvFAY0mQ4Pt3w NacA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750699751; x=1751304551; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ih7l7aGR3GQBtt1mBj++NsVHfE7pPQqbo93fSE/REN0=; b=XhnWCijFhYIkUSxWO0GlOFwdV3YPACe6IgD2wz4WUUQaM9T+tlAV9uO1K59y5HX9/J hhW1p/hGmYfelobNzURi8e03ZHsKfZrB1f7Tubd70MmfK/uP7wG0Y6Fu/9ixBiMpFcf+ SUzPovtPbbUOvSc/yCdEL5kTRBvdgkiLcA8kxfrSR7+on91aIx/jJc/ci6C1eifyQKoj OQ6+PD0S5JLqs6SdxW5d+ePHFfMHPoGlmEXdm1ExR2v1xH7FO2oWEJMgrcBLls7D+UVt 50UzqsBhbNIc4It1gG9pBM+hIpnJvB8i3a8Uvau2VrlTL2OJJrg29hUj1Ayzs8yvAUcb 6v2Q== X-Forwarded-Encrypted: i=1; AJvYcCVMG0wPc9crPOdzZySdEcUoviCM315lKNrWZTLlctpVQfrznU4kTVthRtDxa5IhaNlaQ2C9wcr96g==@kvack.org X-Gm-Message-State: AOJu0YxqDhlZIRE+9XKTmFfxdXPXKpjexG8MK2E2tmSBMJGqDN+HXOOq BELx6R4zXk0mZ1RGyKEBymurO3eaOZ6rkbroTOsYVhZdAaA5bPxjf2zcjjyasAcCtzyq2GrfW4o +8JxmshEqsv5TXdz/9uS8BZyxxT8OmNs7HpdLB8Fn X-Gm-Gg: ASbGncutHyqetj6VAIkQ7UfYTKqyVLVzJgn47hQnMwT/zqIrkgDNP3hrEGU/SwukDWS s14SPEDviQ7JMBqzNn0wtC6HDHtCax90mz7hQD+GTyeefM+EQDODMet10w4hYKm4Enobzu9EvQd WArnKiEj46pgrqgu2VF3tcMoqU15dYXJkHB7S278GEX6tlaWZFfdINOl9VHodZ7QJUxaV98Byd X-Google-Smtp-Source: AGHT+IHIz3m807Q22lF6oC6dBirj1kMW3oRTCSHc3amqhbrzf7C+H/Ag+dMxGhkxJLFATRZHjXv5ZTgwnpv8fDxM7rQ= X-Received: by 2002:a17:903:1249:b0:234:b2bf:e67f with SMTP id d9443c01a7336-237e574becdmr4816395ad.9.1750699750291; Mon, 23 Jun 2025 10:29:10 -0700 (PDT) MIME-Version: 1.0 References: <20250620041224.46646-1-byungchul@sk.com> <20250620041224.46646-10-byungchul@sk.com> <20250623101622.GB3199@system.software.com> <460ACE40-9E99-42B8-90F0-2B18D2D8C72C@nvidia.com> <41e68e52-5747-4b18-810d-4b20ada01c9a@gmail.com> In-Reply-To: <41e68e52-5747-4b18-810d-4b20ada01c9a@gmail.com> From: Mina Almasry Date: Mon, 23 Jun 2025 10:28:57 -0700 X-Gm-Features: AX0GCFtw51Pg4R-sTvdTCfrVPdo1Tdq4h4FqSXVGmxppvQKlqjgu8IQOHqwvWOA Message-ID: Subject: Re: [PATCH net-next v6 9/9] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp() To: Pavel Begunkov Cc: David Hildenbrand , Zi Yan , Byungchul Park , 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, toke@redhat.com, tariqt@nvidia.com, edumazet@google.com, pabeni@redhat.com, saeedm@nvidia.com, leon@kernel.org, ast@kernel.org, daniel@iogearbox.net, 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, hannes@cmpxchg.org, jackmanb@google.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: DF2521A000B X-Stat-Signature: enymxyyfx7ffe9mr1crpaghoe13p8hb4 X-Rspam-User: X-HE-Tag: 1750699751-669178 X-HE-Meta: U2FsdGVkX19EDq4qtnwzTjdwpKfwWl3jxgQ9olq0/nRfDrdLmR7ZmXRTqqInIYqKOkUK6klQUj+PCA7advvu8e4tNImwR/FjWItFZBDNf5qO/TPgBL6TTpaZEdubLSe9bKY60urKp38/J+O7YSLNM0w//vKjpGv90GgtdPHspscr3IVTrrTq3ld624e1gG08Tun8YAVxRTOTpnMEQJgbtJ4iE1sRZlSouZvoJ7aSnKqWyHZEGuRyuSCEwBmiINYkYbMw5SoaH3Mfkt6xd/FRgy/O0Jy2tADmJG2ky9ICE9Ei2+3wO0Nw/jAgat7wV4fOlGy03XJBWEm0Qe6x82uc5HQURrtdr6hFEFvu9eFBjTfOndQRtW9YnSU78NPqUzAxSc3YuJcO4mqLkFxy9FFM8xZCBzYW9tlAGnzMdZS9FSwPPXySeeKMNLUD157DWYl01rTd1bsThGmTdtNtqL0dHo8Zgw8irzpe+Cq5UnxwWty1enGe8WQMBgUbE1R49wqiM6sksB5XKPluShKlziBtoRYLtwT4p4+HyU5KIAB+hIqQ/Y7lwqzfrd0AtZcNPsdVp7dbmOYcjQPxEnbyeJKNrmPJuXkloj6nPOHHwQGkbQ5EzbnLOCJBmAup+NhOvzEzR+CqMGdTa2BTqPCqzjGmQPD+D5bOpf7HomqPNZe7R+12GmOIj5IWwGU2JzPO0FnRCPhLUwFSFUK0hrAqH7KZVyV7FTrQy4+GWdnsFoccMfmgheqaeXYaz3Ia0ZkDNuFu8iB7iqw4Vwg940HVkuZXFh8qc/0LP1TKNmINrq45tzS/p5fWlX+1Epfz1lFzaufYVlSu1/sxZjcIhmHmLcacOxhApdPMSQsnk7HFiNN137mn1sihaFU+nSPidw9pTyrT7F5Na8PC0xrQVRY5WHj8Wva2z9wmisZ/iYa5r+Sz9JKttCBuj61UTH2h+PTmFlGdTndk0cc4S/N1KLoeRj0 5Xl2Posv YxwAcQnK1PH9hjY04BO8v3wzHtTxm91banyQ0NgwBPvM2N0D0RUZtUK4lCvRAW4vhYYaKsF4sMDKvpbQLCDN8Z1GtyAKUMiSINKCNDhz4yHeFfJZh/3Mw2RgXB+sdyqJKBxq7Ab6jMFAeH3dXSz/F+6MOusb8/zYgkEJ1+jzdhJ6cC783aMBuEZdP36nAHPbDKNSsLduW4W16JgTLUrBfXQuakcTqP+yI60T8Iez4zsZHfOFM2bqTEeDRPsNXUvLxRUIWPjh5ksuxULM9ir7S5MAmGF3D5yGhUFvsI1chs+lxGts25lxaeLX2DU5YTSsp+JXFd0re6Ai6b1V+big8RCCEOoQvCKLjgd6MZRssFkhmaxgY3eeGNP1ZqyvgGhGRXbI1Mh2E3uUSA1LBlEpFd467Gvs/gkluBUSAdEKaGyfVUaq65pyEs+n7xAUk5np6uFhZ5dwiz/jrZNqQ8SZ2G6mgluSvSMnTjquE 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 Mon, Jun 23, 2025 at 10:05=E2=80=AFAM Pavel Begunkov wrote: > > On 6/23/25 15:58, David Hildenbrand wrote: > > On 23.06.25 13:13, Zi Yan wrote: > >> On 23 Jun 2025, at 6:16, Byungchul Park wrote: > >> > >>> On Mon, Jun 23, 2025 at 11:16:43AM +0200, David Hildenbrand wrote: > >>>> On 20.06.25 06:12, Byungchul Park wrote: > >>>>> To simplify struct page, the effort to separate its own descriptor = from > >>>>> struct page is required and the work for page pool is on going. > >>>>> > >>>>> To achieve that, all the code should avoid directly accessing page = pool > >>>>> members of struct page. > >>>>> > >>>>> Access ->pp_magic through struct netmem_desc instead of directly > >>>>> accessing it through struct page in page_pool_page_is_pp(). Plus, = move > >>>>> page_pool_page_is_pp() from mm.h to netmem.h to use struct netmem_d= esc > >>>>> without header dependency issue. > >>>>> > >>>>> Signed-off-by: Byungchul Park > >>>>> Reviewed-by: Toke H=C3=B8iland-J=C3=B8rgensen > >>>>> Reviewed-by: Mina Almasry > >>>>> Reviewed-by: Pavel Begunkov > >>>>> Reviewed-by: Vlastimil Babka > >>>>> Acked-by: Harry Yoo > >>>>> --- > >>>>> include/linux/mm.h | 12 ------------ > >>>>> include/net/netmem.h | 14 ++++++++++++++ > >>>>> mm/page_alloc.c | 1 + > >>>>> 3 files changed, 15 insertions(+), 12 deletions(-) > >>>>> > >>>>> diff --git a/include/linux/mm.h b/include/linux/mm.h > >>>>> index 0ef2ba0c667a..0b7f7f998085 100644 > >>>>> --- a/include/linux/mm.h > >>>>> +++ b/include/linux/mm.h > >>>>> @@ -4172,16 +4172,4 @@ int arch_lock_shadow_stack_status(struct tas= k_struct *t, unsigned long status); > >>>>> */ > >>>>> #define PP_MAGIC_MASK ~(PP_DMA_INDEX_MASK | 0x3UL) > >>>>> > >>>>> -#ifdef CONFIG_PAGE_POOL > >>>>> -static inline bool page_pool_page_is_pp(struct page *page) > >>>>> -{ > >>>>> - return (page->pp_magic & PP_MAGIC_MASK) =3D=3D PP_SIGNATURE; > >>>>> -} > >>>>> -#else > >>>>> -static inline bool page_pool_page_is_pp(struct page *page) > >>>>> -{ > >>>>> - return false; > >>>>> -} > >>>>> -#endif > >>>>> - > >>>>> #endif /* _LINUX_MM_H */ > >>>>> diff --git a/include/net/netmem.h b/include/net/netmem.h > >>>>> index d49ed49d250b..3d1b1dfc9ba5 100644 > >>>>> --- a/include/net/netmem.h > >>>>> +++ b/include/net/netmem.h > >>>>> @@ -56,6 +56,20 @@ NETMEM_DESC_ASSERT_OFFSET(pp_ref_count, pp_ref_c= ount); > >>>>> */ > >>>>> static_assert(sizeof(struct netmem_desc) <=3D offsetof(struct pa= ge, _refcount)); > >>>>> > >>>>> +#ifdef CONFIG_PAGE_POOL > >>>>> +static inline bool page_pool_page_is_pp(struct page *page) > >>>>> +{ > >>>>> + struct netmem_desc *desc =3D (struct netmem_desc *)page; > >>>>> + > >>>>> + return (desc->pp_magic & PP_MAGIC_MASK) =3D=3D PP_SIGNATURE; > >>>>> +} > >>>>> +#else > >>>>> +static inline bool page_pool_page_is_pp(struct page *page) > >>>>> +{ > >>>>> + return false; > >>>>> +} > >>>>> +#endif > >>>> > >>>> I wonder how helpful this cleanup is long-term. > >>>> > >>>> page_pool_page_is_pp() is only called from mm/page_alloc.c, right? > >>> > >>> Yes. > >>> > >>>> There, we want to make sure that no pagepool page is ever returned t= o > >>>> the buddy. > >>>> > >>>> How reasonable is this sanity check to have long-term? Wouldn't we b= e > >>>> able to check that on some higher-level freeing path? > >>>> > >>>> The reason I am commenting is that once we decouple "struct page" fr= om > >>>> "struct netmem_desc", we'd have to lookup here the corresponding "st= ruct > >>>> netmem_desc". > >>>> > >>>> ... but at that point here (when we free the actual pages), the "str= uct > >>>> netmem_desc" would likely already have been freed separately (rememb= er: > >>>> it will be dynamically allocated). > >>>> > >>>> With that in mind: > >>>> > >>>> 1) Is there a higher level "struct netmem_desc" freeing path where w= e > >>>> could check that instead, so we don't have to cast from pages to > >>>> netmem_desc at all. > > As you said, it's just a sanity check, all page pool pages should > be freed by the networking code. It checks the ownership with > netmem_is_pp(), which is basically the same as page_pool_page_is_pp() > but done though some aliasing. > > static inline bool netmem_is_pp(netmem_ref netmem) > { > return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) =3D=3D PP_SI= GNATURE; > } > > I assume there is no point in moving the check to skbuff.c as it > already does exactly same test, but we can probably just kill it. > Even if we do kill it, maybe lets do that in a separate patch, and maybe a separate series. I would recommend not complicating this one? Also, AFAIU, this is about removing/moving the checks in bad_page_reason() and page_expected_state()? I think this check does fire sometimes. I saw at least 1 report in the last year of a bad_page_reason() check firing because the page_pool got its accounting wrong and released a page to the buddy allocator early, so maybe that new patch that removes that check should explain why this check is no longer necessary. --=20 Thanks, Mina