[PATCH v2 06/14] mm/huge_memory: Remove pXd_devmap usage from insert_pXd_pfn()

Alistair Popple posted 14 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 06/14] mm/huge_memory: Remove pXd_devmap usage from insert_pXd_pfn()
Posted by Alistair Popple 3 months, 3 weeks ago
Nothing uses PFN_DEV anymore so no need to create devmap pXd's when
mapping a PFN. Instead special mappings will be created which ensures
vm_normal_page_pXd() will not return pages which don't have an
associated page. This could change behaviour slightly on architectures
where pXd_devmap() does not imply pXd_special() as the normal page
checks would have fallen through to checking VM_PFNMAP/MIXEDMAP instead,
which in theory at least could have returned a page.

However vm_normal_page_pXd() should never have been returning pages for
pXd_devmap() entries anyway, so anything relying on that would have been
a bug.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

Changes since v1:

 - New for v2
---
 mm/huge_memory.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index b096240..6514e25 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1415,11 +1415,7 @@ static int insert_pmd(struct vm_area_struct *vma, unsigned long addr,
 		add_mm_counter(mm, mm_counter_file(fop.folio), HPAGE_PMD_NR);
 	} else {
 		entry = pmd_mkhuge(pfn_t_pmd(fop.pfn, prot));
-
-		if (pfn_t_devmap(fop.pfn))
-			entry = pmd_mkdevmap(entry);
-		else
-			entry = pmd_mkspecial(entry);
+		entry = pmd_mkspecial(entry);
 	}
 	if (write) {
 		entry = pmd_mkyoung(pmd_mkdirty(entry));
@@ -1565,11 +1561,7 @@ static void insert_pud(struct vm_area_struct *vma, unsigned long addr,
 		add_mm_counter(mm, mm_counter_file(fop.folio), HPAGE_PUD_NR);
 	} else {
 		entry = pud_mkhuge(pfn_t_pud(fop.pfn, prot));
-
-		if (pfn_t_devmap(fop.pfn))
-			entry = pud_mkdevmap(entry);
-		else
-			entry = pud_mkspecial(entry);
+		entry = pud_mkspecial(entry);
 	}
 	if (write) {
 		entry = pud_mkyoung(pud_mkdirty(entry));
-- 
git-series 0.9.1
Re: [PATCH v2 06/14] mm/huge_memory: Remove pXd_devmap usage from insert_pXd_pfn()
Posted by David Hildenbrand 3 months, 3 weeks ago
On 16.06.25 13:58, Alistair Popple wrote:
> Nothing uses PFN_DEV anymore so no need to create devmap pXd's when
> mapping a PFN. Instead special mappings will be created which ensures
> vm_normal_page_pXd() will not return pages which don't have an
> associated page. This could change behaviour slightly on architectures
> where pXd_devmap() does not imply pXd_special() as the normal page
> checks would have fallen through to checking VM_PFNMAP/MIXEDMAP instead,
> which in theory at least could have returned a page.
> 
> However vm_normal_page_pXd() should never have been returning pages for
> pXd_devmap() entries anyway, so anything relying on that would have been
> a bug.
> 
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> 
> ---
> 
> Changes since v1:
> 
>   - New for v2
> ---
>   mm/huge_memory.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index b096240..6514e25 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1415,11 +1415,7 @@ static int insert_pmd(struct vm_area_struct *vma, unsigned long addr,
>   		add_mm_counter(mm, mm_counter_file(fop.folio), HPAGE_PMD_NR);
>   	} else {
>   		entry = pmd_mkhuge(pfn_t_pmd(fop.pfn, prot));
> -
> -		if (pfn_t_devmap(fop.pfn))
> -			entry = pmd_mkdevmap(entry);
> -		else
> -			entry = pmd_mkspecial(entry);
> +		entry = pmd_mkspecial(entry);
>   	}
>   	if (write) {
>   		entry = pmd_mkyoung(pmd_mkdirty(entry));
> @@ -1565,11 +1561,7 @@ static void insert_pud(struct vm_area_struct *vma, unsigned long addr,
>   		add_mm_counter(mm, mm_counter_file(fop.folio), HPAGE_PUD_NR);
>   	} else {
>   		entry = pud_mkhuge(pfn_t_pud(fop.pfn, prot));
> -
> -		if (pfn_t_devmap(fop.pfn))
> -			entry = pud_mkdevmap(entry);
> -		else
> -			entry = pud_mkspecial(entry);
> +		entry = pud_mkspecial(entry);
>   	}
>   	if (write) {
>   		entry = pud_mkyoung(pud_mkdirty(entry));


Why not squash this patch into #3, and remove the pmd_special() check 
from vm_normal_page_pmd() in the same go? Seems wrong to handle the 
PMD/PUD case separately.

But now I am confused why some pte_devmap() checks are removed in patch 
#3, while others are removed in #7.

Why not split it up into (a) stop setting p*_devmap() and (b) remove 
p*_devmap().

Logically makes more sense to me ... :)

-- 
Cheers,

David / dhildenb
Re: [PATCH v2 06/14] mm/huge_memory: Remove pXd_devmap usage from insert_pXd_pfn()
Posted by Alistair Popple 3 months, 3 weeks ago
On Tue, Jun 17, 2025 at 11:49:20AM +0200, David Hildenbrand wrote:
> On 16.06.25 13:58, Alistair Popple wrote:
> > Nothing uses PFN_DEV anymore so no need to create devmap pXd's when
> > mapping a PFN. Instead special mappings will be created which ensures
> > vm_normal_page_pXd() will not return pages which don't have an
> > associated page. This could change behaviour slightly on architectures
> > where pXd_devmap() does not imply pXd_special() as the normal page
> > checks would have fallen through to checking VM_PFNMAP/MIXEDMAP instead,
> > which in theory at least could have returned a page.
> > 
> > However vm_normal_page_pXd() should never have been returning pages for
> > pXd_devmap() entries anyway, so anything relying on that would have been
> > a bug.
> > 
> > Signed-off-by: Alistair Popple <apopple@nvidia.com>
> > 
> > ---
> > 
> > Changes since v1:
> > 
> >   - New for v2
> > ---
> >   mm/huge_memory.c | 12 ++----------
> >   1 file changed, 2 insertions(+), 10 deletions(-)
> > 
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index b096240..6514e25 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -1415,11 +1415,7 @@ static int insert_pmd(struct vm_area_struct *vma, unsigned long addr,
> >   		add_mm_counter(mm, mm_counter_file(fop.folio), HPAGE_PMD_NR);
> >   	} else {
> >   		entry = pmd_mkhuge(pfn_t_pmd(fop.pfn, prot));
> > -
> > -		if (pfn_t_devmap(fop.pfn))
> > -			entry = pmd_mkdevmap(entry);
> > -		else
> > -			entry = pmd_mkspecial(entry);
> > +		entry = pmd_mkspecial(entry);
> >   	}
> >   	if (write) {
> >   		entry = pmd_mkyoung(pmd_mkdirty(entry));
> > @@ -1565,11 +1561,7 @@ static void insert_pud(struct vm_area_struct *vma, unsigned long addr,
> >   		add_mm_counter(mm, mm_counter_file(fop.folio), HPAGE_PUD_NR);
> >   	} else {
> >   		entry = pud_mkhuge(pfn_t_pud(fop.pfn, prot));
> > -
> > -		if (pfn_t_devmap(fop.pfn))
> > -			entry = pud_mkdevmap(entry);
> > -		else
> > -			entry = pud_mkspecial(entry);
> > +		entry = pud_mkspecial(entry);
> >   	}
> >   	if (write) {
> >   		entry = pud_mkyoung(pud_mkdirty(entry));
> 
> 
> Why not squash this patch into #3, and remove the pmd_special() check from
> vm_normal_page_pmd() in the same go? Seems wrong to handle the PMD/PUD case
> separately.

Yeah, that was mostly because "someone" (and thankyou btw, it was somewhat my
mess) changed all this while I was working on it :-) I wanted to make the rebase
fixups obvious but will squash them for v3.

> But now I am confused why some pte_devmap() checks are removed in patch #3,
> while others are removed in #7.
> 
> Why not split it up into (a) stop setting p*_devmap() and (b) remove
> p*_devmap().
> 
> Logically makes more sense to me ... :)

Heh. You're right. For various reasons this patch series has gone through a
couple of reorderings, mainly to get rid of unused stuff early in the series but
that didn't work out due to that RISC-V bug. I needed a break from silly rebase
build errors so this was a good checkpoint.

But I've reworked things for v3 to get the ordering a bit more sensible.

> -- 
> Cheers,
> 
> David / dhildenb
>