Received-SPF: pass (google.com: domain of linux-kernel+bounces-59952-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
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 v3 1/3] mm/compaction: enable compacting >0 order folios.
Date: Fri, 09 Feb 2024 15:44:35 -0500
Message-ID: <5FE83378-555A-4207-A8E2-12C820DDA666@nvidia.com>
In-Reply-To: <b1db01bc-e0a3-42d3-8193-6e4bfd7c9f86@suse.cz>
References: <20240202161554.565023-1-zi.yan@sent.com>
 <20240202161554.565023-2-zi.yan@sent.com>
 <c42a6c9f-7d45-428c-95b9-98367ddba9d3@suse.cz>
 <24D4AA55-42F5-4E9B-822D-2BE61D0FA51F@nvidia.com>
 <b1db01bc-e0a3-42d3-8193-6e4bfd7c9f86@suse.cz>
Content-Type: multipart/signed;
 boundary="=_MailMate_66D5A86F-B943-4699-BFF0-294BC2023A29_=";
 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?FY4tKVxKiDIvwS1rLXzM8dLaBsZclrK0Q6CBl3/2YV7kh2jbmkST34eu9sNA?=
 =?us-ascii?Q?TAX/wsj/BZoe7hQmeHHat0evFil17xbBjGE863cWRb7sg7U/Of0EJD7DnpvD?=
 =?us-ascii?Q?sI5V1LB8adsg+HaSPoP+jiQxp8ZLyQgGLZaqHmZUSvVdUqZwPKV4FW7ffrG1?=
 =?us-ascii?Q?CSuCXsQxdP0WKxmhEMvAC3ppUOpXG+W+GQOFypxD+rStSpmz1BGtSbmVSyLW?=
 =?us-ascii?Q?Fa0yasVg5hLBnuLUjX+g/GJbfe4T8LKwu3tkfB10kqatfvqcujwWpK5Rrves?=
 =?us-ascii?Q?7/2y/lJWILJImIa6pps21ssrD4Du5R1ltOUzNyza+U2t80UfOUDnDQMLtf0a?=
 =?us-ascii?Q?mNyZOBmTQn3cFQC/dn4/IOqcLfF7ksg3QkxF4sYdVS8ZajgOr7tTImlnaG9U?=
 =?us-ascii?Q?9PXHSiMUQ34tmb+c9YqsI8qcilri1vXcgk2YHWkU44ugZOumf72IJkdpH2E8?=
 =?us-ascii?Q?4L1VTrg2PwofkA5FYlkrJmTOY1Pi34koOTvOVs1k44hsLhH90/zwhbfd4tlG?=
 =?us-ascii?Q?HTSuupaieHrZ7yBnwnGZVHj20Ow2f2/1p0QPL3Sb6CbLoR7jJ7FgmNfQfnXp?=
 =?us-ascii?Q?ML6CpRN6ZuOmX+227cLqF3cZ/VnZupnpyexEASHw7RC49UyWkhlOGyxsaFQp?=
 =?us-ascii?Q?5Ko8oA+5TEprhdDO3CA7GScFneeZbzJ8y6xGZZ4lDl3a+edZ09uZnmjtAjml?=
 =?us-ascii?Q?VKtt0NR6z+P4OqxDDrGLuzMx7Pe8lBFb4ZeztMMXbVGjX4Z0dUf/iVYgg1wU?=
 =?us-ascii?Q?8HHirquN1OEbPQ7t504M6lfuZL01IEiAr2/lCDa2shDpp9T2EOCyE1DvbkK5?=
 =?us-ascii?Q?r3m86EGKWyGFUCfsMuRa4FHY47A/m5gn5SRlxmd/78k+OyLx5NzScqVP8jsl?=
 =?us-ascii?Q?ZasvcCRxRkYy3WvZrtxsxnCKpokycudrL46y4c3Gw0Od0f9oo20EFQsXaLt7?=
 =?us-ascii?Q?jlhYAUF/1MPon6UxUJ6jgdNKYMHYZ9WYV5R6ZgodhU2a1oPBSfHrV0Hd7GK2?=
 =?us-ascii?Q?qI4EWhMjcPu0n/Yh2Tfx5M075W2OjWSQIiUTOXkd24BNHctqSW9EMYl11IzA?=
 =?us-ascii?Q?KYv5vH6nl22OVSqPBSNcdu81vETUHDsnXTjdkmfIK/+y37aZSm2UghBQU1z+?=
 =?us-ascii?Q?hRS7LDWPMVlFdRFB0ZqLNOXNRPuCwfjV6T8MmeCLlgIvQ4uf5NA+USZurXdl?=
 =?us-ascii?Q?9rpXBICUE4u4gKMO/tbl4w2bS8+aQ0vNEbUi5+A+HqPBAusNU5qqU16vglwR?=
 =?us-ascii?Q?YqhjIq2SnSTS4WAV5V1/K17MJdnf1N7Z4RR4QeaQt/x39aDwN8WcOTPdjbnV?=
 =?us-ascii?Q?xdnk7qH50SFFho4GZK+dVu1+bc1ZqDNbPHkv/Xv+VwCs9ddEtZZU0d1N+TLL?=
 =?us-ascii?Q?IiLWCCpFqWatVSBPJCWNw8c15ErsLXTU67WhzXV8fiVMfI9GjoHHMKTW14aJ?=
 =?us-ascii?Q?qV/XmqKdSQSBQ2OGHxBd0Y9qjcwAnzM9LdfzEMa/KxPSNc5v76huJ3/s/P5I?=
 =?us-ascii?Q?+rTlymddUrVODV5oB3I3DUcG7BfLhEdDVKOOpH40/ZQEn5CR+s7tEITJeBIr?=
 =?us-ascii?Q?ULSiM9HkmJWQIVBecH4kFlSWPpEfCkUkE83gpxr0?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7a346ec7-b049-43bf-4365-08dc29afeef6
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5744.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2024 20:44:38.5889
 (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: nUpkjKEaaEVc1WpXI5+epyYrxBcGxN9xdjDzq3Oo6q4Tkgh1kcbkgEAT6GeJuAm7
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4207

--=_MailMate_66D5A86F-B943-4699-BFF0-294BC2023A29_=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On 9 Feb 2024, at 15:43, Vlastimil Babka wrote:

> On 2/9/24 20:25, Zi Yan wrote:
>> On 9 Feb 2024, at 9:32, Vlastimil Babka wrote:
>>
>>> On 2/2/24 17:15, Zi Yan wrote:
>>>> From: Zi Yan <ziy@nvidia.com>
>>>>
>>>> migrate_pages() supports >0 order folio migration and during compact=
ion,
>>>> even if compaction_alloc() cannot provide >0 order free pages,
>>>> migrate_pages() can split the source page and try to migrate the bas=
e pages
>>>> from the split. It can be a baseline and start point for adding supp=
ort for
>>>> compacting >0 order folios.
>>>>
>>>> Suggested-by: Huang Ying <ying.huang@intel.com>
>>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>>> ---
>>>>  mm/compaction.c | 43 +++++++++++++++++++++++++++++++++++--------
>>>>  1 file changed, 35 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/mm/compaction.c b/mm/compaction.c
>>>> index 4add68d40e8d..e43e898d2c77 100644
>>>> --- a/mm/compaction.c
>>>> +++ b/mm/compaction.c
>>>> @@ -816,6 +816,21 @@ static bool too_many_isolated(struct compact_co=
ntrol *cc)
>>>>  	return too_many;
>>>>  }
>>>>
>>>> +/*
>>>> + * 1. if the page order is larger than or equal to target_order (i.=
e.,
>>>> + * cc->order and when it is not -1 for global compaction), skip it =
since
>>>> + * target_order already indicates no free page with larger than tar=
get_order
>>>> + * exists and later migrating it will most likely fail;
>>>> + *
>>>> + * 2. compacting > pageblock_order pages does not improve memory fr=
agmentation,
>>>> + * skip them;
>>>> + */
>>>> +static bool skip_isolation_on_order(int order, int target_order)
>>>> +{
>>>> +	return (target_order !=3D -1 && order >=3D target_order) ||
>>>> +		order >=3D pageblock_order;
>>>> +}
>>>> +
>>>>  /**
>>>>   * isolate_migratepages_block() - isolate all migrate-able pages wi=
thin
>>>>   *				  a single pageblock
>>>> @@ -1010,7 +1025,7 @@ isolate_migratepages_block(struct compact_cont=
rol *cc, unsigned long low_pfn,
>>>>  		/*
>>>>  		 * Regardless of being on LRU, compound pages such as THP and
>>>>  		 * hugetlbfs are not to be compacted unless we are attempting
>>>> -		 * an allocation much larger than the huge page size (eg CMA).
>>>> +		 * an allocation larger than the compound page size.
>>>>  		 * We can potentially save a lot of iterations if we skip them
>>>>  		 * at once. The check is racy, but we can consider only valid
>>>>  		 * values and the only danger is skipping too much.
>>>> @@ -1018,11 +1033,18 @@ isolate_migratepages_block(struct compact_co=
ntrol *cc, unsigned long low_pfn,
>>>>  		if (PageCompound(page) && !cc->alloc_contig) {
>>>>  			const unsigned int order =3D compound_order(page);
>>>>
>>>> -			if (likely(order <=3D MAX_PAGE_ORDER)) {
>>>> -				low_pfn +=3D (1UL << order) - 1;
>>>> -				nr_scanned +=3D (1UL << order) - 1;
>>>> +			/*
>>>> +			 * Skip based on page order and compaction target order
>>>> +			 * and skip hugetlbfs pages.
>>>> +			 */
>>>> +			if (skip_isolation_on_order(order, cc->order) ||
>>>> +			    PageHuge(page)) {
>>>
>>> Hm I'd try to avoid a new PageHuge() test here.
>>>
>>> Earlier we have a block that does
>>>                 if (PageHuge(page) && cc->alloc_contig) {
>>> 			...
>>>
>>> think I'd rather rewrite it to handle the PageHuge() case completely =
and
>>> just make it skip the 1UL << order pages there for !cc->alloc_config.=
 Even
>>> if it means duplicating a bit of the low_pfn and nr_scanned bumping c=
ode.
>>>
>>> Which reminds me the PageHuge() check there is probably still broken =
ATM:
>>>
>>> https://lore.kernel.org/all/8fa1c95c-4749-33dd-42ba-243e492ab109@suse=
=2Ecz/
>>>
>>> Even better reason not to add another one.
>>> If the huge page materialized since the first check, we should bail o=
ut when
>>> testing PageLRU later anyway.
>>
>>
>> OK, so basically something like:
>>
>> if (PageHuge(page)) {
>>     if (cc->alloc_contig) {
>
> Yeah but I'd handle the !cc->alloc_contig first as that ends with a got=
o,
> and then the rest doesn't need to be "} else { ... }" with extra identa=
tion

OK. No problem.


--
Best Regards,
Yan, Zi

--=_MailMate_66D5A86F-B943-4699-BFF0-294BC2023A29_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

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

iQJDBAEBCgAtFiEE6rR4j8RuQ2XmaZol4n+egRQHKFQFAmXGjrMPHHppeUBudmlk
aWEuY29tAAoJEOJ/noEUByhUi4sP/jmE7G+XwVjTsP/UkOLtl9M7BfmM2NRWlaBz
O7Gw+KcX4rqBFqtFogndeaOneTqmeks+cmZvQLPXw2eir2XDhtYm0Xi0cxzWeJXs
9VNCUAdm081oWTcYpCEwMq+l2FON5GEj2vgZlsZRlg6jJiB5QbzhVOWbJjrjTJAr
SyupY0n4IwoMubNGt/ELMoH72hKaC3uWiOIjJs04yON3sZ4K1//LS3Fug++/kDRG
bIZ+LFAj7vRiaopO7+D+RAKrPrMC20y9ueMLfFJzxTZXX7qoYlI1E5x+tdU+6FO7
8SpA0XI0JJBpaYeG2iO/FRMAJCODUpVhIHVJiyFVoDRhVhH2Ht8IZS95hLqemxKX
Xk82SKhwqufAnR8G2EBDncMjEjMXLt9ft9x6GhEp7yXI3b4EokdxgwuWRO3KbfiW
3kgk+dgXQuXcDbXooz6unFt7LV0fPkXPwhX9O4Pbve2dLMS2yv3ky0ziXMVRNtgC
DCsA+V+Dx5M600t5SFXiGPqqRijTjpXUvgP7kHVty1TP0m/wzN5EzkzIlV36JFf9
psHYT6JzPCpIifgWMKRbkE0zt+HguoItlXD1DDQX9ldUM5uWpeMVvHOb2tGIXhQz
QfGFM44yrNH6cVqMmKOlvVVQzQ7BnlulJsEzs+NS6KrOe0RHvyHoENvB6qxWSZKu
dMSUn9Q1
=hMm+
-----END PGP SIGNATURE-----

--=_MailMate_66D5A86F-B943-4699-BFF0-294BC2023A29_=--