Re: [PATCH 05/12] fs/proc/task_mmu: write-protect device-private THPs in PAGEMAP_SCAN

[Lorenzo Stoakes (ARM)]

On Wed, Nov 11, 2026 at 12:55:13PM -0500, Tal Zussman wrote:
> PAGEMAP_SCAN with PM_SCAN_WP_MATCHING write-protects a huge PMD through
> make_uffd_wp_pmd(), which handles a present THP and a migration entry

It's crazy to me that we write-protect page tables through a procfs interface
like this. But I guess we have to live with it now.

I suppose that answers my q about this on a previous commit...

> but does nothing for a device-private THP. The PMD never gets the uffd
> bit.

Sigh yeah. Device-private feels like another 'just have to happen to know to
update X million open-coded parts of the code'.

We really need to find a way to abstract this stuff.

>
> pagemap_thp_category() reports a non-present PMD without the uffd bit as
> written. A user tracking writes with async uffd-wp therefore sees every
> page of a device-private THP as written on every scan, whether it was
> written to or not. This requires a driver that migrates THPs to device
> memory.
>
> A device-private PMD must be write-protected like any other huge PMD. To
> fix it, pass it to set_uffd_wp_swp_pmd() as well, which downgrades a
> writable entry and sets the uffd bit. When the THP migrates back,
> set_pmd_migration_entry() carries the uffd bit over to the migration
> entry, so the write protection is kept.
>
> Fixes: a30b48bf1b24 ("mm/migrate_device: implement THP migration of zone device pages")
> Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
> ---
>  fs/proc/task_mmu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 53780a454432..752f1b13d47a 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -2524,7 +2524,8 @@ static void make_uffd_wp_pmd(struct vm_area_struct *vma,
>  		old = pmdp_invalidate_ad(vma, addr, pmdp);
>  		pmd = pmd_mkuffd(old);
>  		set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
> -	} else if (pmd_is_migration_entry(pmd)) {
> +	} else if (pmd_is_migration_entry(pmd) ||
> +		   pmd_is_device_private_entry(pmd)) {

Could this not just be !pmd_none() + cover all softleaf entries? Or is that not
valid?

It seems completely insane to have to specifically spell out all the
possibilities, open-coded, everywhere.

>  		pmd = set_uffd_wp_swp_pmd(pmd);
>  		set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
>  	}
>
> --
> 2.39.5
>

--
Cheers, Lorenzo