Message-ID: <F60614E1-28D1-466B-A434-7FF1061B0778@nvidia.com>
Date: Wed, 30 Jul 2025 07:30:40 -0400
From: Zi Yan <ziy@...dia.com>
To: Mika Penttilä <mpenttil@...hat.com>
Cc: Balbir Singh <balbirs@...dia.com>, <linux-mm@...ck.org>,
 <linux-kernel@...r.kernel.org>, Karol Herbst <kherbst@...hat.com>,
 Lyude Paul <lyude@...hat.com>, Danilo Krummrich <dakr@...nel.org>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Jérôme Glisse <jglisse@...hat.com>,
 Shuah Khan <shuah@...nel.org>, David Hildenbrand <david@...hat.com>,
 Barry Song <baohua@...nel.org>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Ryan Roberts <ryan.roberts@....com>, Matthew Wilcox <willy@...radead.org>,
 Peter Xu <peterx@...hat.com>, Kefeng Wang <wangkefeng.wang@...wei.com>,
 Jane Chu <jane.chu@...cle.com>, Alistair Popple <apopple@...dia.com>,
 Donet Tom <donettom@...ux.ibm.com>, Matthew Brost <matthew.brost@...el.com>,
 Francois Dugast <francois.dugast@...el.com>,
 Ralph Campbell <rcampbell@...dia.com>
Subject: Re: [v2 02/11] mm/thp: zone_device awareness in THP handling code

On 30 Jul 2025, at 7:27, Zi Yan wrote:

> On 30 Jul 2025, at 7:16, Mika Penttilä wrote:
>
>> Hi,
>>
>> On 7/30/25 12:21, Balbir Singh wrote:
>>> Make THP handling code in the mm subsystem for THP pages aware of zone
>>> device pages. Although the code is designed to be generic when it comes
>>> to handling splitting of pages, the code is designed to work for THP
>>> page sizes corresponding to HPAGE_PMD_NR.
>>>
>>> Modify page_vma_mapped_walk() to return true when a zone device huge
>>> entry is present, enabling try_to_migrate() and other code migration
>>> paths to appropriately process the entry. page_vma_mapped_walk() will
>>> return true for zone device private large folios only when
>>> PVMW_THP_DEVICE_PRIVATE is passed. This is to prevent locations that are
>>> not zone device private pages from having to add awareness. The key
>>> callback that needs this flag is try_to_migrate_one(). The other
>>> callbacks page idle, damon use it for setting young/dirty bits, which is
>>> not significant when it comes to pmd level bit harvesting.
>>>
>>> pmd_pfn() does not work well with zone device entries, use
>>> pfn_pmd_entry_to_swap() for checking and comparison as for zone device
>>> entries.
>>>
>>> Zone device private entries when split via munmap go through pmd split,
>>> but need to go through a folio split, deferred split does not work if a
>>> fault is encountered because fault handling involves migration entries
>>> (via folio_migrate_mapping) and the folio sizes are expected to be the
>>> same there. This introduces the need to split the folio while handling
>>> the pmd split. Because the folio is still mapped, but calling
>>> folio_split() will cause lock recursion, the __split_unmapped_folio()
>>> code is used with a new helper to wrap the code
>>> split_device_private_folio(), which skips the checks around
>>> folio->mapping, swapcache and the need to go through unmap and remap
>>> folio.
>>>
>>> Cc: Karol Herbst <kherbst@...hat.com>
>>> Cc: Lyude Paul <lyude@...hat.com>
>>> Cc: Danilo Krummrich <dakr@...nel.org>
>>> Cc: David Airlie <airlied@...il.com>
>>> Cc: Simona Vetter <simona@...ll.ch>
>>> Cc: "Jérôme Glisse" <jglisse@...hat.com>
>>> Cc: Shuah Khan <shuah@...nel.org>
>>> Cc: David Hildenbrand <david@...hat.com>
>>> Cc: Barry Song <baohua@...nel.org>
>>> Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>
>>> Cc: Ryan Roberts <ryan.roberts@....com>
>>> Cc: Matthew Wilcox <willy@...radead.org>
>>> Cc: Peter Xu <peterx@...hat.com>
>>> Cc: Zi Yan <ziy@...dia.com>
>>> Cc: Kefeng Wang <wangkefeng.wang@...wei.com>
>>> Cc: Jane Chu <jane.chu@...cle.com>
>>> Cc: Alistair Popple <apopple@...dia.com>
>>> Cc: Donet Tom <donettom@...ux.ibm.com>
>>> Cc: Mika Penttilä <mpenttil@...hat.com>
>>> Cc: Matthew Brost <matthew.brost@...el.com>
>>> Cc: Francois Dugast <francois.dugast@...el.com>
>>> Cc: Ralph Campbell <rcampbell@...dia.com>
>>>
>>> Signed-off-by: Matthew Brost <matthew.brost@...el.com>
>>> Signed-off-by: Balbir Singh <balbirs@...dia.com>
>>> ---
>>>  include/linux/huge_mm.h |   1 +
>>>  include/linux/rmap.h    |   2 +
>>>  include/linux/swapops.h |  17 +++
>>>  mm/huge_memory.c        | 268 +++++++++++++++++++++++++++++++++-------
>>>  mm/page_vma_mapped.c    |  13 +-
>>>  mm/pgtable-generic.c    |   6 +
>>>  mm/rmap.c               |  22 +++-
>>>  7 files changed, 278 insertions(+), 51 deletions(-)
>>>
>
> <snip>
>
>>> +/**
>>> + * split_huge_device_private_folio - split a huge device private folio into
>>> + * smaller pages (of order 0), currently used by migrate_device logic to
>>> + * split folios for pages that are partially mapped
>>> + *
>>> + * @folio: the folio to split
>>> + *
>>> + * The caller has to hold the folio_lock and a reference via folio_get
>>> + */
>>> +int split_device_private_folio(struct folio *folio)
>>> +{
>>> +	struct folio *end_folio = folio_next(folio);
>>> +	struct folio *new_folio;
>>> +	int ret = 0;
>>> +
>>> +	/*
>>> +	 * Split the folio now. In the case of device
>>> +	 * private pages, this path is executed when
>>> +	 * the pmd is split and since freeze is not true
>>> +	 * it is likely the folio will be deferred_split.
>>> +	 *
>>> +	 * With device private pages, deferred splits of
>>> +	 * folios should be handled here to prevent partial
>>> +	 * unmaps from causing issues later on in migration
>>> +	 * and fault handling flows.
>>> +	 */
>>> +	folio_ref_freeze(folio, 1 + folio_expected_ref_count(folio));
>>
>> Why can't this freeze fail? The folio is still mapped afaics, why can't there be other references in addition to the caller?
>
> Based on my off-list conversation with Balbir, the folio is unmapped in
> CPU side but mapped in the device. folio_ref_freeeze() is not aware of
> device side mapping.

Maybe we should make it aware of device private mapping? So that the
process mirrors CPU side folio split: 1) unmap device private mapping,
2) freeze device private folio, 3) split unmapped folio, 4) unfreeze,
5) remap device private mapping.

>
>>
>>> +	ret = __split_unmapped_folio(folio, 0, &folio->page, NULL, NULL, true);
>>
>> Confusing to  __split_unmapped_folio() if folio is mapped...
>
> From driver point of view, __split_unmapped_folio() probably should be renamed
> to __split_cpu_unmapped_folio(), since it is only dealing with CPU side
> folio meta data for split.
>
>
> Best Regards,
> Yan, Zi


Best Regards,
Yan, Zi

Powered by blists - more mailing lists