Received-SPF: pass (google.com: domain of linux-kernel+bounces-159452-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: Barry Song <21cnbao@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
 "Matthew Wilcox (Oracle)" <willy@infradead.org>,
 Yang Shi <shy828301@gmail.com>, Ryan Roberts <ryan.roberts@arm.com>,
 David Hildenbrand <david@redhat.com>, Lance Yang <ioworker0@gmail.com>,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] mm/rmap: do not add fully unmapped large folio to
 deferred split list
Date: Thu, 25 Apr 2024 22:50:20 -0400
Message-ID: <730660D2-E1BA-4A2E-B99C-2F160F9D9A9B@nvidia.com>
In-Reply-To: <CAGsJ_4xzb8RrEuPEbnvR4GbDWuoGCYL4FsC3TObOifAZ4CHGOA@mail.gmail.com>
References: <20240425211136.486184-1-zi.yan@sent.com>
 <CAGsJ_4wa0LskQkoZf9r5bG5+wEkyfCYveMBSTbuDe0=t1QetTg@mail.gmail.com>
 <6C31DF81-94FB-4D09-A3B8-0CED2AD8EDDB@nvidia.com>
 <CAGsJ_4xzb8RrEuPEbnvR4GbDWuoGCYL4FsC3TObOifAZ4CHGOA@mail.gmail.com>
Content-Type: multipart/signed;
 boundary="=_MailMate_AAC7DC85-3691-49B8-8AE2-2FB09CD15A3D_=";
 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?T3c5N2FkOHp2OC94bGZDNldBRm12ZG42UUVtdHJDWkZGLzU5cXp5K2s5SFdH?=
 =?utf-8?B?bVdZejNOUHh5ZzRDVzQ2Rlg2N2t3NFlGQTBoamxvdXcydklPNnFuZC9iZ2cz?=
 =?utf-8?B?T0VGaURpTndVK2ZkRGRreUlmRXNITnZSNk9VUVJvbUNwRnA3eUtIaEhiYWl5?=
 =?utf-8?B?UmM2c2x6dm5EL2dIV0RMdUhBRkNFRHBVYWxKamlsekNwU1c1cS9sbWNiM1JC?=
 =?utf-8?B?QTZLS2hHbnQ5WTNBd1JzWW9sR2dEbGZPVmFteU5BNjNaUDh6RGRqazRTdjhX?=
 =?utf-8?B?SzQ4NFNqcGRDSUFJV2F2Yi85cXhYU3ZDLzc1dGtJRG4yd2xQOGRwb05FaWxL?=
 =?utf-8?B?a2lDbVNTVTRKbW95bXoyVGYrZXdpVXFXNjlDbys0enY3NCtPWmxyTlB0SUpz?=
 =?utf-8?B?SUhYd2NTMnNGcTN2Qk9oL2dCc1p4UVNlY1VFNXh5eitzM0orSXRkLzVGWG9o?=
 =?utf-8?B?RjFiNzR6cUFzM3o4c093NGV4b1I3Tm0vcUduS1F5NVRtNG5kaTZ1Wjd0K005?=
 =?utf-8?B?YkFBY3ZmaTBVVGdKN2hPckZaWC9TNUdrODFibUVPbFZaWUJyU3hiM2xqcmtt?=
 =?utf-8?B?eVhTUDFXZXBzV0k5V0trU2s3aGF2YjNVR2J3bklWY0liemV0YnVDbFFWRUlR?=
 =?utf-8?B?WExyYWJabU1rK09uMHdLVTRaWEdGUE5uaWlld3RBNWN6SGs2bFk1bFlkYi9V?=
 =?utf-8?B?RG51ZSsxdHovWGtJbVpRZEQxQnNxWDg3WFloRVEzYlE4ekVvYVUzOW0wK0pJ?=
 =?utf-8?B?QXNJMTJFMzk0RmpER2hEcklDZ2ttbmxoWkNaV3V4TFFxNVJpM2FxMTJIQ1Rn?=
 =?utf-8?B?SGJVQnMvWGdvcFpxZURkRGd3MGpYUkZVN0RRb1FVQStTbkJUblBIc2MyMjhK?=
 =?utf-8?B?VUxwQnVSdHU4dGhHYytRdGt0cURSVmdXZWtVZlpvRHBVdUc2VEpEdTNZeVlE?=
 =?utf-8?B?OHZ1akRHV1BMdktER0FQMnZTeFVEQVZHQWR4eXEyRVg0a3NqSzhPb0p4VGhp?=
 =?utf-8?B?NU9NODk5c2xHMnhrWGp1TUs3VUdkK1FUOE5Fc2ZvQS9oRnBBS3FGZ3ZnNmZs?=
 =?utf-8?B?UkhGYlpLRitscElEQkhjbzFmU2dtb0tCcVkwWXVuc2QzbnMrOCtVMS82Vm9Y?=
 =?utf-8?B?QllCYTFCZVVpWm10L2tmMzB6SFgwM1ZrT0NnZEg0ekRpUUFqT2JmZmRTeDF1?=
 =?utf-8?B?M0NtYTdaVG5rK1EvQUFYY1prODNrYi9pdnova0MyYkRjejJNbW5qQ3dFVWlP?=
 =?utf-8?B?QXcyTVoya1N1Qmh5UmE4azBvNmczNHJWVnNobzVWZnRGVFU1bzgxMHZjd1g5?=
 =?utf-8?B?U25MR2laV2RlTVBTLzJqU1BoMzdjMkQ3NGJDWDZSTWxIeDN4b1JNSG55d2J6?=
 =?utf-8?B?d3JyczdFNTh0dm1SWmZKWEhLVDY3WWRpV3NTbkxFT2QweXFxTVoyTEVpYXcy?=
 =?utf-8?B?MTd1S0U4MG5ZTjFlZkQzUHc2UlEzbmQwUVJneWlaNTViLy94NndSUUNZUGp2?=
 =?utf-8?B?VVliZ3Jva0lQRmxuU2twTWF4YkoyMEpZQ0lFNTJjMXpjTnRsR09KWExUTXlH?=
 =?utf-8?B?VDh6Q2ZhTFlBNTFuMW1KNEF6NHRZT3dTOEhUeTdOMWJGZWU2Q0RSSGhlTHJB?=
 =?utf-8?B?YXJ5VUUrdkg4bmkwQStmcGdqWEVZdi9JemlKM0phR2tTcnBYNDUrWFI3U1Fr?=
 =?utf-8?B?eHhNUGh1dFNmRlYxNGhITFBuNjlqRW1ITHFiSFErSWx6ZXRpM2RtTVh4dGFS?=
 =?utf-8?B?VExiUEdlK2hpZW1sMDR1NUV1TUEvR25Bd00ySDcyN21TcG5jT3lOTVdiM1Vk?=
 =?utf-8?B?aXhOcXY2QkdLSXBNL29Zd0szM292VFhyN2xJaEhzbzBWQ0JZbmg2NTRNckJL?=
 =?utf-8?B?Y2Urd25VT1hlSlF6RUFWKzlHd0JCK2Rkd0Y4NncxS2tFQUhBMFl1d2xRNkEz?=
 =?utf-8?B?alg1YzZRaTJzRlBwNm4wbWE1cDNQdXRZTFg3NUdmUzIxWEtDMFIvbTBSUDRF?=
 =?utf-8?B?Y1lxbVU2bS9kUkxYc3FGMlZmTCs2WlJHSndPYlNreFphNkEvV1lxRWlSU2Ni?=
 =?utf-8?B?aTFLQS9vNmw0ZkxrQlpxOVFXTlpHVDJzN2NxY3EvMG01bG5NMjNncXhZWFdG?=
 =?utf-8?Q?DQG0=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ff74b5a7-b871-44be-a788-08dc659b9e79
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5744.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2024 02:50:23.4007
 (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: VQWqA3OLyf6+Xy+J6WDlTl1wtr9y3uDUdJSJ7xiLNeuFhusrKTa80iOS/W8Vdv0j
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB8535

--=_MailMate_AAC7DC85-3691-49B8-8AE2-2FB09CD15A3D_=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On 25 Apr 2024, at 22:23, Barry Song wrote:

> On Fri, Apr 26, 2024 at 9:55=E2=80=AFAM Zi Yan <ziy@nvidia.com> wrote:
>>
>> On 25 Apr 2024, at 21:45, Barry Song wrote:
>>
>>> On Fri, Apr 26, 2024 at 5:11=E2=80=AFAM Zi Yan <zi.yan@sent.com> wrot=
e:
>>>>
>>>> From: Zi Yan <ziy@nvidia.com>
>>>>
>>>> In __folio_remove_rmap(), a large folio is added to deferred split l=
ist
>>>> if any page in a folio loses its final mapping. But it is possible t=
hat
>>>> the folio is fully unmapped and adding it to deferred split list is
>>>> unnecessary.
>>>>
>>>> For PMD-mapped THPs, that was not really an issue, because removing =
the
>>>> last PMD mapping in the absence of PTE mappings would not have added=
 the
>>>> folio to the deferred split queue.
>>>>
>>>> However, for PTE-mapped THPs, which are now more prominent due to mT=
HP,
>>>> they are always added to the deferred split queue. One side effect
>>>> is that the THP_DEFERRED_SPLIT_PAGE stat for a PTE-mapped folio can =
be
>>>> unintentionally increased, making it look like there are many partia=
lly
>>>> mapped folios -- although the whole folio is fully unmapped stepwise=
=2E
>>>>
>>>> Core-mm now tries batch-unmapping consecutive PTEs of PTE-mapped THP=
s
>>>> where possible starting from commit b06dc281aa99 ("mm/rmap: introduc=
e
>>>> folio_remove_rmap_[pte|ptes|pmd]()"). When it happens, a whole PTE-m=
apped
>>>> folio is unmapped in one go and can avoid being added to deferred sp=
lit
>>>> list, reducing the THP_DEFERRED_SPLIT_PAGE noise. But there will sti=
ll be
>>>> noise when we cannot batch-unmap a complete PTE-mapped folio in one =
go
>>>> -- or where this type of batching is not implemented yet, e.g., migr=
ation.
>>>>
>>>> To avoid the unnecessary addition, folio->_nr_pages_mapped is checke=
d
>>>> to tell if the whole folio is unmapped. If the folio is already on
>>>> deferred split list, it will be skipped, too.
>>>>
>>>> Note: commit 98046944a159 ("mm: huge_memory: add the missing
>>>> folio_test_pmd_mappable() for THP split statistics") tried to exclud=
e
>>>> mTHP deferred split stats from THP_DEFERRED_SPLIT_PAGE, but it does =
not
>>>> fix the above issue. A fully unmapped PTE-mapped order-9 THP was sti=
ll
>>>> added to deferred split list and counted as THP_DEFERRED_SPLIT_PAGE,=

>>>> since nr is 512 (non zero), level is RMAP_LEVEL_PTE, and inside
>>>> deferred_split_folio() the order-9 folio is folio_test_pmd_mappable(=
).
>>>>
>>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>>> Reviewed-by: Yang Shi <shy828301@gmail.com>
>>>> ---
>>>>  mm/rmap.c | 8 +++++---
>>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>>> index a7913a454028..220ad8a83589 100644
>>>> --- a/mm/rmap.c
>>>> +++ b/mm/rmap.c
>>>> @@ -1553,9 +1553,11 @@ static __always_inline void __folio_remove_rm=
ap(struct folio *folio,
>>>>                  * page of the folio is unmapped and at least one pa=
ge
>>>>                  * is still mapped.
>>>>                  */
>>>> -               if (folio_test_large(folio) && folio_test_anon(folio=
))
>>>> -                       if (level =3D=3D RMAP_LEVEL_PTE || nr < nr_p=
mdmapped)
>>>> -                               deferred_split_folio(folio);
>>>> +               if (folio_test_large(folio) && folio_test_anon(folio=
) &&
>>>> +                   list_empty(&folio->_deferred_list) &&
>>>> +                   ((level =3D=3D RMAP_LEVEL_PTE && atomic_read(map=
ped)) ||
>>>> +                    (level =3D=3D RMAP_LEVEL_PMD && nr < nr_pmdmapp=
ed)))
>>>> +                       deferred_split_folio(folio);
>>>
>>> Hi Zi Yan,
>>> in case a mTHP is mapped by two processed (forked but not CoW yet), i=
f we
>>> unmap the whole folio by pte level in one process only, are we still =
adding this
>>> folio into deferred list?
>>
>> No. Because the mTHP is still fully mapped by the other process. In te=
rms of code,
>> nr will be 0 in that case and this if condition is skipped. nr is only=
 increased
>> from 0 when one of the subpages in the mTHP has no mapping, namely pag=
e->_mapcount
>> becomes negative and last is true in the case RMAP_LEVEL_PTE.
>
> Ok. i see, so "last" won't be true?
>
> case RMAP_LEVEL_PTE:
> do {
> last =3D atomic_add_negative(-1, &page->_mapcount);
>    if (last && folio_test_large(folio)) {
>        last =3D atomic_dec_return_relaxed(mapped);
>        last =3D (last < ENTIRELY_MAPPED);
> }
>
> if (last)
>      nr++;
> } while (page++, --nr_pages > 0);
> break;

Right, because for every subpage its corresponding
last =3D atomic_add_negative(-1, &page->_mapcount); is not true after the=
 unmapping.


--
Best Regards,
Yan, Zi

--=_MailMate_AAC7DC85-3691-49B8-8AE2-2FB09CD15A3D_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

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

iQJDBAEBCgAtFiEE6rR4j8RuQ2XmaZol4n+egRQHKFQFAmYrFmwPHHppeUBudmlk
aWEuY29tAAoJEOJ/noEUByhUm4UQAJ0aWBln5/8UqROLl4+DY5ruik7J6kqv5U4e
sRutUToZX07er2ou/uaYXzq7fb0jsvrqOZuA0StrKsE3YeKQ9DyILAoPEv97f3j6
QkNkpMzmgdJ69LGeohjBnIP5WOE7wyE+dwmMgNtITd9W6GDj5U5g//EKVs8ufvuG
27yq672HvNzFIEVVsTD+vjT9nVenyymzg93NW/MaU4esmHeur5DNvF63qavuGr5D
RI6MmFZSeASfDf76rsUEJrYYUpdzyCmlNIhuynDOe1+WV2RZXDtT8cvoDTqp2X8l
J1VYYABTL5u/TbQOgOiU6daua5pLtX2fZ1SkrM+U4WDVvzrsErTtmUDjzZGEEiS3
x1f05ODyBNoMWhW9ov3S2GmjPqFqLQrMhcJ5w4GE+SmdtrAkG2s7lt/+tL1NHh/R
ZJAYjiMfxZI+Xa4e0TqBIGJ3PyOLLMjZAEGdcHpPDqNj6TDFD04t3vsFLUGTMKNv
VqIPMxb3tgjRN/CnwkcsKvZFMrBeT0Uxh5IfxreVO09gcThoFrvIdT9WprJq5zi5
tELq1NtbU23fXA2tljYhtDRD4IN41E53pQJ2aW0x2TslWzfiqZIJn1LMQ12E0A8j
8B0BEkVd84E0dbSUaw/j02jwWSJ+U2kMtcpjV9gdigorFN82hKvMgL9ai10xXw0X
oLAXNonZ
=457H
-----END PGP SIGNATURE-----

--=_MailMate_AAC7DC85-3691-49B8-8AE2-2FB09CD15A3D_=--