Message-ID: <2586B1FF-91EF-41C2-AD98-7EF699716CAB@nvidia.com>
Date: Thu, 25 Apr 2024 11:16:33 -0400
From: Zi Yan <ziy@...dia.com>
To: David Hildenbrand <david@...hat.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>,
 Yang Shi <shy828301@...il.com>, Ryan Roberts <ryan.roberts@....com>,
 Barry Song <21cnbao@...il.com>, Lance Yang <ioworker0@...il.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] mm/rmap: do not add fully unmapped large folio to
 deferred split list

On 25 Apr 2024, at 11:15, David Hildenbrand wrote:

> On 25.04.24 16:53, Zi Yan wrote:
>> On 25 Apr 2024, at 3:19, David Hildenbrand wrote:
>>
>>> On 25.04.24 00:46, Zi Yan wrote:
>>>> From: Zi Yan <ziy@...dia.com>
>>>>
>>>> In __folio_remove_rmap(), a large folio is added to deferred split list
>>>> if any page in a folio loses its final mapping. It is possible that
>>>> the folio is unmapped fully, but it is unnecessary to add the folio
>>>> to deferred split list at all. Fix it by checking folio->_nr_pages_mapped
>>>> before adding a folio to deferred split list. If the folio is already
>>>> on the deferred split list, it will be skipped. This issue applies to
>>>> both PTE-mapped THP and mTHP.
>>>>
>>>> Commit 98046944a159 ("mm: huge_memory: add the missing
>>>> folio_test_pmd_mappable() for THP split statistics") tried to exclude
>>>> 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 still
>>>
>>> Once again: your patch won't fix it either.
>>>
>>>> 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().
>>>> However, this miscount was present even earlier due to implementation,
>>>> since PTEs are unmapped individually and first PTE unmapping adds the THP
>>>> into the deferred split list.
>>>
>>> It will still be present. Just less frequently.
>>
>> OK. Let me reread the email exchanges between you and Yang and clarify
>> the details in the commit log.
>
> Likely something like:
>
> --
> In __folio_remove_rmap(), a large folio is added to deferred split list
> if any page in a folio loses its final mapping. But, it is possible that
> the folio is now fully unmapped and adding it to the 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 mTHP, we will always end up adding them to the deferred split queue.
>
> One side effect of this is that we will frequently increase the THP_DEFERRED_SPLIT_PAGE stat for PTE-mapped THP, making it look like we frequently get many partially mapped folios -- although we are simply
> unmapping the whole thing stepwise.
>
> Core-mm will now try batch-unmapping consecutive PTEs of PTE-mapped THPs where possible. If we're lucky, we unmap the whole thing in one go and can avoid adding the folio to the deferred split queue, reducing the THP_DEFERRED_SPLIT_PAGE noise.
>
> But there will still 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.
> --
>
> Feel free to reuse what you consider reasonable.

Sure. Thank you a lot for drafting it!

--
Best Regards,
Yan, Zi

Download attachment "signature.asc" of type "application/pgp-signature" (855 bytes)

Powered by blists - more mailing lists