[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6bb233ef-5e56-4546-b571-6a5f052d8b45@redhat.com>
Date: Tue, 17 Jun 2025 11:49:20 +0200
From: David Hildenbrand <david@...hat.com>
To: Alistair Popple <apopple@...dia.com>, akpm@...ux-foundation.org
Cc: linux-mm@...ck.org, gerald.schaefer@...ux.ibm.com,
dan.j.williams@...el.com, jgg@...pe.ca, willy@...radead.org,
linux-kernel@...r.kernel.org, nvdimm@...ts.linux.dev,
linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
linux-xfs@...r.kernel.org, jhubbard@...dia.com, hch@....de,
zhang.lyra@...il.com, debug@...osinc.com, bjorn@...nel.org,
balbirs@...dia.com, lorenzo.stoakes@...cle.com,
linux-arm-kernel@...ts.infradead.org, loongarch@...ts.linux.dev,
linuxppc-dev@...ts.ozlabs.org, linux-riscv@...ts.infradead.org,
linux-cxl@...r.kernel.org, dri-devel@...ts.freedesktop.org, John@...ves.net,
m.szyprowski@...sung.com
Subject: Re: [PATCH v2 06/14] mm/huge_memory: Remove pXd_devmap usage from
insert_pXd_pfn()
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@...dia.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
Powered by blists - more mailing lists