Received-SPF: pass (google.com: domain of linux-kernel+bounces-67295-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Zi Yan <ziy@nvidia.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
 "\"Huang, Ying\"" <ying.huang@intel.com>,
 Ryan Roberts <ryan.roberts@arm.com>,
 Andrew Morton <akpm@linux-foundation.org>,
 "\"Matthew Wilcox (Oracle)\"" <willy@infradead.org>,
 David Hildenbrand <david@redhat.com>,
 "\"Yin, Fengwei\"" <fengwei.yin@intel.com>, Yu Zhao <yuzhao@google.com>,
 "\"Kirill A . Shutemov\"" <kirill.shutemov@linux.intel.com>,
 Johannes Weiner <hannes@cmpxchg.org>,
 Baolin Wang <baolin.wang@linux.alibaba.com>,
 Kemeng Shi <shikemeng@huaweicloud.com>,
 Mel Gorman <mgorman@techsingularity.net>,
 Rohan Puri <rohan.puri15@gmail.com>, Mcgrof Chamberlain <mcgrof@kernel.org>,
 Adam Manzanares <a.manzanares@samsung.com>,
 "\"Vishal Moola (Oracle)\"" <vishal.moola@gmail.com>
Subject: Re: [PATCH v5 2/3] mm/compaction: add support for >0 order folio
 memory compaction.
Date: Thu, 15 Feb 2024 11:13:56 -0500
Message-ID: <01060AED-C371-4E3C-A1E6-77DADB3B4E60@nvidia.com>
In-Reply-To: <7d2b44bb-e3b0-435d-98ff-670b5029bd93@suse.cz>
References: <20240214220420.1229173-1-zi.yan@sent.com>
 <20240214220420.1229173-3-zi.yan@sent.com>
 <7d2b44bb-e3b0-435d-98ff-670b5029bd93@suse.cz>
Content-Type: multipart/signed;
 boundary="=_MailMate_B7C8D105-1764-4414-AB77-E568CB5ACF43_=";
 micalg=pgp-sha512; protocol="application/pgp-signature"
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?FLfDedM/RZnAwAHu2ggCAkQ00fijM0l95zbtunDdlxebZ0DbXpgsuh/oBlNT?=
 =?us-ascii?Q?Xc8dNGHMYQTfPCEbqZCEhsP1WObxYXAK53ZQWBdWvVUHyaLQxrCPtRmsZGXh?=
 =?us-ascii?Q?ROEvwz8zbYRiZfpMhz1yLRjy39/rVJs9ZeDRzCxmKdzZLMquxt0T0VoF3J+v?=
 =?us-ascii?Q?7C4TD93wJEHUusuuyUswRnkMRDZPhqreiofi0DByq7/KOe9FJJt+WZY3JjxF?=
 =?us-ascii?Q?c3FZ399Jv6b8E5a07wVv2Ma3ELMSA5639xDhnu0TrygxYAP2XfVDQPPG7k8+?=
 =?us-ascii?Q?6dyJIGZ6BaAbGYy8AKabYdyT2dLUG93FRvl+6HlJuFzT169UmgR8UJM3eMiK?=
 =?us-ascii?Q?/Xua86/X8a40mQny5Xr2uFs+ifYcLwjSHKP5ap8/4Qf0f8Bd94sQqQ+pN5E4?=
 =?us-ascii?Q?0BfJ1CPRg83wfsyOrK1jAgI/DF/wMRcRxo3H1lLez+4eYAQOI9OjhNkKmhxv?=
 =?us-ascii?Q?hkqnI7JcWRZ11WOkjCpUOlAV5GS3i12RKUAL25aeochh6vANyl1IJOjdIv+M?=
 =?us-ascii?Q?eDyNA3R9A7TS6hvrBkcBiAtN51EbzQFo7z9a63nqVpQwtKbOS0sl/1pxES/i?=
 =?us-ascii?Q?ofGLhmkMzkAaZgNIvTjb55zr3o48WysVH31QSHsOGYjePzxO82jFauVFACYU?=
 =?us-ascii?Q?3xsHv4mVJd7KwT/SINFEDeIbJBauqXm/LDkNaafrXL/ZZRBzklRRD0kx22YA?=
 =?us-ascii?Q?DuhQtc5GrAcx8yyWgNfHXaEYc2u8sxP1mdFvK+dh4MSPzism0pKynPiFPw2V?=
 =?us-ascii?Q?jhoVQg1EHwIFd/gdFB1Sh50YVHbSnpUz+/pfP6pMKG95OiKQ7oymUX7r1+ZI?=
 =?us-ascii?Q?eAr2SHKuS/xlpAHSaW4bVVoVyst+tmGnO3z2boBezcSuOQ03yyqGQ9kY/u8+?=
 =?us-ascii?Q?4I/1A/x6wlLACfpiHSmJ82subSH5SWLpCXo+kQpDm3Irif/UgZtr4/gfk4ld?=
 =?us-ascii?Q?056hELyTgsyI088adIhYk9hAjZeuzclbtyhSmyNmPTkGBp60WaGZuozzI2BU?=
 =?us-ascii?Q?5vf8IVbmkkO19gYACG16VYAgaMvVUGtKhiDvwSzZtjTNarL0Ag331g2++YEa?=
 =?us-ascii?Q?wjzhpoC/pImjsqAikHrAJluCmrhM7WuXkkU5WPHIJiZvn72qbvNdFWVGQtVr?=
 =?us-ascii?Q?fekgb+5in1JX+UxmKo7ARVbQE35952cZWI0AAmG7eLaJkmYE+Iud65NBgqbi?=
 =?us-ascii?Q?Eg2UQSy7m9GjQQrhPUwbhhu/IjT29f24zCARyClAyNI55asBb+ukH/feddeh?=
 =?us-ascii?Q?MjnpmBCTq62GJakI6t29tRKNrRLKMLtyQOkczYMQGFgIQM9RhcJuPBwIWp7V?=
 =?us-ascii?Q?E4eBPxZWpUQP6LPeqKhvz11ftPTmRPHzf9Ic/QGAf8ws/EYS72gylo5cnq5I?=
 =?us-ascii?Q?6sWuS4e2FjnRecCZDGLHnOZQQXmnVPMkibGSI/zUuIOCGCcxd2g+8Y8qoR61?=
 =?us-ascii?Q?dWr/XYPXuBSUCOqFT+4hh7hJrGHBaLqedqrGWk3wA7nXPn1dcWa5G+YfBmEg?=
 =?us-ascii?Q?DTvyh2VO0xfbb56aFGr4xGQaVU3vV2zfacS4dAUM2OU6IEj6aQbwwX8kHdL2?=
 =?us-ascii?Q?LOT3eYF/pwPw04sPXhw=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7ac6dc64-ccc2-4bdb-3ebe-08dc2e411e61
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5744.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2024 16:13:59.8494
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: xedW/vSdjarkO7F4QKBG8B0R7sZskQPkMyY1tiZYu5RFWF46epcU4fvskpErNUZD
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB7875

--=_MailMate_B7C8D105-1764-4414-AB77-E568CB5ACF43_=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On 15 Feb 2024, at 11:07, Vlastimil Babka wrote:

> On 2/14/24 23:04, Zi Yan wrote:
>> From: Zi Yan <ziy@nvidia.com>
>>
>> Before last commit, memory compaction only migrates order-0 folios and=

>> skips >0 order folios.  Last commit splits all >0 order folios during
>> compaction.  This commit migrates >0 order folios during compaction by=

>> keeping isolated free pages at their original size without splitting t=
hem
>> into order-0 pages and using them directly during migration process.
>>
>> What is different from the prior implementation:
>> 1. All isolated free pages are kept in a NR_PAGE_ORDERS array of page
>>    lists, where each page list stores free pages in the same order.
>> 2. All free pages are not post_alloc_hook() processed nor buddy pages,=

>>    although their orders are stored in first page's private like buddy=

>>    pages.
>> 3. During migration, in new page allocation time (i.e., in
>>    compaction_alloc()), free pages are then processed by post_alloc_ho=
ok().
>>    When migration fails and a new page is returned (i.e., in
>>    compaction_free()), free pages are restored by reversing the
>>    post_alloc_hook() operations using newly added
>>    free_pages_prepare_fpi_none().
>>
>> Step 3 is done for a latter optimization that splitting and/or merging=

>> free pages during compaction becomes easier.
>>
>> Note: without splitting free pages, compaction can end prematurely due=
 to
>> migration will return -ENOMEM even if there is free pages.  This happe=
ns
>> when no order-0 free page exist and compaction_alloc() return NULL.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> Tested-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> Tested-by: Yu Zhao <yuzhao@google.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Thanks.

>
> Noticed a possible simplification:
>
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -447,6 +447,8 @@ extern void prep_compound_page(struct page *page, =
unsigned int order);
>>
>>  extern void post_alloc_hook(struct page *page, unsigned int order,
>>  					gfp_t gfp_flags);
>> +extern bool free_pages_prepare_fpi_none(struct page *page, unsigned i=
nt order);
>> +
>>  extern int user_min_free_kbytes;
>>
>>  extern void free_unref_page(struct page *page, unsigned int order);
>> @@ -481,7 +483,7 @@ int split_free_page(struct page *free_page,
>>   * completes when free_pfn <=3D migrate_pfn
>>   */
>>  struct compact_control {
>> -	struct list_head freepages;	/* List of free pages to migrate to */
>> +	struct list_head freepages[NR_PAGE_ORDERS];	/* List of free pages to=
 migrate to */
>>  	struct list_head migratepages;	/* List of pages being migrated */
>>  	unsigned int nr_freepages;	/* Number of isolated free pages */
>>  	unsigned int nr_migratepages;	/* Number of pages to migrate */
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 7ae4b74c9e5c..e6e2ac722a82 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1179,6 +1179,12 @@ static __always_inline bool free_pages_prepare(=
struct page *page,
>>  	return true;
>>  }
>>
>> +__always_inline bool free_pages_prepare_fpi_none(struct page *page,
>> +			unsigned int order)
>> +{
>> +	return free_pages_prepare(page, order, FPI_NONE);
>
> Seems like free_pages_prepare() currently only passes  fpi_flags to
> should_skip_kasan_poison() and that ignores them. You could remove the
> parameter from both and declare and use free_pages_prepare(page, order)=

> directly.

Got it. I can send a cleanup patch after this series. No, to avoid unnece=
ssary
code churn, it is better to put a cleanup patch before this series and us=
e
free_pages_prepare(). Will do it in v6.

>> +}
>> +
>>  /*
>>   * Frees a number of pages from the PCP lists
>>   * Assumes all pages on list are in same zone.


--
Best Regards,
Yan, Zi

--=_MailMate_B7C8D105-1764-4414-AB77-E568CB5ACF43_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQJDBAEBCgAtFiEE6rR4j8RuQ2XmaZol4n+egRQHKFQFAmXOOEQPHHppeUBudmlk
aWEuY29tAAoJEOJ/noEUByhUmGgP/2BNwWsv6ZsY/3YWp9mAa8zO3dGiDPXhybOS
FTibDHhqaM4v1lLa7bj0dy9uQzkrNTd0f7RI7Zh2IP7eleffzo/8ajaszqHO10q/
Khyhgq0ltnCz11hd36SbjZdmALpm7uXNX637EGo9paBENQzvHMyo/MwBnsSXGiPW
alS42+T3gc8eSoewfZ5kLMTv/k0a9CbTpkiZDRKYpytzGYo2r7sUwecAjpz3rMOU
p/SoYfh6cuc0Q9zmNXuvDCjowQbc8+gV9Zl5VGmz6QfE/2am4DfRcCrtFmvWgBIT
pUhn36915hvXX11zfYoaUD8Y7FU80P7YC5hQtI+EXl0lMgXw2EGuJutRaXG+T84i
qzsC7gQvAOdLiRA2wW4uQb4U2yvutA0q3mBml4/b6A39bQR5BbxW66K86xplH/Vq
2deLwQICvDTJ7FplrufxAxl1HiOPikxRO8oepS2muRzc6lEKyaewlK/5BaC342FT
3WwcXzAz6smWKnoCbmRLn8oAfe0qkQISiSq/s7mkZI4K5P/9HwudROpPHXP8AM+z
qyOE1fExuZSZJiCqLlSgXNxc2OuaZ9Y3Amnm39hXIEKZTtBf18mV2m74ewr48CHe
Or9CYGIV3uTuQ7atMqFZblguYDLtwaBM2k/RuH18y2Acc0w4JiUSxYZexKSxTEnD
TPLSExcg
=uxam
-----END PGP SIGNATURE-----

--=_MailMate_B7C8D105-1764-4414-AB77-E568CB5ACF43_=--