linux-kernel - Re: [v6 02/15] mm/huge_memory: add device-private THP support to PMD operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3uxfirnfvki4kztyf3vac5vph6kzhnf623li46gmaqux5ivf5e@3uvtpfeymgug>
Date: Fri, 26 Sep 2025 11:53:29 +1000
From: Alistair Popple <apopple@...dia.com>
To: David Hildenbrand <david@...hat.com>
Cc: Balbir Singh <balbirs@...dia.com>, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, damon@...ts.linux.dev, dri-devel@...ts.freedesktop.org, 
	Matthew Brost <matthew.brost@...el.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>, 
	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>, Francois Dugast <francois.dugast@...el.com>
Subject: Re: [v6 02/15] mm/huge_memory: add device-private THP support to PMD
 operations

On 2025-09-25 at 19:53 +1000, David Hildenbrand <david@...hat.com> wrote...
> On 25.09.25 02:25, Alistair Popple wrote:
> > On 2025-09-16 at 22:21 +1000, Balbir Singh <balbirs@...dia.com> wrote...
> > > Extend core huge page management functions to handle device-private THP
> > > entries.  This enables proper handling of large device-private folios in
> > > fundamental MM operations.
> > > 
> > > The following functions have been updated:
> > > 
> > > - copy_huge_pmd(): Handle device-private entries during fork/clone
> > > - zap_huge_pmd(): Properly free device-private THP during munmap
> > > - change_huge_pmd(): Support protection changes on device-private THP
> > > - __pte_offset_map(): Add device-private entry awareness
> > > 
> > > Signed-off-by: Matthew Brost <matthew.brost@...el.com>
> > > Signed-off-by: Balbir Singh <balbirs@...dia.com>
> > > 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>
> > > ---
> > >   include/linux/swapops.h | 32 +++++++++++++++++++++++
> > >   mm/huge_memory.c        | 56 ++++++++++++++++++++++++++++++++++-------
> > >   mm/pgtable-generic.c    |  2 +-
> > >   3 files changed, 80 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> > > index 64ea151a7ae3..2687928a8146 100644
> > > --- a/include/linux/swapops.h
> > > +++ b/include/linux/swapops.h
> > > @@ -594,10 +594,42 @@ static inline int is_pmd_migration_entry(pmd_t pmd)
> > >   }
> > >   #endif  /* CONFIG_ARCH_ENABLE_THP_MIGRATION */
> > > +#if defined(CONFIG_ZONE_DEVICE) && defined(CONFIG_ARCH_ENABLE_THP_MIGRATION)
> > > +
> > > +/**
> > > + * is_pmd_device_private_entry() - Check if PMD contains a device private swap entry
> > > + * @pmd: The PMD to check
> > > + *
> > > + * Returns true if the PMD contains a swap entry that represents a device private
> > > + * page mapping. This is used for zone device private pages that have been
> > > + * swapped out but still need special handling during various memory management
> > > + * operations.
> > > + *
> > > + * Return: 1 if PMD contains device private entry, 0 otherwise
> > > + */
> > > +static inline int is_pmd_device_private_entry(pmd_t pmd)
> > > +{
> > > +	return is_swap_pmd(pmd) && is_device_private_entry(pmd_to_swp_entry(pmd));
> > > +}
> > > +
> > > +#else /* CONFIG_ZONE_DEVICE && CONFIG_ARCH_ENABLE_THP_MIGRATION */
> > > +
> > > +static inline int is_pmd_device_private_entry(pmd_t pmd)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +#endif /* CONFIG_ZONE_DEVICE && CONFIG_ARCH_ENABLE_THP_MIGRATION */
> > > +
> > >   static inline int non_swap_entry(swp_entry_t entry)
> > >   {
> > >   	return swp_type(entry) >= MAX_SWAPFILES;
> > >   }
> > > +static inline int is_pmd_non_present_folio_entry(pmd_t pmd)
> > 
> > I can't think of a better name either although I am curious why open-coding it
> > was so nasty given we don't have the equivalent for pte entries. Will go read
> > the previous discussion.
> 
> I think for PTEs we just handle all cases (markers, hwpoison etc) properly,
> manye not being supported yet on the PMD level. See copy_nonpresent_pte() as
> an example.
> 
> We don't even have helpers like is_pte_migration_entry().
> 
> > > diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
> > > index 567e2d084071..0c847cdf4fd3 100644
> > > --- a/mm/pgtable-generic.c
> > > +++ b/mm/pgtable-generic.c
> > > @@ -290,7 +290,7 @@ pte_t *___pte_offset_map(pmd_t *pmd, unsigned long addr, pmd_t *pmdvalp)
> > >   	if (pmdvalp)
> > >   		*pmdvalp = pmdval;
> > > -	if (unlikely(pmd_none(pmdval) || is_pmd_migration_entry(pmdval)))
> > > +	if (unlikely(pmd_none(pmdval) || !pmd_present(pmdval)))
> > 
> > Why isn't is_pmd_non_present_folio_entry() used here?
> 
> 
> I thought I argued that
> 
> 	if (!pmd_present(pmdval)))
> 
> Should be sufficient here in my last review?

My bad, I'm a bit behind catching up on the last review comments. But agree it's
sufficient, was just curious why it wasn't used so will go read your previous
comments! Thanks.

> We want to detect page tables we can map after all.
> -- 
> Cheers
> 
> David / dhildenb
>