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 72EADC4332F for ; Thu, 8 Dec 2022 04:42:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B7F108E0003; Wed, 7 Dec 2022 23:42:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B2EE98E0001; Wed, 7 Dec 2022 23:42:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9F6C38E0003; Wed, 7 Dec 2022 23:42:17 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 8C5488E0001 for ; Wed, 7 Dec 2022 23:42:17 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 56755405EB for ; Thu, 8 Dec 2022 04:42:17 +0000 (UTC) X-FDA: 80217892314.07.0FC5A5B Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by imf12.hostedemail.com (Postfix) with ESMTP id 7F2BF40005 for ; Thu, 8 Dec 2022 04:42:14 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=bxVEtj9i; spf=pass (imf12.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1670474535; 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=j/PvhyblV7HKQCmFJuCM8xjUeOCgi9qEWjiHxHwW/fc=; b=4SbAaeWT8cs3U9kCEIa/FrYWm9Mkor3D5HdE/hwZ2idbQDVQNoZThAh38h+TEVxHWF48WP SquRdCYr+Dx4km3xf3G8DtFXQuQIlb6h6v0FD2pcVLgXs7+MSmM6nvYu/PBSoaXLsMfaUd Ahw+WOO70bArJHJgW0M/zrBrilTEqBU= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=bxVEtj9i; spf=pass (imf12.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670474535; a=rsa-sha256; cv=none; b=IVxZaNa8Ars7BWmo00XPJp9v34OSZzTum4RVXyMEDgzkDboO6ts46FcN69M8n+cmWZ+jyE YgLNKQrAFj4VP/bM1+Di3ir3yWdxYdJi3plHGugCyUyuTdG0daSOIdzah4EKJHhNmz88lZ K6HcauJGyAz84uLEXMdepewX7ULidZ0= Received: by mail-wm1-f49.google.com with SMTP id l26so227486wms.4 for ; Wed, 07 Dec 2022 20:42:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; 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=j/PvhyblV7HKQCmFJuCM8xjUeOCgi9qEWjiHxHwW/fc=; b=bxVEtj9id7NYGZWl5RP1fEHo9S7pZWpFPtW459Zq4i26NcLBhq4fFkGimTzsXqYGV9 tqNGoSWXTYlF+aqK4r6Omvq4Ib/HxORussF8BE3wjR8tq6P8l8V9Fbr7f+Ns74bWujMd 3QmmNRJFzhdXPv3LhqvcQYFz6sHimKnVddADlr2EJjzLFWBjvygGaq19JE8Dp+L6f9xQ fmE/4OzaFfvYI6vxGf8O2gAvuaE3olog9u4xBtqZ6pR70UYIzySiyX1FQmrM8zahKoMR J6F2a/dEejDoy33GLBsD0+9GmLMt869CQVSribquFY0OCEfDWvRp6ph2s8NOpijF9Jxa OF/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=j/PvhyblV7HKQCmFJuCM8xjUeOCgi9qEWjiHxHwW/fc=; b=FgTDehHWrXtrchzqHm2xJDzUfbkX6grMfBcAu746tOOH+/eT1QNX2ozmURvLWqSsLv +P81L+M0TUbQxKGlAp0VGE/padWVLlwEK7r9NEBVJwyJorSF4ABNv920FItNXOy2zfJZ zfhti+d7Diejm5jmJ149aFHpa73cte4AXBoBg+aS3SFFzjzeYf2Htip40gAYjzjc6B/a IYjlbISsJDRph7DSSoLTP0lJXS4KkCx/Jm1S14TJu9H6L0FNIgmwefoim3yY6c8i7o8Q 5ffcKg3aL658Ufy7XbJ+A6ZaUuXzYG5TmW6F0plQbZZf34Zzfh1AbC7QcBUm+Zc2/Mon PRjw== X-Gm-Message-State: ANoB5pkoA4K+m4ND3Gi8p7dQNdenVuoaX0/RdK+v09BgCP6ts2ln5xzg d9ScRHvBTQs7lPCi6KLcbsCErypNAo4pXm29eguYfw== X-Google-Smtp-Source: AA0mqf5joQloInZ0rfKwtDbqKUhM24CtsHzxr6in5aTp1K+dJQf69rQD0uBdSPPGaJ6lI9cJRyd9PXeHD1p3Y3YLvtM= X-Received: by 2002:a05:600c:4b1b:b0:3d1:d771:373 with SMTP id i27-20020a05600c4b1b00b003d1d7710373mr9524251wmp.18.1670474533107; Wed, 07 Dec 2022 20:42:13 -0800 (PST) MIME-Version: 1.0 References: <20221207223731.32784-1-sidhartha.kumar@oracle.com> <92965844-c430-8b8e-d9f1-705d7578bceb@nvidia.com> In-Reply-To: From: Muchun Song Date: Thu, 8 Dec 2022 12:41:36 +0800 Message-ID: Subject: Re: [PATCH mm-unstable] mm: clarify folio_set_compound_order() zero support To: John Hubbard Cc: Sidhartha Kumar , LKML , Linux Memory Management List , Andrew Morton , Mike Kravetz , willy@infradead.org, tsahu@linux.ibm.com, David Hildenbrand , muchun.song@linux.dev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 7F2BF40005 X-Rspam-User: X-Stat-Signature: tasoyf887oz76fdhpgjni9d4riofdtth X-HE-Tag: 1670474534-288267 X-HE-Meta: U2FsdGVkX19uZA/TJXteW9JiDZJJK8sKD1gNHnSDQDe485zj2eiX78rYmha2iNKhUsS/NNJY6NCAt9OgbhLXb5It0sifLIq59iK5cPYcBw0+8oKjhCPWWOPKLfEZWac2HGe3MXV0cWo5K6YaCStPr3YYM2BeTQ5hLwxE717UQwpRiiweLhgFQa3VjOqDyLYfbEyGkGRG6LnqN2SUEec36WvEk4fOlWHtjXC4L20KQNTF9G8Y10YODJG4BlmG6OCZIOYW20Z8Td95jyZNWe8XMZ3HspCipmUU/uA/JQuEiykCK9OM6Sd2R5eVPO81NBKvpSFh+hL45bsCJbMe10T/ac1OQQWN357nahl9BoVjW/T/9P9iSnCMOXXRFhWJV2wfEKV7s/xBrV+hIogHgpNpMpTlpR0lxcuwKaqTcEtbE0Hs9PNWMH/3oD3QgIhcu+HKs97JIbCam+Cb8qDPJBug3wdgokJqhwVSJ6yAHGhFuGwIiq/dpnOYKijYUoi4AE81St3RgR7Mq/4q/Oj3tUH+nzirXFSvCtxiuK1E2hFbuEZKlWCCfrbhI1be6b84jl/FORdS/C4PzMbYKgkQcR4443ePTOekW+Y+30QV/ynllJq8xFcVdsitzhTc4dboxlatxeI7oW31YVGBFHf6LmJvsyH+oRtegRE5fKTaC1ZXQ+KRQCCo/ZMLhZm87eUxlbV+3duEzvSIPbTxIhNRKAozzpBRQ4xFOiX9/J1E7fHtPVSexP1Ba4CLYd5ATEAFnxR4skMdsHRXeCY7I2He8ZwA7n50jZLPS1oCvOoUjSbuoZKwIABNpIOfeOGtmpcvW4JBW320efiCZ3eYgDQ5IZenev3uv0ZnylySkjwfZJJ/aEiQnwHczrMWlKMzEmGrmyRdYN/I7KXaNAJXwvcdcMms9PJX9sZmUXliH09KkgwSKIb7wmtHJWTXeFgaM9/eePzz65PWhjSPnrSWGcKQjpE bFEvmFvX rhk2pSQwzN8bLq8PrwWnUfrTzWV8czZjkE5Yq2/h9FzbnS6gcNbbf4wPn8a23n8QjDo6U9r1g1Ut1gMP0jtrlz6HIA9bkHrl57xY2qLW3UfmF6jyNBXYZtLtSBPdZcwjTrSAz 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: On Thu, Dec 8, 2022 at 10:27 AM John Hubbard wrote: > > On 12/7/22 17:42, Sidhartha Kumar wrote: > >> Wouldn't it be better to instead just create a new function for that > >> case, such as: > >> > >> dissolve_large_folio() > >> > > > > Prior to the folio conversion, the helper function __destroy_compound_g= igantic_page() did: > > > > set_compound_order(page, 0); > > #ifdef CONFIG_64BIT > > page[1].compound_nr =3D 0; > > #endif > > > > as part of dissolving the page. My goal for this patch was to create a = function that would encapsulate that segment of code with a single call of = folio_set_compound_order(folio, 0). set_compound_order() does not set compo= und_nr to 0 when 0 is passed in to the order argument so explicitly setting= it is required. I don't think a separate dissolve_large_folio() function f= or the hugetlb case is needed as __destroy_compound_gigantic_folio() is pre= tty concise as it is. > > > > Instead of "this is abusing function X()" comments, we should prefer > well-named functions that do something understandable. And you can get > that by noticing that folio_set_compound_order() collapses down to > nearly nothing in the special "order 0" case. So just inline that code > directly into __destroy_compound_gigantic_folio(), taking a moment to > fill in and consolidate the CONFIG_64BIT missing parts in mm.h. > > And now you can get rid of this cruft and "abuse" comment, and instead > just end up with two simple lines of code that are crystal clear--as > they should be, in a "__destroy" function. Like this: > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 105878936485..cf227ed00945 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -1754,6 +1754,7 @@ static inline void set_page_links(struct page *page= , enum zone_type zone, > #endif > } > > +#ifdef CONFIG_64BIT > /** > * folio_nr_pages - The number of pages in the folio. > * @folio: The folio. > @@ -1764,13 +1765,32 @@ static inline long folio_nr_pages(struct folio *f= olio) > { > if (!folio_test_large(folio)) > return 1; > -#ifdef CONFIG_64BIT > return folio->_folio_nr_pages; > +} > + > +static inline void folio_set_nr_pages(struct folio *folio, long nr_pages= ) I like this approach and helper name since it is more consistent with folio_nr_pages. > +{ > + folio->_folio_nr_pages =3D nr_pages; > +} > #else > +/** > + * folio_nr_pages - The number of pages in the folio. > + * @folio: The folio. > + * > + * Return: A positive power of two. > + */ > +static inline long folio_nr_pages(struct folio *folio) > +{ > + if (!folio_test_large(folio)) > + return 1; > return 1L << folio->_folio_order; > -#endif > } > > +static inline void folio_set_nr_pages(struct folio *folio, long nr_pages= ) > +{ > +} > +#endif > + > /** > * folio_next - Move to the next physical folio. > * @folio: The folio we're currently operating on. > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index e3500c087893..b507a98063e6 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1344,7 +1344,8 @@ static void __destroy_compound_gigantic_folio(struc= t folio *folio, > set_page_refcounted(p); > } > > - folio_set_compound_order(folio, 0); > + folio->_folio_order =3D 0; I suggest not touch _folio_order directly, we can introduce another helper = like folio_sert_order to set -> _folio_order pairing with folio_order. Thanks. > + folio_set_nr_pages(folio, 0); > __folio_clear_head(folio); > } > > > Yes? > > thanks, > -- > John Hubbard > NVIDIA