[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c2d563b2-da9e-4e3b-989a-97f7eb828d6a@nvidia.com>
Date: Fri, 12 Sep 2025 14:51:53 +1000
From: Balbir Singh <balbirs@...dia.com>
To: SeongJae Park <sj@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
dri-devel@...ts.freedesktop.org, Andrew Morton <akpm@...ux-foundation.org>,
David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>,
Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
Byungchul Park <byungchul@...com>, Gregory Price <gourry@...rry.net>,
Ying Huang <ying.huang@...ux.alibaba.com>,
Alistair Popple <apopple@...dia.com>, Oscar Salvador <osalvador@...e.de>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>, Nico Pache <npache@...hat.com>,
Ryan Roberts <ryan.roberts@....com>, Dev Jain <dev.jain@....com>,
Barry Song <baohua@...nel.org>, Lyude Paul <lyude@...hat.com>,
Danilo Krummrich <dakr@...nel.org>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Ralph Campbell <rcampbell@...dia.com>,
Mika Penttilä <mpenttil@...hat.com>,
Matthew Brost <matthew.brost@...el.com>,
Francois Dugast <francois.dugast@...el.com>
Subject: Re: [v5 03/15] mm/rmap: extend rmap and migration support
device-private entries
On 9/12/25 11:59, SeongJae Park wrote:
> On Mon, 8 Sep 2025 10:04:36 +1000 Balbir Singh <balbirs@...dia.com> wrote:
>
>> Add device-private THP support to reverse mapping infrastructure,
>> enabling proper handling during migration and walk operations.
>>
>> The key changes are:
>> - add_migration_pmd()/remove_migration_pmd(): Handle device-private
>> entries during folio migration and splitting
>> - page_vma_mapped_walk(): Recognize device-private THP entries during
>> VMA traversal operations
>>
>> This change supports folio splitting and migration operations on
>> device-private entries.
>>
>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>> Cc: David Hildenbrand <david@...hat.com>
>> Cc: Zi Yan <ziy@...dia.com>
>> Cc: Joshua Hahn <joshua.hahnjy@...il.com>
>> Cc: Rakie Kim <rakie.kim@...com>
>> Cc: Byungchul Park <byungchul@...com>
>> Cc: Gregory Price <gourry@...rry.net>
>> Cc: Ying Huang <ying.huang@...ux.alibaba.com>
>> Cc: Alistair Popple <apopple@...dia.com>
>> Cc: Oscar Salvador <osalvador@...e.de>
>> Cc: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
>> Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>
>> Cc: "Liam R. Howlett" <Liam.Howlett@...cle.com>
>> Cc: Nico Pache <npache@...hat.com>
>> Cc: Ryan Roberts <ryan.roberts@....com>
>> Cc: Dev Jain <dev.jain@....com>
>> Cc: Barry Song <baohua@...nel.org>
>> 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: Ralph Campbell <rcampbell@...dia.com>
>> Cc: Mika Penttilä <mpenttil@...hat.com>
>> Cc: Matthew Brost <matthew.brost@...el.com>
>> Cc: Francois Dugast <francois.dugast@...el.com>
>>
>> Signed-off-by: Balbir Singh <balbirs@...dia.com>
>> ---
>> mm/damon/ops-common.c | 20 +++++++++++++++++---
>> mm/huge_memory.c | 16 +++++++++++++++-
>> mm/page_idle.c | 5 +++--
>> mm/page_vma_mapped.c | 12 ++++++++++--
>> mm/rmap.c | 19 ++++++++++++++++---
>> 5 files changed, 61 insertions(+), 11 deletions(-)
>>
>> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
>> index 998c5180a603..eda4de553611 100644
>> --- a/mm/damon/ops-common.c
>> +++ b/mm/damon/ops-common.c
>> @@ -75,12 +75,24 @@ void damon_ptep_mkold(pte_t *pte, struct vm_area_struct *vma, unsigned long addr
>> void damon_pmdp_mkold(pmd_t *pmd, struct vm_area_struct *vma, unsigned long addr)
>> {
>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> - struct folio *folio = damon_get_folio(pmd_pfn(pmdp_get(pmd)));
>> + pmd_t pmdval = pmdp_get(pmd);
>> + struct folio *folio;
>> + bool young = false;
>> + unsigned long pfn;
>> +
>> + if (likely(pmd_present(pmdval)))
>> + pfn = pmd_pfn(pmdval);
>> + else
>> + pfn = swp_offset_pfn(pmd_to_swp_entry(pmdval));
>>
>> + folio = damon_get_folio(pfn);
>> if (!folio)
>> return;
>>
>> - if (pmdp_clear_young_notify(vma, addr, pmd))
>> + if (likely(pmd_present(pmdval)))
>> + young |= pmdp_clear_young_notify(vma, addr, pmd);
>> + young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE);
>> + if (young)
>> folio_set_young(folio);
>>
>> folio_set_idle(folio);
>> @@ -203,7 +215,9 @@ static bool damon_folio_young_one(struct folio *folio,
>> mmu_notifier_test_young(vma->vm_mm, addr);
>> } else {
>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> - *accessed = pmd_young(pmdp_get(pvmw.pmd)) ||
>> + pmd_t pmd = pmdp_get(pvmw.pmd);
>> +
>> + *accessed = (pmd_present(pmd) && pmd_young(pmd)) ||
>> !folio_test_idle(folio) ||
>> mmu_notifier_test_young(vma->vm_mm, addr);
>
> Could you please elaborate more about why the above change is needed on the
> commit message?
>
> For example, I found below from v3 of this patch series:
>
> 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.
>
> Adding that kind of elaboration on the commit message would be helpful.
>
> Also, seems the DAMON part change is not really required to be made together
> with other changes. If I'm not wrong, could you make DAMON part change as a
> separate commit?
>
> The code looks good to me.
>
> Reviewed-by: SeongJae Park <sj@...nel.org>
>
>
Thanks SJ!
v3 had a separate flag during page vma mapped walk, so the walkers had to opt-in
with a flag, I removed that after David's review feedback
This change is required because we now support large folios in device private
and hence the DAMON changes are included in this patch
Balbir
Powered by blists - more mailing lists