Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Lorenzo Stoakes]

+cc Lance, linux-kernel

Your subject line is 83 characters long and is way too detailed how about 'fix
device-private PMD handling'?

You forgot to include linux-kernel@vger.kernel.org on the mail, lore seems to be
a bit broken atm but in general it's helpful to include that.

Also is useful to make this [PATCH mm-hotfixes] to make it really clear it's
intended as a hotfix.

Some commit msg language nits:

On Mon, Jun 22, 2026 at 01:06:51PM +0000, Wei Yang wrote:
> For pmd_trans_huge() and pmd_is_migration_entry(), we does following
> before return the pmd entry:

Sounds better as:

	For PMD entries that satisfy pmd_trans_huge() or pmd_is_migration_entry(), we
	perform the following actions:

>
>   * re-validate pmd entry after PTL
>   * check PVMW_MIGRATION
>   * check_pmd()
>   * handle on pte level if split under us
>
> But for device-private pmd, we just return after pmd_lock().

->

	However, for device-private PMD entries, we simply acquire the PMD lock
	and return.

Also can you please give some justification here as to why all this also applies
to device-private PMD? Right now it sounds hand wavey.

>
> If a softleaf entry is present, e.g. device-private pmd, the existing
> code simply acquires the PMD lock and returns success even if
> PVMW_MIGRATION is set (indicating a migration entry is sought), meaning
> that the caller can incorrectly interpret the entry as something it is
> not, causing data corruption.

This is repetitive, you already mentioned device-private PMD, you already
mentioned that it simply acquires the PMD lock.

You should talk about what issue it caused and why:

	This is particularly problematic when PVMW_MIGRATION is set (meaning a
	migration entry is sought), as it causes a device-private PMD entry to
	be returned with a different data layout, causing memory corruption.

>
> This patch fixes commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration
> support device-private entries") by following the same pattern as
> pmd_trans_huge() and pmd_is_migration_entry() for device private entry.

This is pretty useless. We see what patch it fixes in the Fixes tag, and you're
just repeating things you said above, I'd drop it.

>
> Fixes: 65edfda6f3f2 ("mm/rmap: extend rmap and migration support device-private entries")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Suggested-by: David Hildenbrand <david@kernel.org>
> Cc: David Hildenbrand <david@kernel.org>
> Cc: Balbir Singh <balbirs@nvidia.com>
> Cc: SeongJae Park <sj@kernel.org>
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Lorenzo Stoakes <ljs@kernel.org>
>
> ---
> v3:
>   * remove cleanup part, only fix the issue for device-private entry
>   * refine user effect description based on Lorenzo's suggestion
> v2: https://lore.kernel.org/all/20260616063436.20455-1-richard.weiyang@gmail.com/T/#u
>   * specify the possible error case of current code and user visible effect
>   * besides fix, cleanup the pmd entry handling based on David's suggestion
>
> v1: https://lore.kernel.org/linux-mm/20260508013728.21285-1-richard.weiyang@gmail.com/
> ---
>  mm/page_vma_mapped.c | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index 2ccbabfb2cc1..8de3c6b82df6 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -270,21 +270,33 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>  			spin_unlock(pvmw->ptl);
>  			pvmw->ptl = NULL;
>  		} else if (!pmd_present(pmde)) {
> -			const softleaf_t entry = softleaf_from_pmd(pmde);
> +			softleaf_t entry = softleaf_from_pmd(pmde);
>
>  			if (softleaf_is_device_private(entry)) {
>  				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
> -				return true;
> -			}
>
> -			if ((pvmw->flags & PVMW_SYNC) &&
> -			    thp_vma_suitable_order(vma, pvmw->address,
> -						   PMD_ORDER) &&
> -			    (pvmw->nr_pages >= HPAGE_PMD_NR))
> -				sync_with_folio_pmd_zap(mm, pvmw->pmd);
> +				entry = softleaf_from_pmd(*pvmw->pmd);
>
> -			step_forward(pvmw, PMD_SIZE);
> -			continue;
> +				if (softleaf_is_device_private(entry)) {

This is all very horrible. You have an example of how pmde is re-got in the
pmd_trans_huge() branch and pmd_is_device_private_entry() exists...

We can just make this another branch and do the re-check more neatly.

I enclose a patch that does that (untested, please check).


> +					if (pvmw->flags & PVMW_MIGRATION)
> +						return not_found(pvmw);
> +					if (!check_pmd(softleaf_to_pfn(entry), pvmw))
> +						return not_found(pvmw);
> +					return true;
> +				}
> +				/* device-private pmd was split under us: handle on pte level */
> +				spin_unlock(pvmw->ptl);
> +				pvmw->ptl = NULL;
> +			} else {
> +				if ((pvmw->flags & PVMW_SYNC) &&
> +				    thp_vma_suitable_order(vma, pvmw->address,
> +							   PMD_ORDER) &&
> +				    (pvmw->nr_pages >= HPAGE_PMD_NR))
> +					sync_with_folio_pmd_zap(mm, pvmw->pmd);
> +
> +				step_forward(pvmw, PMD_SIZE);
> +				continue;
> +			}
>  		}
>  		if (!map_pte(pvmw, &pmde, &ptl)) {
>  			if (!pvmw->pte)
> --
> 2.34.1
>

Thanks, Lorenzo

----8<----
From e6a3c1c782714ed831c4d46a14bb99226423bf59 Mon Sep 17 00:00:00 2001
From: Wei Yang <richard.weiyang@gmail.com>
Date: Mon, 22 Jun 2026 13:06:51 +0000
Subject: [PATCH] refactored

Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
---
 mm/page_vma_mapped.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index 2ccbabfb2cc1..17dff8aab9f9 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -269,14 +269,24 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			/* THP pmd was split under us: handle on pte level */
 			spin_unlock(pvmw->ptl);
 			pvmw->ptl = NULL;
-		} else if (!pmd_present(pmde)) {
-			const softleaf_t entry = softleaf_from_pmd(pmde);
+		} else if (pmd_is_device_private_entry(pmde)) {
+			softleaf_t entry;
+
+			pvmw->ptl = pmd_lock(mm, pvmw->pmd);
+			pmde = *pvmw->pmd;
+			entry = softleaf_from_pmd(pmde);

-			if (softleaf_is_device_private(entry)) {
-				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
+			if (likely(softleaf_is_device_private(entry))) {
+				if (pvmw->flags & PVMW_MIGRATION)
+					return not_found(pvmw);
+				if (!check_pmd(softleaf_to_pfn(entry), pvmw))
+					return not_found(pvmw);
 				return true;
 			}
-
+			/* device-private pmd was split under us: handle on pte level */
+			spin_unlock(pvmw->ptl);
+			pvmw->ptl = NULL;
+		} else if (!pmd_present(pmde)) {
 			if ((pvmw->flags & PVMW_SYNC) &&
 			    thp_vma_suitable_order(vma, pvmw->address,
 						   PMD_ORDER) &&
--
2.54.0

Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Wei Yang]

On Mon, Jun 22, 2026 at 02:46:40PM +0100, Lorenzo Stoakes wrote:
>+cc Lance, linux-kernel
>
>Your subject line is 83 characters long and is way too detailed how about 'fix
>device-private PMD handling'?
>

Got it.

>You forgot to include linux-kernel@vger.kernel.org on the mail, lore seems to be
>a bit broken atm but in general it's helpful to include that.

Got it.

So usually we send a patch to both linux-mm and linux-kernel? If so, I
remember is later actions.

>
>Also is useful to make this [PATCH mm-hotfixes] to make it really clear it's
>intended as a hotfix.
>

Got it.

>Some commit msg language nits:
>
>On Mon, Jun 22, 2026 at 01:06:51PM +0000, Wei Yang wrote:
>> For pmd_trans_huge() and pmd_is_migration_entry(), we does following
>> before return the pmd entry:
>
>Sounds better as:
>
>	For PMD entries that satisfy pmd_trans_huge() or pmd_is_migration_entry(), we
>	perform the following actions:
>

Sure.

>>
>>   * re-validate pmd entry after PTL
>>   * check PVMW_MIGRATION
>>   * check_pmd()
>>   * handle on pte level if split under us
>>
>> But for device-private pmd, we just return after pmd_lock().
>
>->
>
>	However, for device-private PMD entries, we simply acquire the PMD lock
>	and return.
>

Sure.

>Also can you please give some justification here as to why all this also applies
>to device-private PMD? Right now it sounds hand wavey.
>

I thought below paragraph explain it. Not sure what justification is preferred.

>> If a softleaf entry is present, e.g. device-private pmd, the existing
>> code simply acquires the PMD lock and returns success even if
>> PVMW_MIGRATION is set (indicating a migration entry is sought), meaning
>> that the caller can incorrectly interpret the entry as something it is
>> not, causing data corruption.
>
>This is repetitive, you already mentioned device-private PMD, you already
>mentioned that it simply acquires the PMD lock.
>

Ah, I copied your suggestion from [1]. Hope I don't misunderstand it.

[1]: https://lore.kernel.org/linux-mm/ajUXNjRMraKb6k2n@lucifer/

>You should talk about what issue it caused and why:
>
>	This is particularly problematic when PVMW_MIGRATION is set (meaning a
>	migration entry is sought), as it causes a device-private PMD entry to
>	be returned with a different data layout, causing memory corruption.
>

This looks good. I would take this one, if you prefer.

>>
>> This patch fixes commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration
>> support device-private entries") by following the same pattern as
>> pmd_trans_huge() and pmd_is_migration_entry() for device private entry.
>
>This is pretty useless. We see what patch it fixes in the Fixes tag, and you're
>just repeating things you said above, I'd drop it.
>

Got it.

>> Fixes: 65edfda6f3f2 ("mm/rmap: extend rmap and migration support device-private entries")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Suggested-by: David Hildenbrand <david@kernel.org>
>> Cc: David Hildenbrand <david@kernel.org>
>> Cc: Balbir Singh <balbirs@nvidia.com>
>> Cc: SeongJae Park <sj@kernel.org>
>> Cc: Zi Yan <ziy@nvidia.com>
>> Cc: Lorenzo Stoakes <ljs@kernel.org>
>>
>> ---
>> v3:
>>   * remove cleanup part, only fix the issue for device-private entry
>>   * refine user effect description based on Lorenzo's suggestion
>> v2: https://lore.kernel.org/all/20260616063436.20455-1-richard.weiyang@gmail.com/T/#u
>>   * specify the possible error case of current code and user visible effect
>>   * besides fix, cleanup the pmd entry handling based on David's suggestion
>>
>> v1: https://lore.kernel.org/linux-mm/20260508013728.21285-1-richard.weiyang@gmail.com/
>> ---
>>  mm/page_vma_mapped.c | 32 ++++++++++++++++++++++----------
>>  1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> index 2ccbabfb2cc1..8de3c6b82df6 100644
>> --- a/mm/page_vma_mapped.c
>> +++ b/mm/page_vma_mapped.c
>> @@ -270,21 +270,33 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>>  			spin_unlock(pvmw->ptl);
>>  			pvmw->ptl = NULL;
>>  		} else if (!pmd_present(pmde)) {
>> -			const softleaf_t entry = softleaf_from_pmd(pmde);
>> +			softleaf_t entry = softleaf_from_pmd(pmde);
>>
>>  			if (softleaf_is_device_private(entry)) {
>>  				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
>> -				return true;
>> -			}
>>
>> -			if ((pvmw->flags & PVMW_SYNC) &&
>> -			    thp_vma_suitable_order(vma, pvmw->address,
>> -						   PMD_ORDER) &&
>> -			    (pvmw->nr_pages >= HPAGE_PMD_NR))
>> -				sync_with_folio_pmd_zap(mm, pvmw->pmd);
>> +				entry = softleaf_from_pmd(*pvmw->pmd);
>>
>> -			step_forward(pvmw, PMD_SIZE);
>> -			continue;
>> +				if (softleaf_is_device_private(entry)) {
>
>This is all very horrible. You have an example of how pmde is re-got in the
>pmd_trans_huge() branch and pmd_is_device_private_entry() exists...
>
>We can just make this another branch and do the re-check more neatly.
>

I plan to keep the change small, but yeah it is ugly.

>I enclose a patch that does that (untested, please check).
>
>
>> +					if (pvmw->flags & PVMW_MIGRATION)
>> +						return not_found(pvmw);
>> +					if (!check_pmd(softleaf_to_pfn(entry), pvmw))
>> +						return not_found(pvmw);
>> +					return true;
>> +				}
>> +				/* device-private pmd was split under us: handle on pte level */
>> +				spin_unlock(pvmw->ptl);
>> +				pvmw->ptl = NULL;
>> +			} else {
>> +				if ((pvmw->flags & PVMW_SYNC) &&
>> +				    thp_vma_suitable_order(vma, pvmw->address,
>> +							   PMD_ORDER) &&
>> +				    (pvmw->nr_pages >= HPAGE_PMD_NR))
>> +					sync_with_folio_pmd_zap(mm, pvmw->pmd);
>> +
>> +				step_forward(pvmw, PMD_SIZE);
>> +				continue;
>> +			}
>>  		}
>>  		if (!map_pte(pvmw, &pmde, &ptl)) {
>>  			if (!pvmw->pte)
>> --
>> 2.34.1
>>
>
>Thanks, Lorenzo
>
>----8<----
>>From e6a3c1c782714ed831c4d46a14bb99226423bf59 Mon Sep 17 00:00:00 2001
>From: Wei Yang <richard.weiyang@gmail.com>
>Date: Mon, 22 Jun 2026 13:06:51 +0000
>Subject: [PATCH] refactored
>
>Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>---
> mm/page_vma_mapped.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
>diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>index 2ccbabfb2cc1..17dff8aab9f9 100644
>--- a/mm/page_vma_mapped.c
>+++ b/mm/page_vma_mapped.c
>@@ -269,14 +269,24 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
> 			/* THP pmd was split under us: handle on pte level */
> 			spin_unlock(pvmw->ptl);
> 			pvmw->ptl = NULL;
>-		} else if (!pmd_present(pmde)) {
>-			const softleaf_t entry = softleaf_from_pmd(pmde);
>+		} else if (pmd_is_device_private_entry(pmde)) {
>+			softleaf_t entry;
>+
>+			pvmw->ptl = pmd_lock(mm, pvmw->pmd);
>+			pmde = *pvmw->pmd;
>+			entry = softleaf_from_pmd(pmde);
>
>-			if (softleaf_is_device_private(entry)) {
>-				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
>+			if (likely(softleaf_is_device_private(entry))) {
>+				if (pvmw->flags & PVMW_MIGRATION)
>+					return not_found(pvmw);
>+				if (!check_pmd(softleaf_to_pfn(entry), pvmw))
>+					return not_found(pvmw);
> 				return true;
> 			}
>-
>+			/* device-private pmd was split under us: handle on pte level */
>+			spin_unlock(pvmw->ptl);
>+			pvmw->ptl = NULL;
>+		} else if (!pmd_present(pmde)) {
> 			if ((pvmw->flags & PVMW_SYNC) &&
> 			    thp_vma_suitable_order(vma, pvmw->address,
> 						   PMD_ORDER) &&
>--
>2.54.0

If we prefer this way, I will check and take it.

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Lance Yang]

On 2026/6/22 22:21, Wei Yang wrote:
> On Mon, Jun 22, 2026 at 02:46:40PM +0100, Lorenzo Stoakes wrote:
>> +cc Lance, linux-kernel
>>
>> Your subject line is 83 characters long and is way too detailed how about 'fix
>> device-private PMD handling'?
>>
> 
> Got it.
> 
>> You forgot to include linux-kernel@vger.kernel.org on the mail, lore seems to be
>> a bit broken atm but in general it's helpful to include that.
> 
> Got it.
> 
> So usually we send a patch to both linux-mm and linux-kernel? If so, I
> remember is later actions.

Yeah, please keep linux-kernel copied too. For MM patches, linux-mm +
linux-kernel is the right default, IMHO :)

>>
>> Also is useful to make this [PATCH mm-hotfixes] to make it really clear it's
>> intended as a hotfix.
>>
> 
> Got it.
> 
[...]
>> ----8<----
>> >From e6a3c1c782714ed831c4d46a14bb99226423bf59 Mon Sep 17 00:00:00 2001
>> From: Wei Yang <richard.weiyang@gmail.com>
>> Date: Mon, 22 Jun 2026 13:06:51 +0000
>> Subject: [PATCH] refactored
>>
>> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>> ---
>> mm/page_vma_mapped.c | 20 +++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> index 2ccbabfb2cc1..17dff8aab9f9 100644
>> --- a/mm/page_vma_mapped.c
>> +++ b/mm/page_vma_mapped.c
>> @@ -269,14 +269,24 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>> 			/* THP pmd was split under us: handle on pte level */
>> 			spin_unlock(pvmw->ptl);
>> 			pvmw->ptl = NULL;
>> -		} else if (!pmd_present(pmde)) {
>> -			const softleaf_t entry = softleaf_from_pmd(pmde);
>> +		} else if (pmd_is_device_private_entry(pmde)) {
>> +			softleaf_t entry;
>> +
>> +			pvmw->ptl = pmd_lock(mm, pvmw->pmd);
>> +			pmde = *pvmw->pmd;
>> +			entry = softleaf_from_pmd(pmde);
>>
>> -			if (softleaf_is_device_private(entry)) {
>> -				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
>> +			if (likely(softleaf_is_device_private(entry))) {
>> +				if (pvmw->flags & PVMW_MIGRATION)
>> +					return not_found(pvmw);
>> +				if (!check_pmd(softleaf_to_pfn(entry), pvmw))
>> +					return not_found(pvmw);
>> 				return true;
>> 			}
>> -
>> +			/* device-private pmd was split under us: handle on pte level */
>> +			spin_unlock(pvmw->ptl);
>> +			pvmw->ptl = NULL;
>> +		} else if (!pmd_present(pmde)) {
>> 			if ((pvmw->flags & PVMW_SYNC) &&
>> 			    thp_vma_suitable_order(vma, pvmw->address,
>> 						   PMD_ORDER) &&
>> --
>> 2.54.0
> 
> If we prefer this way, I will check and take it.

And +1 on Lorenzo's diff. Much cleaner.

Cheers, Lance

Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Lorenzo Stoakes]

On Mon, Jun 22, 2026 at 02:21:02PM +0000, Wei Yang wrote:
> On Mon, Jun 22, 2026 at 02:46:40PM +0100, Lorenzo Stoakes wrote:
> >+cc Lance, linux-kernel
> >
> >Your subject line is 83 characters long and is way too detailed how about 'fix
> >device-private PMD handling'?
> >
>
> Got it.
>
> >You forgot to include linux-kernel@vger.kernel.org on the mail, lore seems to be
> >a bit broken atm but in general it's helpful to include that.
>
> Got it.
>
> So usually we send a patch to both linux-mm and linux-kernel? If so, I
> remember is later actions.

Yeah it's better for dealing with kvack going wrong etc. :)

>
> >
> >Also is useful to make this [PATCH mm-hotfixes] to make it really clear it's
> >intended as a hotfix.
> >
>
> Got it.
>
> >Some commit msg language nits:
> >
> >On Mon, Jun 22, 2026 at 01:06:51PM +0000, Wei Yang wrote:
> >> For pmd_trans_huge() and pmd_is_migration_entry(), we does following
> >> before return the pmd entry:
> >
> >Sounds better as:
> >
> >	For PMD entries that satisfy pmd_trans_huge() or pmd_is_migration_entry(), we
> >	perform the following actions:
> >
>
> Sure.
>
> >>
> >>   * re-validate pmd entry after PTL
> >>   * check PVMW_MIGRATION
> >>   * check_pmd()
> >>   * handle on pte level if split under us
> >>
> >> But for device-private pmd, we just return after pmd_lock().
> >
> >->
> >
> >	However, for device-private PMD entries, we simply acquire the PMD lock
> >	and return.
> >
>
> Sure.
>
> >Also can you please give some justification here as to why all this also applies
> >to device-private PMD? Right now it sounds hand wavey.
> >
>
> I thought below paragraph explain it. Not sure what justification is preferred.

Something about device private PMDs splitting the same way THP ones do, in the
pmd_is_device_private_entry() branch of __split_huge_pmd_locked().

>
> >> If a softleaf entry is present, e.g. device-private pmd, the existing
> >> code simply acquires the PMD lock and returns success even if
> >> PVMW_MIGRATION is set (indicating a migration entry is sought), meaning
> >> that the caller can incorrectly interpret the entry as something it is
> >> not, causing data corruption.
> >
> >This is repetitive, you already mentioned device-private PMD, you already
> >mentioned that it simply acquires the PMD lock.
> >
>
> Ah, I copied your suggestion from [1]. Hope I don't misunderstand it.
>
> [1]: https://lore.kernel.org/linux-mm/ajUXNjRMraKb6k2n@lucifer/

Sure, thanks but in context with the above ends up being a bit repetitive.

>
> >You should talk about what issue it caused and why:
> >
> >	This is particularly problematic when PVMW_MIGRATION is set (meaning a
> >	migration entry is sought), as it causes a device-private PMD entry to
> >	be returned with a different data layout, causing memory corruption.
> >
>
> This looks good. I would take this one, if you prefer.

Sure, thanks!

>
> >>
> >> This patch fixes commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration
> >> support device-private entries") by following the same pattern as
> >> pmd_trans_huge() and pmd_is_migration_entry() for device private entry.
> >
> >This is pretty useless. We see what patch it fixes in the Fixes tag, and you're
> >just repeating things you said above, I'd drop it.
> >
>
> Got it.
>
> >> Fixes: 65edfda6f3f2 ("mm/rmap: extend rmap and migration support device-private entries")
> >> Cc: <stable@vger.kernel.org>
> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> >> Suggested-by: David Hildenbrand <david@kernel.org>
> >> Cc: David Hildenbrand <david@kernel.org>
> >> Cc: Balbir Singh <balbirs@nvidia.com>
> >> Cc: SeongJae Park <sj@kernel.org>
> >> Cc: Zi Yan <ziy@nvidia.com>
> >> Cc: Lorenzo Stoakes <ljs@kernel.org>
> >>
> >> ---
> >> v3:
> >>   * remove cleanup part, only fix the issue for device-private entry
> >>   * refine user effect description based on Lorenzo's suggestion
> >> v2: https://lore.kernel.org/all/20260616063436.20455-1-richard.weiyang@gmail.com/T/#u
> >>   * specify the possible error case of current code and user visible effect
> >>   * besides fix, cleanup the pmd entry handling based on David's suggestion
> >>
> >> v1: https://lore.kernel.org/linux-mm/20260508013728.21285-1-richard.weiyang@gmail.com/
> >> ---
> >>  mm/page_vma_mapped.c | 32 ++++++++++++++++++++++----------
> >>  1 file changed, 22 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> >> index 2ccbabfb2cc1..8de3c6b82df6 100644
> >> --- a/mm/page_vma_mapped.c
> >> +++ b/mm/page_vma_mapped.c
> >> @@ -270,21 +270,33 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
> >>  			spin_unlock(pvmw->ptl);
> >>  			pvmw->ptl = NULL;
> >>  		} else if (!pmd_present(pmde)) {
> >> -			const softleaf_t entry = softleaf_from_pmd(pmde);
> >> +			softleaf_t entry = softleaf_from_pmd(pmde);
> >>
> >>  			if (softleaf_is_device_private(entry)) {
> >>  				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
> >> -				return true;
> >> -			}
> >>
> >> -			if ((pvmw->flags & PVMW_SYNC) &&
> >> -			    thp_vma_suitable_order(vma, pvmw->address,
> >> -						   PMD_ORDER) &&
> >> -			    (pvmw->nr_pages >= HPAGE_PMD_NR))
> >> -				sync_with_folio_pmd_zap(mm, pvmw->pmd);
> >> +				entry = softleaf_from_pmd(*pvmw->pmd);
> >>
> >> -			step_forward(pvmw, PMD_SIZE);
> >> -			continue;
> >> +				if (softleaf_is_device_private(entry)) {
> >
> >This is all very horrible. You have an example of how pmde is re-got in the
> >pmd_trans_huge() branch and pmd_is_device_private_entry() exists...
> >
> >We can just make this another branch and do the re-check more neatly.
> >
>
> I plan to keep the change small, but yeah it is ugly.
>
> >I enclose a patch that does that (untested, please check).
> >
> >
> >> +					if (pvmw->flags & PVMW_MIGRATION)
> >> +						return not_found(pvmw);
> >> +					if (!check_pmd(softleaf_to_pfn(entry), pvmw))
> >> +						return not_found(pvmw);
> >> +					return true;
> >> +				}
> >> +				/* device-private pmd was split under us: handle on pte level */
> >> +				spin_unlock(pvmw->ptl);
> >> +				pvmw->ptl = NULL;
> >> +			} else {
> >> +				if ((pvmw->flags & PVMW_SYNC) &&
> >> +				    thp_vma_suitable_order(vma, pvmw->address,
> >> +							   PMD_ORDER) &&
> >> +				    (pvmw->nr_pages >= HPAGE_PMD_NR))
> >> +					sync_with_folio_pmd_zap(mm, pvmw->pmd);
> >> +
> >> +				step_forward(pvmw, PMD_SIZE);
> >> +				continue;
> >> +			}
> >>  		}
> >>  		if (!map_pte(pvmw, &pmde, &ptl)) {
> >>  			if (!pvmw->pte)
> >> --
> >> 2.34.1
> >>
> >
> >Thanks, Lorenzo
> >
> >----8<----
> >>From e6a3c1c782714ed831c4d46a14bb99226423bf59 Mon Sep 17 00:00:00 2001
> >From: Wei Yang <richard.weiyang@gmail.com>
> >Date: Mon, 22 Jun 2026 13:06:51 +0000
> >Subject: [PATCH] refactored
> >
> >Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> >---
> > mm/page_vma_mapped.c | 20 +++++++++++++++-----
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> >
> >diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> >index 2ccbabfb2cc1..17dff8aab9f9 100644
> >--- a/mm/page_vma_mapped.c
> >+++ b/mm/page_vma_mapped.c
> >@@ -269,14 +269,24 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
> > 			/* THP pmd was split under us: handle on pte level */
> > 			spin_unlock(pvmw->ptl);
> > 			pvmw->ptl = NULL;
> >-		} else if (!pmd_present(pmde)) {
> >-			const softleaf_t entry = softleaf_from_pmd(pmde);
> >+		} else if (pmd_is_device_private_entry(pmde)) {
> >+			softleaf_t entry;
> >+
> >+			pvmw->ptl = pmd_lock(mm, pvmw->pmd);
> >+			pmde = *pvmw->pmd;
> >+			entry = softleaf_from_pmd(pmde);
> >
> >-			if (softleaf_is_device_private(entry)) {
> >-				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
> >+			if (likely(softleaf_is_device_private(entry))) {
> >+				if (pvmw->flags & PVMW_MIGRATION)
> >+					return not_found(pvmw);
> >+				if (!check_pmd(softleaf_to_pfn(entry), pvmw))
> >+					return not_found(pvmw);
> > 				return true;
> > 			}
> >-
> >+			/* device-private pmd was split under us: handle on pte level */
> >+			spin_unlock(pvmw->ptl);
> >+			pvmw->ptl = NULL;
> >+		} else if (!pmd_present(pmde)) {
> > 			if ((pvmw->flags & PVMW_SYNC) &&
> > 			    thp_vma_suitable_order(vma, pvmw->address,
> > 						   PMD_ORDER) &&
> >--
> >2.54.0
>
> If we prefer this way, I will check and take it.

Yeah, feel free to go ahead + use it :)

>
> --
> Wei Yang
> Help you, Help me

Thanks, Lorenzo

Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Wei Yang]

On Mon, Jun 22, 2026 at 05:11:02PM +0100, Lorenzo Stoakes wrote:
>On Mon, Jun 22, 2026 at 02:21:02PM +0000, Wei Yang wrote:
>> On Mon, Jun 22, 2026 at 02:46:40PM +0100, Lorenzo Stoakes wrote:
>> >+cc Lance, linux-kernel
>> >
>> >Your subject line is 83 characters long and is way too detailed how about 'fix
>> >device-private PMD handling'?
>> >
>>
>> Got it.
>>
>> >You forgot to include linux-kernel@vger.kernel.org on the mail, lore seems to be
>> >a bit broken atm but in general it's helpful to include that.
>>
>> Got it.
>>
>> So usually we send a patch to both linux-mm and linux-kernel? If so, I
>> remember is later actions.
>
>Yeah it's better for dealing with kvack going wrong etc. :)
>
>>
>> >
>> >Also is useful to make this [PATCH mm-hotfixes] to make it really clear it's
>> >intended as a hotfix.
>> >
>>
>> Got it.
>>
>> >Some commit msg language nits:
>> >
>> >On Mon, Jun 22, 2026 at 01:06:51PM +0000, Wei Yang wrote:
>> >> For pmd_trans_huge() and pmd_is_migration_entry(), we does following
>> >> before return the pmd entry:
>> >
>> >Sounds better as:
>> >
>> >	For PMD entries that satisfy pmd_trans_huge() or pmd_is_migration_entry(), we
>> >	perform the following actions:
>> >
>>
>> Sure.
>>
>> >>
>> >>   * re-validate pmd entry after PTL
>> >>   * check PVMW_MIGRATION
>> >>   * check_pmd()
>> >>   * handle on pte level if split under us
>> >>
>> >> But for device-private pmd, we just return after pmd_lock().
>> >
>> >->
>> >
>> >	However, for device-private PMD entries, we simply acquire the PMD lock
>> >	and return.
>> >
>>
>> Sure.
>>
>> >Also can you please give some justification here as to why all this also applies
>> >to device-private PMD? Right now it sounds hand wavey.
>> >
>>
>> I thought below paragraph explain it. Not sure what justification is preferred.
>
>Something about device private PMDs splitting the same way THP ones do, in the
>pmd_is_device_private_entry() branch of __split_huge_pmd_locked().
>

Hi, Lorenzo

Thanks for your detailed suggestions.

I tried to add the justification here, and the following is the commit log
after consolidate your suggestions.

    For PMD entries that satisfy pmd_trans_huge() or
    pmd_is_migration_entry(), we perform the following actions:
    
      * re-validate pmd entry after PTL
      * check PVMW_MIGRATION
      * check_pmd()
      * handle on pte level if split under us
    
    However, for device-private PMD entries, we simply acquire the PMD lock
    and return. This is not enough, as __split_huge_pmd_locked() would split
    a pmd device-private PMD under us just as it does for THP PMD.
    
    This is particularly problematic when PVMW_MIGRATION is set (meaning a
    migration entry is sought), as it causes a device-private PMD entry to
    be returned with a different data layout, causing memory corruption.

Just feel this is not that smooth. Would you mind taking another look to see
if I get your point correctly?

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Lorenzo Stoakes]

On Mon, Jun 22, 2026 at 11:45:18PM +0000, Wei Yang wrote:
> On Mon, Jun 22, 2026 at 05:11:02PM +0100, Lorenzo Stoakes wrote:
> >On Mon, Jun 22, 2026 at 02:21:02PM +0000, Wei Yang wrote:
> >> On Mon, Jun 22, 2026 at 02:46:40PM +0100, Lorenzo Stoakes wrote:
> >> >+cc Lance, linux-kernel
> >> >
> >> >Your subject line is 83 characters long and is way too detailed how about 'fix
> >> >device-private PMD handling'?
> >> >
> >>
> >> Got it.
> >>
> >> >You forgot to include linux-kernel@vger.kernel.org on the mail, lore seems to be
> >> >a bit broken atm but in general it's helpful to include that.
> >>
> >> Got it.
> >>
> >> So usually we send a patch to both linux-mm and linux-kernel? If so, I
> >> remember is later actions.
> >
> >Yeah it's better for dealing with kvack going wrong etc. :)
> >
> >>
> >> >
> >> >Also is useful to make this [PATCH mm-hotfixes] to make it really clear it's
> >> >intended as a hotfix.
> >> >
> >>
> >> Got it.
> >>
> >> >Some commit msg language nits:
> >> >
> >> >On Mon, Jun 22, 2026 at 01:06:51PM +0000, Wei Yang wrote:
> >> >> For pmd_trans_huge() and pmd_is_migration_entry(), we does following
> >> >> before return the pmd entry:
> >> >
> >> >Sounds better as:
> >> >
> >> >	For PMD entries that satisfy pmd_trans_huge() or pmd_is_migration_entry(), we
> >> >	perform the following actions:
> >> >
> >>
> >> Sure.
> >>
> >> >>
> >> >>   * re-validate pmd entry after PTL
> >> >>   * check PVMW_MIGRATION
> >> >>   * check_pmd()
> >> >>   * handle on pte level if split under us
> >> >>
> >> >> But for device-private pmd, we just return after pmd_lock().
> >> >
> >> >->
> >> >
> >> >	However, for device-private PMD entries, we simply acquire the PMD lock
> >> >	and return.
> >> >
> >>
> >> Sure.
> >>
> >> >Also can you please give some justification here as to why all this also applies
> >> >to device-private PMD? Right now it sounds hand wavey.
> >> >
> >>
> >> I thought below paragraph explain it. Not sure what justification is preferred.
> >
> >Something about device private PMDs splitting the same way THP ones do, in the
> >pmd_is_device_private_entry() branch of __split_huge_pmd_locked().
> >
>
> Hi, Lorenzo
>
> Thanks for your detailed suggestions.
>
> I tried to add the justification here, and the following is the commit log
> after consolidate your suggestions.
>
>     For PMD entries that satisfy pmd_trans_huge() or
>     pmd_is_migration_entry(), we perform the following actions:
>
>       * re-validate pmd entry after PTL
>       * check PVMW_MIGRATION
>       * check_pmd()
>       * handle on pte level if split under us
>
>     However, for device-private PMD entries, we simply acquire the PMD lock
>     and return. This is not enough, as __split_huge_pmd_locked() would split
>     a pmd device-private PMD under us just as it does for THP PMD.
>
>     This is particularly problematic when PVMW_MIGRATION is set (meaning a
>     migration entry is sought), as it causes a device-private PMD entry to
>     be returned with a different data layout, causing memory corruption.
>
> Just feel this is not that smooth. Would you mind taking another look to see
> if I get your point correctly?

Honestly I'd just drop the whole pmd_trans_huge()/pmd_is_migration_entry() bit
and say:

	Commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration support
	device-private entries") introduced the concept of device-private
	PMD entries, but did not correctly update the rmap walk code to
	account for them.

	As a result, when page_vma_mapped_walk() encounters device-private
	PMD entries, it takes no action other than to acquire the PMD lock
	and exit.

	However this is highly problematic for two reasons - firstly,
	device private entries possess a PFN so check_pmd() needs to be
	called to ensure an overlapping PFN range.

	Secondly, and more importantly, if PVMW_MIGRATION is set the
	caller assumes the returned entry is a migration entry, resulting
	in memory corruption when the caller tries to interpret the device
	private entry as such.

	In addition, commit 146287290023 ("mm/huge_memory: implement
	device-private THP splitting") allowed device private PMDs to be
	split like THP mappings, but again did not update this code path.

	As a result, we might race a PMD split prior to acquiring the PMD
	lock.

	This patch addresses all of these issues by invoking check_pmd(),
	ensuring PMVW_MIGRATION is not set and checks whether a split raced
	us we do for PMD THP and migration entries.

>
> --
> Wei Yang
> Help you, Help me

Cheers, Lorenzo

Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Wei Yang]

On Tue, Jun 23, 2026 at 06:02:56PM +0100, Lorenzo Stoakes wrote:
>On Mon, Jun 22, 2026 at 11:45:18PM +0000, Wei Yang wrote:
>> On Mon, Jun 22, 2026 at 05:11:02PM +0100, Lorenzo Stoakes wrote:
>> >On Mon, Jun 22, 2026 at 02:21:02PM +0000, Wei Yang wrote:
>> >> On Mon, Jun 22, 2026 at 02:46:40PM +0100, Lorenzo Stoakes wrote:
>> >> >+cc Lance, linux-kernel
>> >> >
>> >> >Your subject line is 83 characters long and is way too detailed how about 'fix
>> >> >device-private PMD handling'?
>> >> >
>> >>
>> >> Got it.
>> >>
>> >> >You forgot to include linux-kernel@vger.kernel.org on the mail, lore seems to be
>> >> >a bit broken atm but in general it's helpful to include that.
>> >>
>> >> Got it.
>> >>
>> >> So usually we send a patch to both linux-mm and linux-kernel? If so, I
>> >> remember is later actions.
>> >
>> >Yeah it's better for dealing with kvack going wrong etc. :)
>> >
>> >>
>> >> >
>> >> >Also is useful to make this [PATCH mm-hotfixes] to make it really clear it's
>> >> >intended as a hotfix.
>> >> >
>> >>
>> >> Got it.
>> >>
>> >> >Some commit msg language nits:
>> >> >
>> >> >On Mon, Jun 22, 2026 at 01:06:51PM +0000, Wei Yang wrote:
>> >> >> For pmd_trans_huge() and pmd_is_migration_entry(), we does following
>> >> >> before return the pmd entry:
>> >> >
>> >> >Sounds better as:
>> >> >
>> >> >	For PMD entries that satisfy pmd_trans_huge() or pmd_is_migration_entry(), we
>> >> >	perform the following actions:
>> >> >
>> >>
>> >> Sure.
>> >>
>> >> >>
>> >> >>   * re-validate pmd entry after PTL
>> >> >>   * check PVMW_MIGRATION
>> >> >>   * check_pmd()
>> >> >>   * handle on pte level if split under us
>> >> >>
>> >> >> But for device-private pmd, we just return after pmd_lock().
>> >> >
>> >> >->
>> >> >
>> >> >	However, for device-private PMD entries, we simply acquire the PMD lock
>> >> >	and return.
>> >> >
>> >>
>> >> Sure.
>> >>
>> >> >Also can you please give some justification here as to why all this also applies
>> >> >to device-private PMD? Right now it sounds hand wavey.
>> >> >
>> >>
>> >> I thought below paragraph explain it. Not sure what justification is preferred.
>> >
>> >Something about device private PMDs splitting the same way THP ones do, in the
>> >pmd_is_device_private_entry() branch of __split_huge_pmd_locked().
>> >
>>
>> Hi, Lorenzo
>>
>> Thanks for your detailed suggestions.
>>
>> I tried to add the justification here, and the following is the commit log
>> after consolidate your suggestions.
>>
>>     For PMD entries that satisfy pmd_trans_huge() or
>>     pmd_is_migration_entry(), we perform the following actions:
>>
>>       * re-validate pmd entry after PTL
>>       * check PVMW_MIGRATION
>>       * check_pmd()
>>       * handle on pte level if split under us
>>
>>     However, for device-private PMD entries, we simply acquire the PMD lock
>>     and return. This is not enough, as __split_huge_pmd_locked() would split
>>     a pmd device-private PMD under us just as it does for THP PMD.
>>
>>     This is particularly problematic when PVMW_MIGRATION is set (meaning a
>>     migration entry is sought), as it causes a device-private PMD entry to
>>     be returned with a different data layout, causing memory corruption.
>>
>> Just feel this is not that smooth. Would you mind taking another look to see
>> if I get your point correctly?
>
>Honestly I'd just drop the whole pmd_trans_huge()/pmd_is_migration_entry() bit
>and say:
>
>	Commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration support
>	device-private entries") introduced the concept of device-private
>	PMD entries, but did not correctly update the rmap walk code to
>	account for them.
>
>	As a result, when page_vma_mapped_walk() encounters device-private
>	PMD entries, it takes no action other than to acquire the PMD lock
>	and exit.
>
>	However this is highly problematic for two reasons - firstly,
>	device private entries possess a PFN so check_pmd() needs to be
>	called to ensure an overlapping PFN range.
>
>	Secondly, and more importantly, if PVMW_MIGRATION is set the
>	caller assumes the returned entry is a migration entry, resulting
>	in memory corruption when the caller tries to interpret the device
>	private entry as such.
>
>	In addition, commit 146287290023 ("mm/huge_memory: implement
>	device-private THP splitting") allowed device private PMDs to be
>	split like THP mappings, but again did not update this code path.
>
>	As a result, we might race a PMD split prior to acquiring the PMD
>	lock.
>
>	This patch addresses all of these issues by invoking check_pmd(),
>	ensuring PMVW_MIGRATION is not set and checks whether a split raced
>	us we do for PMD THP and migration entries.
>

Have to say this is much much better, thanks!

>
>Cheers, Lorenzo

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Lance Yang]

On 2026/6/22 21:46, Lorenzo Stoakes wrote:
> +cc Lance, linux-kernel

Thanks for the cc.

[...]

> 
> ----8<----
>  From e6a3c1c782714ed831c4d46a14bb99226423bf59 Mon Sep 17 00:00:00 2001
> From: Wei Yang <richard.weiyang@gmail.com>
> Date: Mon, 22 Jun 2026 13:06:51 +0000
> Subject: [PATCH] refactored
> 
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
>   mm/page_vma_mapped.c | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index 2ccbabfb2cc1..17dff8aab9f9 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -269,14 +269,24 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>   			/* THP pmd was split under us: handle on pte level */
>   			spin_unlock(pvmw->ptl);
>   			pvmw->ptl = NULL;
> -		} else if (!pmd_present(pmde)) {
> -			const softleaf_t entry = softleaf_from_pmd(pmde);
> +		} else if (pmd_is_device_private_entry(pmde)) {
> +			softleaf_t entry;
> +
> +			pvmw->ptl = pmd_lock(mm, pvmw->pmd);
> +			pmde = *pvmw->pmd;
> +			entry = softleaf_from_pmd(pmde);
> 
> -			if (softleaf_is_device_private(entry)) {
> -				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
> +			if (likely(softleaf_is_device_private(entry))) {
> +				if (pvmw->flags & PVMW_MIGRATION)
> +					return not_found(pvmw);
> +				if (!check_pmd(softleaf_to_pfn(entry), pvmw))
> +					return not_found(pvmw);
>   				return true;
>   			}
> -
> +			/* device-private pmd was split under us: handle on pte level */
> +			spin_unlock(pvmw->ptl);
> +			pvmw->ptl = NULL;
> +		} else if (!pmd_present(pmde)) {
>   			if ((pvmw->flags & PVMW_SYNC) &&
>   			    thp_vma_suitable_order(vma, pvmw->address,
>   						   PMD_ORDER) &&
> --
> 2.54.0

Cool, looks way cleaner :) Nothing jumped out at me :)

Cheers, Lance