Received-SPF: pass (google.com: domain of linux-kernel+bounces-200758-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Zi Yan <ziy@nvidia.com>
To: Luis Chamberlain <mcgrof@kernel.org>,
 "Pankaj Raghav (Samsung)" <kernel@pankajraghav.com>
Cc: xu.xin16@zte.com.cn, akpm@linux-foundation.org, v-songbaohua@oppo.com,
 mhocko@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org,
 yang.yang29@zte.com.cn, David Hildenbrand <david@redhat.com>
Subject: Re: [PATCH linux-next] mm: huge_memory: fix misused
 mapping_large_folio_support() for anon folios
Date: Tue, 04 Jun 2024 06:57:28 -0700
Message-ID: <E5C46860-D455-4300-98A0-A8C2081F6FF0@nvidia.com>
In-Reply-To: <E30FDAA1-B4D4-41C9-993C-0AD5044F30C0@nvidia.com>
References: <20240604134738264WKaKYb3q_YTE32hNAy2lz@zte.com.cn>
 <fe35d494-b54e-4302-8c75-24abc9094ea1@redhat.com>
 <E30FDAA1-B4D4-41C9-993C-0AD5044F30C0@nvidia.com>
Content-Type: multipart/signed;
 boundary="=_MailMate_01E4B2E9-59F6-41DB-96D2-C9FD75587B8B_=";
 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?lZRncpShFs2p7xQ+SVOklWumugfIeEpCdtgPuj7ZJGNzydHdgBq9kbJTV5xU?=
 =?us-ascii?Q?PzDhleiWTu1q8c+EN1DjfYzp2wMUHXSVp1a/j1iOFVx69+mmrjCt6MhuYm1V?=
 =?us-ascii?Q?u7qmeW2vKt7rJc7YiYvJpnrhAaxIm78D92irYXByvgiVfQeOuAiwE0ayow3Y?=
 =?us-ascii?Q?b8EYtDOd9vGcck2s5lbPGbievhk23jsk9SIHDXfZVHYV17PNvDVkwFm5qK+t?=
 =?us-ascii?Q?IKccSe6CmJ9p8c/5YJzSs+EBu6RSly6sEJqZL6ZLkZRfJOAOIygi21GouUmK?=
 =?us-ascii?Q?E5zQ0f/4RVS30ZesjrUxgskJ6k/GJfO+L1cIdEK5M/vHi/y8KID4M9ow5RlQ?=
 =?us-ascii?Q?vFv5jHaT7pe8E81xmWK/DSJiTRbX+mBK1GL3Y1gXKDVqvvxWzmwJRbqbcFxk?=
 =?us-ascii?Q?nF3geOY/j+MnfXiBCvtIWJRfw3tp3CouCDdHHxlH6iCbyKtXzuUTz1vOmJOg?=
 =?us-ascii?Q?EcW9sAj3ZZ5HlC4x/3FmfBhynTc+kCzfYsHNqluyqZSFlN0XXS/KbikcN8+r?=
 =?us-ascii?Q?fQD1oRtYykOuUtSn533N1KahncUARu44JYmZ/hrJJjFDSY4DHRK0SJqCTCwi?=
 =?us-ascii?Q?iOPPmMRlO4guyGzpBrrtVJum+3erdYn2tj9MUsr22rHpL7FyjCc7xZPl/JSM?=
 =?us-ascii?Q?yk1u7l5KzNKOuAXv0YUXeczjz78gH8F3quQa6mg4xqKUSe9WWGaRaknWsdnM?=
 =?us-ascii?Q?NS4gTWMimIC2/q2S+7xFp/P6tcHLWmz+87fSM4hh3L38E5qaGezpMFR7emvV?=
 =?us-ascii?Q?+kyYVWcwcx9hQPeJFAYhAZBdH7mvsoO5NOEl0QoS7LwIYHpQFtW3FWSVy7sS?=
 =?us-ascii?Q?eZH1RNRGih3oOj4h5DgImOu8xVHDXMtXnNvgytVWJiAJzMUz884IlGA/rUW2?=
 =?us-ascii?Q?qIj/bCcQi1SUVF2p+EUXXE5E5NFvIc1ta6j3QuGCBH4+Da73qQ0qQLpQLXjy?=
 =?us-ascii?Q?m6/ZRhmr1PcsaVvXWQpGyqTaEyA0p4CtlfGad+mWEEWpCtoqqp5/TZocvCeq?=
 =?us-ascii?Q?v9686qqADbNjb7dQ0yqscxXJ7MBwFbxMFsVl0gVX9278mZCeI2IKoVdqXpZv?=
 =?us-ascii?Q?uKe7dwcUO9byq3p+TdaxZtK/VaWZlqHIH3Tvsnktf49Cl6+NDcX03FPAWpa8?=
 =?us-ascii?Q?REBXlHyw79PorRlW/KGsxswDxwlnCnR3ypH0HJu+a6xZAH2em+RqrADKGB7o?=
 =?us-ascii?Q?EBa3Q8NOJHE2XTJzcVZRFhL7oZ4IBwG2VLg+GrPd0iFB9g7dH5+tUEU11Kw3?=
 =?us-ascii?Q?/HoGN0HcuqIQcXfnWsiO7/8OcSo/HqIFesn/rmrqEIeHjmg5ITSPNvOE0uyu?=
 =?us-ascii?Q?dVPyiXlFlf9vw5MswdWp/gDH403wmmfNt4SI5T5hiBXpUBevU9rFeU5kV/as?=
 =?us-ascii?Q?0ZR1xW095MrFOYigeXxK7SL9tiz3RByUCEzwkL11aKd+dL18f66W/dDczb+w?=
 =?us-ascii?Q?TB+10VWgfJFjFT2OBQMZHO8bxUle14UHaweqrG7Nouz1xGaIefvek42btVp5?=
 =?us-ascii?Q?OsNWAG19IvFSb7+tMBQ3p0APp9qMicSfs5/1XRF8H1/Atmg33lbPvp19njpl?=
 =?us-ascii?Q?ClCDxhBJEmdGG5EUKSY=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 528f80c1-cfb0-4d4c-cdc2-08dc849e4831
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5744.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2024 13:57:33.1850
 (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: qKs/I+dn+sBy3Hbu+In+SvJsC49z7Ijl9cwCRcXCJ4euZ4nNpHx7KhwCMzcwgkXj
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4068

--=_MailMate_01E4B2E9-59F6-41DB-96D2-C9FD75587B8B_=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

+Luis and Pankaj, who are working on enable bs > ps in XFS and touch spli=
t_huge_page_to_list_to_order().


On 4 Jun 2024, at 6:52, Zi Yan wrote:

> On 4 Jun 2024, at 0:57, David Hildenbrand wrote:
>
>> On 04.06.24 07:47, xu.xin16@zte.com.cn wrote:
>>> From: Ran Xiaokai <ran.xiaokai@zte.com.cn>
>>>
>>> When I did a large folios split test, a WARNING
>>> "[ 5059.122759][  T166] Cannot split file folio to non-0 order"
>>> was triggered. But my test cases are only for anonmous folios.
>>> while mapping_large_folio_support() is only reasonable for page
>>> cache folios.
>>
>> Agreed.
>>
>> I wonder if mapping_large_folio_support() should either
>>
>> a) Complain if used for anon folios, so we can detect the wrong use mo=
re easily. (VM_WARN_ON_ONCE())
>
> This is much better.
>
>>
>> b) Return "true" for anonymous mappings, although that's more debatabl=
e.
>
> This might fix the warning here, but the function might get wrong uses =
easily.
>
>>
>>>
>>> In split_huge_page_to_list_to_order(), the folio passed to
>>> mapping_large_folio_support() maybe anonmous folio. The
>>> folio_test_anon() check is missing. So the split of the anonmous THP
>>> is failed. This is also the same for shmem_mapping(). We'd better add=

>>> a check for both. But the shmem_mapping() in __split_huge_page() is
>>> not involved, as for anonmous folios, the end parameter is set to -1,=
 so
>>> (head[i].index >=3D end) is always false. shmem_mapping() is not call=
ed.
>>>
>>> Using /sys/kernel/debug/split_huge_pages to verify this, with this
>>> patch, large anon THP is successfully split and the warning is ceased=
=2E
>>>
>>> Signed-off-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
>>> Cc: xu xin <xu.xin16@zte.com.cn>
>>> Cc: Yang Yang <yang.yang29@zte.com.cn>
>>> ---
>>>   mm/huge_memory.c | 38 ++++++++++++++++++++------------------
>>>   1 file changed, 20 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 317de2afd371..4c9c7e5ea20c 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -3009,31 +3009,33 @@ int split_huge_page_to_list_to_order(struct p=
age *page, struct list_head *list,
>>>   	if (new_order >=3D folio_order(folio))
>>>   		return -EINVAL;
>>>
>>> -	/* Cannot split anonymous THP to order-1 */
>>> -	if (new_order =3D=3D 1 && folio_test_anon(folio)) {
>>> -		VM_WARN_ONCE(1, "Cannot split to order-1 folio");
>>> -		return -EINVAL;
>>> -	}
>>> -
>>>   	if (new_order) {
>>>   		/* Only swapping a whole PMD-mapped folio is supported */
>>>   		if (folio_test_swapcache(folio))
>>>   			return -EINVAL;
>>> -		/* Split shmem folio to non-zero order not supported */
>>> -		if (shmem_mapping(folio->mapping)) {
>>> -			VM_WARN_ONCE(1,
>>> -				"Cannot split shmem folio to non-0 order");
>>> -			return -EINVAL;
>>> -		}
>>> -		/* No split if the file system does not support large folio */
>>> -		if (!mapping_large_folio_support(folio->mapping)) {
>>> -			VM_WARN_ONCE(1,
>>> -				"Cannot split file folio to non-0 order");
>>> -			return -EINVAL;
>>> +
>>> +		if (folio_test_anon(folio)) {
>>> +			/* Cannot split anonymous THP to order-1 */
>>> +			if (new_order =3D=3D 1) {
>>> +				VM_WARN_ONCE(1, "Cannot split to order-1 folio");
>>> +				return -EINVAL;
>>> +			}
>>> +		} else {
>>> +			/* Split shmem folio to non-zero order not supported */
>>> +			if (shmem_mapping(folio->mapping)) {
>>> +				VM_WARN_ONCE(1,
>>> +					"Cannot split shmem folio to non-0 order");
>>> +				return -EINVAL;
>>> +			}
>>> +			/* No split if the file system does not support large folio */
>>> +			if (!mapping_large_folio_support(folio->mapping)) {
>>> +				VM_WARN_ONCE(1,
>>> +					"Cannot split file folio to non-0 order");
>>> +				return -EINVAL;
>>> +			}
>>>   		}
>>>   	}
>>
>> What about the following sequence:
>>
>> if (folio_test_anon(folio)) {
>> 	if (new_order =3D=3D 1)
>> 		...
>> } else if (new_order) {
>> 	if (shmem_mapping(...))
>> 		...
>> 	...
>> }
>>
>> if (folio_test_swapcache(folio) && new_order)
>> 	return -EINVAL;
>>
>> Should result in less churn and reduce indentation level.
>
> Yeah, this looks better to me.
>
> Best Regards,
> Yan, Zi


Best Regards,
Yan, Zi

--=_MailMate_01E4B2E9-59F6-41DB-96D2-C9FD75587B8B_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

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

iQJDBAEBCgAtFiEEh7yFAW3gwjwQ4C9anbJR82th+ooFAmZfHUgPHHppeUBudmlk
aWEuY29tAAoJEJ2yUfNrYfqKZREP/2JcrAqmQ85b4R30JsfmWOihbYj0UMpv25ih
ZVDC+K4ceHYApboC2ijOIIQUE7lsmDWP1y5TlF1SivcRysn9wX3oSpaOUdqOTHRa
tWiRI5A/e1LqzQsXFaW6N+O+sayR+3i58pjyGbp05e/bqYnEXnwR6Hju+B4VuEse
YpwHPfyt0QR+CJ/pMRNOl06ZrmGP2eOUEFpZVRiZa7FO8xD0xbuNcB4Ze93m4X/R
WNtD8g8ThVGRMeaD+yU22at/i1b9yfcO0tKbRN7Jxenh1E4tarXqccPFWqwckB9l
sn2AgOn70tHnwr2AnxirLFFq7frKtkd3ef6gYyO+7OKmgYbVs04INQ3+TVQfOyRr
74QjVW/kmejoivWlzksYWKXfGOoCInmOqPnSXuahWnv5NH7muBUSqeOu31GU+Q6a
vTLkrMkg1F/lN9uEHK7hzI6W58J5BM55JmXIhWO00ETqOeKV6sQncizt9WN8tzm9
d94dp8SYptvRfdT24OoYlW8R5iqQA5yAJJzIIDSpEha3qi5+f7DMLiiEP/HLS7x3
907mAItKzqzHjLk9S5CPHxqdu70nfCzuDfJLu4LC2/tgrIidN/Zhi4oyBhBayXjO
eemcoI6YS2QqGllS/VPxuJTn0aZbuZls8U23Ul1N4SBgqAPRtITtSsSxlh4M1XWg
MHMGm+/T
=JihK
-----END PGP SIGNATURE-----

--=_MailMate_01E4B2E9-59F6-41DB-96D2-C9FD75587B8B_=--