Received-SPF: pass (google.com: domain of linux-kernel+bounces-204473-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: xu.xin16@zte.com.cn
Cc: david@redhat.com, v-songbaohua@oppo.com, akpm@linux-foundation.org,
 linux-kernel@vger.kernel.org, linux-mm@kvack.org, mhocko@kernel.org,
 yang.yang29@zte.com.cn, ran.xiaokai@zte.com.cn,
 Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH linux-next v2] mm: huge_memory: fix misused
 =?utf-8?q?mapping=5Flarge=5Ffolio=5Fsupport=28=29=C2=A0for?= anon folios
Date: Thu, 06 Jun 2024 07:28:31 -0700
Message-ID: <51DED95F-832A-4836-AA81-556968F6B645@nvidia.com>
In-Reply-To: <20240606174203124_OW-VQZ_ZLm4lGEimA-K9@zte.com.cn>
References: <20240606174203124_OW-VQZ_ZLm4lGEimA-K9@zte.com.cn>
Content-Type: multipart/signed;
 boundary="=_MailMate_01928395-E6FC-4913-90CA-7618991F85B8_=";
 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:
	=?utf-8?B?S015elp6OXdtT3YwOENMUW5Ea0pBQ2xrV0o1bFVIRENTVlNrQlJHTk8zYXEw?=
 =?utf-8?B?WHl4YXJVb1pSMjNHZ21Vd1FRQS83UzQ4SythaVNTdnhwVFJLZ2w5eHdzb21B?=
 =?utf-8?B?c091Q05wY0didEwydExDdFhtdGVhbVdnR21majlwYUltVDdWNGt4N3cyOVJ0?=
 =?utf-8?B?eGJrWjZjaDdyYzdld2d2R3hsUzVOYTVqUGRnSE1iN0NBTC9objh0UmFySmw0?=
 =?utf-8?B?NnRSdXBrTVg2NTNkSzNKeUtuYWd3S0pnL1FkMk1tMlovdVVPK09QY0o5TzRV?=
 =?utf-8?B?RVdQWWVnb3dodmYxN1ZpOWJkYmxDK05oUDVOeEtrNWNJYm5LQk1weU5zNVFB?=
 =?utf-8?B?QzRrSEw0V3g2bHZ1MkpVb3EwTm5RQUtodFVieDFBdmdhd1FuU2VFTGtIaTQy?=
 =?utf-8?B?ck5IWXJvOXdLQnBrUXVQSFRsZ0NLMGhZdjhuSHNtbkY0Uk9KSWs4YVdpZTNZ?=
 =?utf-8?B?bEZzNHlpQlVodUdXVnVmRE5XWmtud2RzZFZNUGZQUWtjZ3JIMXcva21wNlhZ?=
 =?utf-8?B?aXpYUGZFdXJXUDlmeW1LWlMxNGRXQ1h0UjdkcEdjQ1p2S1BCMksxQW93UWFO?=
 =?utf-8?B?UWNTTlJ4a0pzTVhyTUU1YVdOYTNDTXFBdTF4dVk3ODhoTWJMM1Nob1VnWW5q?=
 =?utf-8?B?cGhWOWRGS3pyMUs4MWN2U2w0VXo3QjJBRU81MUFIMDJvNm1ZZHloaGZXc1N0?=
 =?utf-8?B?bU11VWNBNlplcm9IbHBQQUw5SG5lU04vTHZwR3ZncFl5eDZPS3k5VmJqa2Uv?=
 =?utf-8?B?d1RDRkxHeENrbHQ0czhDWVZUeUlGZ0wrY2hUYVlnaTloT2srOGZqbUkzb3Bm?=
 =?utf-8?B?dTFVaExxbXhGTnhpcExLRGpxbjhwaDV6WmxCUkdqV09NRGVMMkNXblpnMlRq?=
 =?utf-8?B?ZnFZSEdSa29oTkt2MUtvcm90OUR2WmNzRVNZQmhnTUlKazVPMnRoU3B3dElq?=
 =?utf-8?B?SGJWTU1ZVVlObC9pODkvQktUdGl4VC9ROHFhbXo4VUpPZ0tEQmlkM3IxQTlG?=
 =?utf-8?B?cDI3WS9DdVVSbm9ablVJQ1ZwQmRpUEswekduRUVhUFZIdWY2ZFpGZ3lhZmpk?=
 =?utf-8?B?ek1uR3B2R3dZdmlxME4wbXF6SEE5cEt1UzB3NnhNQXU2UTNnVmM2RWUrNG1w?=
 =?utf-8?B?bUdmK3RRY0NoWU56LzAwNWNuZlFpamN4bEJ6SndBN252ekdqSjlmaldIeGdq?=
 =?utf-8?B?VWtVSkRtN0tKQk1qUUt6dzdtRTFFVGxickY3cEh6NGFRdnNyeDlHN0hudTFk?=
 =?utf-8?B?SVFNWlRkTEVqc3NnaldzTGZWSUZmNTVCc01RQXN2WmRUekkyVlNVUE0veUdq?=
 =?utf-8?B?b1B0Nnd0UXZiMWdZTXFva2JvNmV1b21YZ1gwZ1ZCV0xERmNNQWJBTDd4MkJM?=
 =?utf-8?B?OTZhZUg4M0U3bTJaRXJIam84RzVSSWgvU3FJZGV0NkNJS2hNSThLVEZtU01T?=
 =?utf-8?B?Z3BCOFdBK21iQXd5d3dtYTM4alZWbHM2dldxdDFPcWwxbTVRWnVodjcxaGIw?=
 =?utf-8?B?cGVZWDhVS09raFNMb1cxTDFwbk9SUUhwUWo2aXVYZGZ6WDdTVkNYVExvcUtS?=
 =?utf-8?B?cS9GaURheXBDWS9jOE41WDJ6dFZkR2NpZVZUeWh4ZVEvY3ptblVZSjY1Vy92?=
 =?utf-8?B?OHlzTVV5ZFpZYkU3N2o2cEpkd1ZGUmJsbm9qVngxSlVLdlVoNkF0SmdZajhB?=
 =?utf-8?B?WldlS1pYTXdJQ3NzUG0xUEZUUTJKajlaZ2ZTZy9qQW0xdU5VYUVaQWs3RUR4?=
 =?utf-8?B?MmU5Sys4UEd6aE9nU21na2orVkJ6ZHgvNXBDQUF3S0xMNEk1b2ZsazRpU2E0?=
 =?utf-8?B?aitvaGNTT3l6WjNPUCtMdUQ3aWhrL2U2S1AxRDM2UGNBNkIyaWRoZ042KzFl?=
 =?utf-8?B?RWJZRGZEY0NmSWVBUFhYRVplOUlzbGNrcy9JeXRVcEtZbEFaUFY5Z1p3ZW13?=
 =?utf-8?B?dUtEL3pib25VY3BHeVByb3kvL3BycTFZdXpTZ1VFNCtveFhOVFBGZW5ONGpR?=
 =?utf-8?B?TmxNTURaeWlvMzVROHZYZkt6NHFIUWFFc3hPa0hEc3M3QXU4d3B3REFmdE9w?=
 =?utf-8?B?OUpqaDl2bTlEV0ZabE00U0NJNyt2R0YzYk14OEN4ZmRlZ0N6eFhmcFFNOC9X?=
 =?utf-8?Q?FP1o=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e3aec396-40c6-4250-e375-08dc8634f3a7
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5744.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jun 2024 14:28:36.5617
 (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: vACHrMDFmpc0tXPiukr+ZiNhKfkET60qCdfEv7oZiQXA1gT6PbkjLf+WHGrY9D+C
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4469

--=_MailMate_01928395-E6FC-4913-90CA-7618991F85B8_=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

+Matthew

For mapping_large_folio_support() changes.

On 6 Jun 2024, at 2:42, 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 the test cases are only for anonmous folios.
> while mapping_large_folio_support() is only reasonable for page
> cache folios.
>
> 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, s=
o
> (head[i].index >=3D end) is always false. shmem_mapping() is not called=
=2E
>
> Also add a VM_WARN_ON_ONCE() in mapping_large_folio_support()
> for anon mapping, So we can detect the wrong use more easily.
>
> THP folios maybe exist in the pagecache even the file system doesn't
> support large folio, it is because when CONFIG_TRANSPARENT_HUGEPAGE
> is enabled, khugepaged will try to collapse read-only file-backed pages=

> to THP. But the mapping does not actually support multi order
> large folios properly.
>
> Using /sys/kernel/debug/split_huge_pages to verify this, with this
> patch, large anon THP is successfully split and the warning is ceased.
>
> Signed-off-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> ---
>  include/linux/pagemap.h |  4 ++++
>  mm/huge_memory.c        | 27 ++++++++++++++++-----------
>  2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index ee633712bba0..59f1df0cde5a 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -381,6 +381,10 @@ static inline void mapping_set_large_folios(struct=
 address_space *mapping)
>   */
>  static inline bool mapping_large_folio_support(struct address_space *m=
apping)
>  {
> +	/* AS_LARGE_FOLIO_SUPPORT is only reasonable for pagecache folios */
> +	VM_WARN_ONCE((unsigned long)mapping & PAGE_MAPPING_ANON,
> +			"Anonymous mapping always supports large folio");
> +
>  	return IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
>  		test_bit(AS_LARGE_FOLIO_SUPPORT, &mapping->flags);
>  }
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 317de2afd371..62d57270b08e 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3009,30 +3009,35 @@ int split_huge_page_to_list_to_order(struct pag=
e *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))
> +	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 if (new_order) {
>  		/* 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)) {
> +		/* No split if the file system does not support large folio.
> +		 * Note that we might still have THPs in such mappings due to
> +		 * CONFIG_READ_ONLY_THP_FOR_FS. But in that case, the mapping
> +		 * does not actually support large folios properly.
> +		 */
> +		if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
> +			!mapping_large_folio_support(folio->mapping)) {

Shouldn=E2=80=99t this be

if (!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
	!mapping_large_folio_support(folio->mapping)) {

?

When CONFIG_READ_ONLY_THP_FOR_FS is not set, we need to check
mapping_large_folio_support(), otherwise we do not.

>  			VM_WARN_ONCE(1,
>  				"Cannot split file folio to non-0 order");
>  			return -EINVAL;
>  		}
>  	}
>
> +	/* Only swapping a whole PMD-mapped folio is supported */
> +	if (folio_test_swapcache(folio) && new_order)
> +		return -EINVAL;
>
>  	is_hzp =3D is_huge_zero_folio(folio);
>  	if (is_hzp) {
> -- =

> 2.15.2


Best Regards,
Yan, Zi

--=_MailMate_01928395-E6FC-4913-90CA-7618991F85B8_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

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

iQJDBAEBCgAtFiEEh7yFAW3gwjwQ4C9anbJR82th+ooFAmZhx48PHHppeUBudmlk
aWEuY29tAAoJEJ2yUfNrYfqKyJkP/ifVHxcOgwpM68Tdu9NdFppyJOXUxnfptvaU
mVDUfOGtnumYdazGt6E9d3rccO/smsLzmCHTvcwh5vUlR5UbM5idIbpf9EgzJK9r
iFW6MZXEMHJE+hYQWZdat024to5o+XuqwYbHqWEjvqB7CrYGA8pGhL9+T/2gzK54
o0a6QidYK+jm910SvpFAPV2HLsxOQaHoS28I4X0oZb4HpNRonvE+VQdshv4w9r4S
EOrQx4USsUSR1vao8D4/wmxPciJjkUPYNw0X/aTqALBSGXUEN23rIv3Z1vpPjtXf
MJrYxBS1vaBoatf734gkvjIkzDBOfKIEMK5MLrGWv0nQwEx/uSAWzf9X7NdITR1y
dSN0yStqzYNczS5vZWpBsyyeZyP82km0ZLDns8C/mQhTBtlx/a55DSVAXoC0Jy1p
h/TzcxjzzyzP5mZTn32zXjaVC11M3IcuYfj/BFayAK8NROEOeFaPBJtG5TIfcy96
GCAM5f3B6o5n2PxWK9kXTH9pZB9qyYjWx3i0ewfqlPATLxaFkR4WWTQkGYKMl03O
PZnCvyuSf2FWFCyuPhlpvdiwYTfNdaJD4k6acVuqEnY83PN/C6J/sBPFN0+rT6x+
uVz8R+wxm3eRGI/TGcuu6UsAs5qRnDR0CrxVdkj3K1dfYqoKQ9Xf6KIvB97ovihU
BSaZgwzz
=XUlL
-----END PGP SIGNATURE-----

--=_MailMate_01928395-E6FC-4913-90CA-7618991F85B8_=--