Received-SPF: pass (google.com: domain of linux-kernel+bounces-119233-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Zi Yan <ziy@nvidia.com>
To: Baolin Wang <baolin.wang@linux.alibaba.com>,
 Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, Matthew Wilcox <willy@infradead.org>,
 Yang Shi <shy828301@gmail.com>, Huang Ying <ying.huang@intel.com>,
 "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
 Ryan Roberts <ryan.roberts@arm.com>, "Yin, Fengwei" <fengwei.yin@intel.com>,
 SeongJae Park <sj@kernel.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5] mm/migrate: split source folio if it is on deferred
 split list
Date: Tue, 26 Mar 2024 10:53:56 -0400
Message-ID: <6A1CCFDF-CC38-4B29-A514-3D500EA04B16@nvidia.com>
In-Reply-To: <df9a644c-a007-46ac-98e3-61d4014fcfff@linux.alibaba.com>
References: <20240322193304.522496-1-zi.yan@sent.com>
 <1ecde325-097b-49e9-9f1f-d93edbc8e1f1@linux.alibaba.com>
 <8DAC0D9E-9B0D-4035-9211-2190C6F8284A@nvidia.com>
 <df9a644c-a007-46ac-98e3-61d4014fcfff@linux.alibaba.com>
Content-Type: multipart/signed;
 boundary="=_MailMate_A52ACB3A-425E-491A-AD70-D4B583EC8054_=";
 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?p2JioGjuZesKy/q6g/P09kj26x/Xt0EOUN4OqdGl92MsUhN0BJeGtdUbasc0?=
 =?us-ascii?Q?Jr+Hg43MeodGWh8A+P+d0JEsy6hH8O2BAbf5E03YWNEeXTRz21gG1ZQSWGPT?=
 =?us-ascii?Q?TszGiN+FE+0FiAA2vkhVgbczjs1G7nmDGGTW/m/0Ke4jtAvAfz+H/p15OEDL?=
 =?us-ascii?Q?Mpt7DN1dFUW5eHrazDT886SObP9tZMLv8qe0VDG+H8XaaWYMOFXsJWvkLxxd?=
 =?us-ascii?Q?WdNuZezeDVPtq2m3plzAMCjm/EmcnwZjvU0EEdKOJ+fLrlVdXDTflHZJKxRd?=
 =?us-ascii?Q?YJWxtG2hxLLnt4WH2pmx5JGKQV8zbsqSkdZXTQIDQmvs6H/FhPk+r2XWndWP?=
 =?us-ascii?Q?c2wYxoJnhiWtseyVGy3dPAUCINSQx27+S/QkX8ekL0QhaKUrpYbniQT5HUzt?=
 =?us-ascii?Q?6uULTuPp8sEa9hDzj5mPx3svNUdPmyYcHkajoKSyG1LkpH9zPv5c7KLxRLPM?=
 =?us-ascii?Q?uVLQscOaIRDbIKUc+PHPtIpizS1CRqQDYboIM2GVSGbv9+jtgJ8GzER1gvjN?=
 =?us-ascii?Q?HvPrrHlMCTNkEe0ca7uPL56Gul+GoFAXPevQHCtPpS41ImCrNt+8sXZwGTWf?=
 =?us-ascii?Q?UXniruCX0xDMEiVemI3jvSv7H3PhSEhPmTljWCF6OA+oUrVdfPO0v8kc6Uam?=
 =?us-ascii?Q?5tXPMU1JLouo99HBgHYKpPZwlmFPCLzx64jHeEYTw/DHWSnl4J1PgjktOVft?=
 =?us-ascii?Q?6SavJg3HXsTdgKv3bQ6TUKK9rMOIm3AReEjPkL2yZZfMbym7NPhC8Ig8NIaV?=
 =?us-ascii?Q?gba62nT7Uxd2RkaAMzxpD50IbPBPQDWOEJjzHRXsB+UWvtLhTrPXYihZO11+?=
 =?us-ascii?Q?QIs2DiThCatPt0YRR5laLl94spKKIdS31606yyeo6K2m9VjCGTEdF1U3Noa0?=
 =?us-ascii?Q?eCJOujcdGfjTYN0lMZffjG+F3To9aswFRsLPpwDFtp4hW4NJ+uqphCvMEjWT?=
 =?us-ascii?Q?7D7h36f4oMycH9sFuJLfpcyPQMKDfGTtPnK0RGoxCIofLyKC5BkeJQm9h/Zo?=
 =?us-ascii?Q?A9UR20j1AHQkFPkfpTt3mjZ+JXh4BSzCaeN/2Sio2tURVh8jOiaf2U4TnlhU?=
 =?us-ascii?Q?SKnQePxzWPCeA93OvkOTomTtnyQKtdeXxT2c0RlAlwBB3XH4JEtIU4KxtR5T?=
 =?us-ascii?Q?ADrnueAIgwbk6z0AGbPWG5xAHuOZJ7Vlo/NBgDvWCLhedqpubb+Yp0+86RM3?=
 =?us-ascii?Q?PgNA70LEEde9hdaxPhu1nL3Vgs7ZnxbHes8NouCaKU74qKIfZsZ1MQt6XfLY?=
 =?us-ascii?Q?Aqdsua/1hy/ctmtvARIHn5vmS7Ji74JRDKjwwhDputozz40cETdmzx4eVbW/?=
 =?us-ascii?Q?hU6mo9DNcRMfddA8prFhABGe4kiFDhsSnq21SgsBG1g7PdM+6wyjQYtoWi3d?=
 =?us-ascii?Q?b20RMKZ3R9DOlxHY3DPqJf2pZlE1u1grfYoDKLHnIysA42+w6ghcZ7lLstop?=
 =?us-ascii?Q?80XeRg5MLf+g2IWvnQZLQe6euniqSbYRLEZR6OYEueNUx0XXdomU662J2Gpp?=
 =?us-ascii?Q?cQQKxRx+nCUccQRQPK2pUNMiwVaJlTqtcX1gD7HvqDEwr68pXxehbc7NN2h6?=
 =?us-ascii?Q?+gDjvspRl8yvFf44UJ0=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 157f8477-24f1-4788-21e2-08dc4da49168
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5744.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Mar 2024 14:53:59.0636
 (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: Eus8FgN6rz/dlwUAxxmgGk2tYun/m6lLziCU//Wnkf4zGcL8wVn8HJthBQkKX/8V
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB8124

--=_MailMate_A52ACB3A-425E-491A-AD70-D4B583EC8054_=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On 26 Mar 2024, at 10:42, Baolin Wang wrote:

> On 2024/3/26 21:26, Zi Yan wrote:
>> On 26 Mar 2024, at 2:19, Baolin Wang wrote:
>>
>>> On 2024/3/23 03:33, Zi Yan wrote:
>>>> From: Zi Yan <ziy@nvidia.com>
>>>>
>>>> If the source folio is on deferred split list, it is likely some sub=
pages
>>>> are not used. Split it before migration to avoid migrating unused su=
bpages.
>>>>
>>>> Commit 616b8371539a6 ("mm: thp: enable thp migration in generic path=
")
>>>> did not check if a THP is on deferred split list before migration, t=
hus,
>>>> the destination THP is never put on deferred split list even if the =
source
>>>> THP might be. The opportunity of reclaiming free pages in a partiall=
y
>>>> mapped THP during deferred list scanning is lost, but no other harmf=
ul
>>>> consequence is present[1].
>>>>
>>>>   From v4:
>>>> 1. Simplify _deferred_list check without locking and do not count as=

>>>>      migration failures. (per Matthew Wilcox)
>>>>
>>>>   From v3:
>>>> 1. Guarded deferred list code behind CONFIG_TRANSPARENT_HUGEPAGE to =
avoid
>>>>      compilation error (per SeongJae Park).
>>>>
>>>>   From v2:
>>>> 1. Split the source folio instead of migrating it (per Matthew Wilco=
x)[2].
>>>>
>>>>   From v1:
>>>> 1. Used dst to get correct deferred split list after migration
>>>>      (per Ryan Roberts).
>>>>
>>>> [1]: https://lore.kernel.org/linux-mm/03CE3A00-917C-48CC-8E1C-6A9871=
3C817C@nvidia.com/
>>>> [2]: https://lore.kernel.org/linux-mm/Ze_P6xagdTbcu1Kz@casper.infrad=
ead.org/
>>>>
>>>> Fixes: 616b8371539a ("mm: thp: enable thp migration in generic path"=
)
>>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>>> ---
>>>>    mm/migrate.c | 23 +++++++++++++++++++++++
>>>>    1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/mm/migrate.c b/mm/migrate.c
>>>> index ab9856f5931b..6bd9319624a3 100644
>>>> --- a/mm/migrate.c
>>>> +++ b/mm/migrate.c
>>>> @@ -1652,6 +1652,29 @@ static int migrate_pages_batch(struct list_he=
ad *from,
>>>>     			cond_resched();
>>>>   +			/*
>>>> +			 * The rare folio on the deferred split list should
>>>> +			 * be split now. It should not count as a failure.
>>>> +			 * Only check it without removing it from the list.
>>>> +			 * Since the folio can be on deferred_split_scan()
>>>> +			 * local list and removing it can cause the local list
>>>> +			 * corruption. Folio split process below can handle it
>>>> +			 * with the help of folio_ref_freeze().
>>>> +			 *
>>>> +			 * nr_pages > 2 is needed to avoid checking order-1
>>>> +			 * page cache folios. They exist, in contrast to
>>>> +			 * non-existent order-1 anonymous folios, and do not
>>>> +			 * use _deferred_list.
>>>> +			 */
>>>> +			if (nr_pages > 2 &&
>>>> +			   !list_empty(&folio->_deferred_list)) {
>>>> +				if (try_split_folio(folio, from) =3D=3D 0) {
>>>
>>> IMO, we should move the split folios into the 'split_folios' list ins=
tead of the 'from' list, otherwise there might be unhandled folios remain=
ing in the from list.
>>
>> Can you elaborate on the actual situation you are thinking about? Than=
ks.
>
> Sure.
>
> Suppose there is only one large folio in the from list that needs to be=
 migrated, and this large folio is in the _deferred_list, which means it =
needs to be split. Your patch will re-add the split base pages back into =
the 'from' list. However, please see the list_for_each_entry_safe macro:
>
> #define list_for_each_entry_safe(pos, n, head, member)			\
> 	for (pos =3D list_first_entry(head, typeof(*pos), member),	\
> 		n =3D list_next_entry(pos, member);			\
> 	     !list_entry_is_head(pos, head, member); 			\
> 	     pos =3D n, n =3D list_next_entry(n, member))
>
> It will terminate the iteration early because the next entry 'n' taken =
out in advance is already the head, leading to the remaining split base p=
ages still in the from list. This can cause the following crash when I di=
d some migration testing:
>
> [  412.576943] ------------[ cut here ]------------
> [  412.576947] kernel BUG at mm/migrate.c:2634!
> [  412.577132] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> [  412.577201] CPU: 59 PID: 9581 Comm: numa01 Kdump: loaded Tainted: G =
         E      6.9.0-rc1+ #69
> ........
> [  412.578651] Call Trace:
> [  412.578692]  <TASK>
> [  412.578730]  ? die+0x33/0x90
> [  412.578770]  ? do_trap+0xdf/0x110
> [  412.578815]  ? migrate_misplaced_folio+0x1f2/0x2b0
> [  412.578875]  ? do_error_trap+0x65/0x80
> [  412.578922]  ? migrate_misplaced_folio+0x1f2/0x2b0
> [  412.578977]  ? exc_invalid_op+0x4e/0x70
> [  412.579048]  ? migrate_misplaced_folio+0x1f2/0x2b0
> [  412.579131]  ? asm_exc_invalid_op+0x16/0x20
> [  412.579182]  ? migrate_misplaced_folio+0x1f2/0x2b0
> [  412.579255]  do_numa_page+0x205/0x5b0
> [  412.579305]  __handle_mm_fault+0x2b0/0x6c0
> [  412.579354]  handle_mm_fault+0x105/0x270
> [  412.579404]  do_user_addr_fault+0x214/0x6b0
> [  412.579453]  exc_page_fault+0x64/0x140
> [  412.579509]  asm_exc_page_fault+0x22/0x30
>
> 2583 int migrate_misplaced_folio(struct folio *folio, struct vm_area_st=
ruct *vma,
> 2584                             int node)
> 2585 {
> 		......
>
> 2628         if (nr_succeeded) {
> 2629                 count_vm_numa_events(NUMA_PAGE_MIGRATE, nr_succeed=
ed);
> 2630                 if (!node_is_toptier(folio_nid(folio)) && node_is_=
toptier(node))
> 2631                         mod_node_page_state(pgdat, PGPROMOTE_SUCCE=
SS,
> 2632                                             nr_succeeded);
> 2633         }
> 2634         BUG_ON(!list_empty(&migratepages));
> 2635         return isolated;
> 2636
> 2637 out:

Got it. Thanks.

>
> After changing as below, the system crash issue is gone.
>
> +++ b/mm/migrate.c
> @@ -1668,7 +1668,7 @@ static int migrate_pages_batch(struct list_head *=
from,
>                          */
>                         if (nr_pages > 2 &&
>                            !list_empty(&folio->_deferred_list)) {
> -                               if (try_split_folio(folio, from) =3D=3D=
 0) {
> +                               if (try_split_folio(folio, split_folios=
) =3D=3D 0) {
>                                         stats->nr_thp_split +=3D is_thp=
;
>                                         stats->nr_split++;
>                                         continue;

Let me resend with this fix.

--
Best Regards,
Yan, Zi

--=_MailMate_A52ACB3A-425E-491A-AD70-D4B583EC8054_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

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

iQJDBAEBCgAtFiEE6rR4j8RuQ2XmaZol4n+egRQHKFQFAmYC4YUPHHppeUBudmlk
aWEuY29tAAoJEOJ/noEUByhUVyEP/R3cXGdRZ2F/P+DInKj6ZeIsi22HdBt52fBM
D99R0g4DTKt/7zvZFl/gaqX0IUOBu3eoLH5MsLDfjKgxTdfSrPy4dkZg+lsVpKMg
EyZGBGchgOLlWetrauPGypFwHTBizjvMt6q4tXWW71NQGtOhCl32g10ZOCQWARyC
5ZA7kmYoEYCMGFOlL6gL7o7A3qpDyGzg6O3E+BI9Kgq21LEPZjejgk1nNzF5Jef5
5Uh7DMDVMnW0ql6xYz2h2u/bFj7NwMJARostktl9v1/2AC84Z/z6UlfSAfSxvLX+
dIwNRROMcs3a7EKQUMZ8ncJbC6MBPqK10uXnv20QDLq+9OrR0t2FoSI/M2uM/pPj
c25bqwbIzqbzbTs0ChJEK0CL+ycD5gDXwSKTkItNUrNd1571EBK4+tmZ2AJ31Y6w
pWyanFfFj5WPyT++M51rxYqIqHQSo1Zcv09hBqCyppPxzgKcieX/u5JQEDJDVhMU
h89ApHpwix5ebWc/jQxUS85D2FeEAG+2y+FFXs9kINaSFhtHsmzbG7zv6nib3sjE
ROz9TWPWif/0NH4k1PoIVkaRfxtf399jCEBExpxYNX0mTimV7wMnMs4K3xuK1Gy1
3LNbqBpIwtKa1DizMiMIskq4zZkBoMd/VYY6CnCXt9ARNHYtPZOqpbn1J9NhOsDQ
DIqWH0Df
=Vo/E
-----END PGP SIGNATURE-----

--=_MailMate_A52ACB3A-425E-491A-AD70-D4B583EC8054_=--