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 3C173C7115B for ; Mon, 23 Jun 2025 17:05:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CE1E86B009B; Mon, 23 Jun 2025 13:05:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C90486B009C; Mon, 23 Jun 2025 13:05:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B58866B00A0; Mon, 23 Jun 2025 13:05:19 -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 A41B86B009B for ; Mon, 23 Jun 2025 13:05:19 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 47D5C1A0A56 for ; Mon, 23 Jun 2025 17:05:19 +0000 (UTC) X-FDA: 83587291158.02.014189D Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by imf25.hostedemail.com (Postfix) with ESMTP id EF74BA0015 for ; Mon, 23 Jun 2025 17:05:16 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gqyowvMQ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of asml.silence@gmail.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=asml.silence@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750698317; a=rsa-sha256; cv=none; b=IgS4EMtt5a0+1BwM5H6+gKpHfynL/36eKCKFATlqCExde8HhRfSeFRrU0RPGdZt/C2zRq/ vRchOION+Kb4EXYMIWn1bAIyfWwLObbVqNo8G2fiJFhrf8iB9J0iS4VZlTJJ0mWCTL1YRJ jxj7oqr4tXsfTUsYKP8/2BlmwpRHXgQ= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gqyowvMQ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of asml.silence@gmail.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=asml.silence@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1750698317; 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=pi3M8vSwwyPlCy0ds+MLiPgrDsER6WDeUbD1bJQq8OE=; b=bfYNJVQVRGGZdjoFXl5oOW01+IW2WkOdqJi7nXp7nHjozzYvPqL+oRTVUyIw4NzbhI6YJH 8vesvU/scpabIh2g+5UhpGqDGqiBS+ZJ7X6hG0uMo2Ev7ekCKtWzdKHPPJmLaDHAQtp91t gmxU5kZ/15MOxJw0SY5ZRqmOwSt1rj4= Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-ade5b8aab41so923795766b.0 for ; Mon, 23 Jun 2025 10:05:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1750698315; x=1751303115; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=pi3M8vSwwyPlCy0ds+MLiPgrDsER6WDeUbD1bJQq8OE=; b=gqyowvMQ4cIS9ljbPyQanvP+oZWWCk3hc8Ilq8oS/2yeefaLDQ1R9BXHP2NQED5igD EWBEkCSi82zHxqt9qfpcUcu41lZuz79Ao3ygVusX/I/j2tRxYTpderXggNOS4fp+4dJ/ iycvw78wm9NDC+TJGqZRkTauZqNRola+OAd/D6rfdPLLnvz2/TMCt/Yc84So1SL0yT3S cu73HaoCS1feH3wTHRi5JDnHFP8wD6RWX3TAwl4NwMTrYsnGBgXW6Uhgz50lwBAHPMR5 yyEfnIabqQ3C8ZuRepeoefCcCWNxKzuIb5lUvkPEJC6sa0vgbVMxJdmLfOg/3EGtAMuq tq4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750698315; x=1751303115; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=pi3M8vSwwyPlCy0ds+MLiPgrDsER6WDeUbD1bJQq8OE=; b=BGyrZ3r/CuUNtzaujssdMnZvkxpd4M2VnLog7dkUCXwb+bV0iuSk/9EH2gYsgtsqC4 xYJsXPN7rVSAtsiyVthg/lSWvHNhnomRmAiGJ9MlWcV2cm4joCvFsp7eAhjf0EBCN6Gi XFtQsH6VWUO/QZLGrGJUMiuKVO0x4RMTFDJX4bjq5+RAk2f0rGiE46PuNxbfmFRXVnOg Qhfxu6D1W/p6tJUGP235PyUQjQjH07msq8tIHuK2uDvdAdM5Z7rE3y4dqcbA3pa1HrLx xYd0Rl5Pl33zoRYHQgBh9eGmL0mX5D1T92ZrkppvgPWE+mRelxv0wbb1hxwnGUVoE1Pp A1tA== X-Forwarded-Encrypted: i=1; AJvYcCX9DYPVvT7RNuE65oQipE1vpnbZyMHS2ic7v5ytAKDg1n4Ol1Owz3PUYHkdvjXSEA/ohUGeOwyVpQ==@kvack.org X-Gm-Message-State: AOJu0YzCwQIbTmjZ5yrIaAkjRUv78ij2JKU9efu6tbF1arFovCErJeBJ HHgJTHxlvo3295bIvjo5RXUlS+oOZ63VJK9o1Io6Taa+6q4a5+ZQRQOBA5qMJ7y8 X-Gm-Gg: ASbGncu6tvygGNzHheXHsgYGfAZpQhh42KnsvztNt8gRbUTR6Vt3SGmAaKiAAK/KXal v/1dl1h8j7YBYS2JOe8vOcZC69bogx4f77VfYkz74xEEqIJEIW417izLnZkXxj1gK+2QX/OGRso zrQv8hygU9gb+4LyH8bgE/J9k5UfECvmSUWMJwufAIeOQnX6KiYWt0H8CNjNrP/gJXsGYFO6f23 9mdZhEu+psqAW/8Jm2rcVsFzsROalQmar626+B6qZumWdNQWWZQqpzv7njm+tt+dFAzOfzMM3/S 0utFDWcon1JFhXvHIUCCrVIKZcHIn4HKjRlQ6JfTUU9AQaBNk5heW8olEq6hYZLDKY/mBgozJkk = X-Google-Smtp-Source: AGHT+IFkMr16MZMhGYOEMve0k+oWilXDkDPpSQstoYWVNutXXp/lz9Oc+tMJS730MOiqaVVD0hEmBQ== X-Received: by 2002:a17:907:3f18:b0:ade:4339:9367 with SMTP id a640c23a62f3a-ae057a222b4mr1293806966b.26.1750698314880; Mon, 23 Jun 2025 10:05:14 -0700 (PDT) Received: from ?IPV6:2620:10d:c096:325::2ef? ([2620:10d:c092:600::1:85c4]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ae054082d25sm728555966b.88.2025.06.23.10.05.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 Jun 2025 10:05:14 -0700 (PDT) Message-ID: <41e68e52-5747-4b18-810d-4b20ada01c9a@gmail.com> Date: Mon, 23 Jun 2025 18:06:37 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next v6 9/9] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp() To: David Hildenbrand , Zi Yan , Byungchul Park Cc: willy@infradead.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, kernel_team@skhynix.com, kuba@kernel.org, almasrymina@google.com, 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 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> Content-Language: en-US From: Pavel Begunkov In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Queue-Id: EF74BA0015 X-Rspamd-Server: rspam10 X-Stat-Signature: o1qif1oodd3a9xzix1goz1d659oqp9hd X-HE-Tag: 1750698316-374252 X-HE-Meta: U2FsdGVkX18InW7UILUmdprrgxiIDyYe03M7jNqlqrP6akd+FbkbFCmkvWmkeCi+ksh+cXzpd1BxTTVjoZ5jj/aoEbh2ttlR9EkY5kvZ5VXXH8zTSqz9bgmxcrg/MP4SXRZAh70p4NPUmRZZVPGhwq7Ktc5ui1Yt6RSPYjNb9949xdp6jRQcpZ39krMHRMmKaSrPsJfzCzN8GPxeDOj8Cvteb6oGFYjwJoqdM0Vh9+LcI52mob/PTjCA3PWmHUVerBBNNMRhU35DWEzTPlGluJWw0qOLuuy2MuIyLy+OhJjkz1HBTBPpMHt9O1jEyIFyriVDYNKc4xQq3JyRp5BftVoqGdLvDuuHf86jvXMWnORbvmN8vLTqV7l58FYgzUWNOwf1i4sVoS/9HJxwno13/ZqZ+Vvyk72WnG1LbEpCQJzSEtPihUhPyoPu1orGQ/qAwhLVyttYe3cr5PmaiSpryaAhE+yZq4OruLsSCGsgvuWicXBImFWVoBaUlYFUz8hcaAwkm+3l9J//iw0vHcxER9UXFiq6SZ7aQ0eaubTP5mw+w0TErXIpi7hYpLFR6kx8x3aNG0Xr6hZqrGv6pOlk+UUyyrjwUW9rQ/NakYPjsGAOp29OTJ4zzk+dsoTflTHQC9akKAWPd3Xze2RSOs2izET6KdxdkpQeFeFUeN7c8PTcKi7qjLaw2XLhClHkdBySVSgEg9oy2P+LO4ovMM4Qbwbim/X2Kvxr9LVYoqJestP8C/GuyDC63HrczxQO0FRnLHywOGFmz1fdbpZ62ogTMS7z7DeQ2/TlyYUgSrgDmtIDdOm/jzVBLRajg8O9LG8PhjF91sfeAokLt+vU8u0YLuwBJzwzmOr3P0OiCFSgm0n6n6auDgN3Ees7NMkDPufIn5v6sEZDvyFlUXE24UuS2kRx6G+W7BCdZX8HJY+s9DwYozgXQUECquG+LQvEBqq4sFo/6i5Hq1hXojcJDG6 HbCGseZk AGZefVDa44RrR+DXygteG3IrKvbxvaasZfwPGKDZbhFecq94ZcddJntV+PCr0FUAOlt6ZVg/75jiG/P/BAOo5Yti+TjKtKfwJ13CD5dr4IOyNA+vLmT4KqNOqbPOUOb7/NJzvmo0FyWrsJOrF+eul44/Bf0EkD4j6SuFWbSL+q66wrUBoOdS9vwHixrK4kSom6m8rYqTiIikEwsNU6cAU+2DEHaluJ68q3iTQGFO8WOQMzf0K0vlaHzp+BIF3kgfqezuHmbAlqEtrl8oG2F2VxtMmHSrLaSdwxslIfqETqHvdv9fsRYfpBMxfD9TYtiGogw6SHZ41+Xc1CKO2Zr7+j4inKRwyWFa5ckSHeubIvc39Qw1CZTlThpvF+e9qOmVbKJb2agUjtx9FyQY5rBWDcJ70ZGQ1K3Y04/eN4je/ZwVS7OxsFxcJTLfuyOzLiixgTq5Hfth0xy/vXmrIGxac1uUi/j/Ou7QrICxPZzhMtO6FGcoaWpBBAh/N0SielTM2msadJ9WefohHMTZs2jlSHoL6xUg/ozkO8t23DcV1xhRnaVE1f02WS+s59mQYC2M/ryNGm1GSJvMqKcjnnqJB7LmOtm5G0OxPrUR/ktoPC5vrvph2tzvVgERa/o6g3o1+wqk4ry7qL3CzTS/sJHHkO/fsK5zrT0c/RXSUqIHo4BpiyNAche0FDyIiiqxaOuuMBrs52yTDXmTCINnX/Y7k5Rx4zWLKYO3d4fin3lSzw4Q9IcV7q+xZ1lrp0g== 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 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_desc >>>>> without header dependency issue. >>>>> >>>>> Signed-off-by: Byungchul Park >>>>> Reviewed-by: Toke Høiland-Jørgensen >>>>> 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 task_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) == 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_count); >>>>>     */ >>>>>    static_assert(sizeof(struct netmem_desc) <= offsetof(struct page, _refcount)); >>>>> >>>>> +#ifdef CONFIG_PAGE_POOL >>>>> +static inline bool page_pool_page_is_pp(struct page *page) >>>>> +{ >>>>> +     struct netmem_desc *desc = (struct netmem_desc *)page; >>>>> + >>>>> +     return (desc->pp_magic & PP_MAGIC_MASK) == 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 to >>>> the buddy. >>>> >>>> How reasonable is this sanity check to have long-term? Wouldn't we be >>>> able to check that on some higher-level freeing path? >>>> >>>> The reason I am commenting is that once we decouple "struct page" from >>>> "struct netmem_desc", we'd have to lookup here the corresponding "struct >>>> netmem_desc". >>>> >>>> ... but at that point here (when we free the actual pages), the "struct >>>> netmem_desc" would likely already have been freed separately (remember: >>>> it will be dynamically allocated). >>>> >>>> With that in mind: >>>> >>>> 1) Is there a higher level "struct netmem_desc" freeing path where we >>>> 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) == PP_SIGNATURE; } 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. -- Pavel Begunkov