On Mon, Aug 11, 2025 at 01:26:25PM +0200, David Hildenbrand wrote:
> The huge zero folio is refcounted (+mapcounted -- is that a word?)
> differently than "normal" folios, similarly (but different) to the ordinary
> shared zeropage.
>
> For this reason, we special-case these pages in
> vm_normal_page*/vm_normal_folio*, and only allow selected callers to
> still use them (e.g., GUP can still take a reference on them).
Hm, interestingly in gup_fast_pmd_leaf() we explicitly check pmd_special(),
so surely setting the zero huge pmd special will change behaviour there?
But I guess this is actually _more_ correct as it's not really sensible to
grab the huge zero PMD page.
Then again, follow_huge_pmd() _will_, afaict.
I see the GUP fast change was introduced by commit ae3c99e650da ("mm/gup:
detect huge pfnmap entries in gup-fast") so was specifically intended for
pfnmap not the zero page.
>
> vm_normal_page_pmd() already filters out the huge zero folio, to
> indicate it a special (return NULL). However, so far we are not making
> use of pmd_special() on architectures that support it
> (CONFIG_ARCH_HAS_PTE_SPECIAL), like we would with the ordinary shared
> zeropage.
>
> Let's mark PMD mappings of the huge zero folio similarly as special, so we
> can avoid the manual check for the huge zero folio with
> CONFIG_ARCH_HAS_PTE_SPECIAL next, and only perform the check on
> !CONFIG_ARCH_HAS_PTE_SPECIAL.
>
> In copy_huge_pmd(), where we have a manual pmd_special() check to handle
> PFNMAP, we have to manually rule out the huge zero folio. That code
> needs a serious cleanup, but that's something for another day.
>
> While at it, update the doc regarding the shared zero folios.
>
> No functional change intended: vm_normal_page_pmd() still returns NULL
> when it encounters the huge zero folio.
>
> Reviewed-by: Oscar Salvador <osalva...@suse.de>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
I R-b this before, and Wei did also, did you drop because of changes?
Anyway, apart from query about GUP-fast above, this LGTM so:
Reviewed-by: Lorenzo Stoakes <lorenzo.stoa...@oracle.com>
> ---
> mm/huge_memory.c | 8 ++++++--
> mm/memory.c | 15 ++++++++++-----
> 2 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index ec89e0607424e..58bac83e7fa31 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1309,6 +1309,7 @@ static void set_huge_zero_folio(pgtable_t pgtable,
> struct mm_struct *mm,
> {
> pmd_t entry;
> entry = folio_mk_pmd(zero_folio, vma->vm_page_prot);
> + entry = pmd_mkspecial(entry);
> pgtable_trans_huge_deposit(mm, pmd, pgtable);
> set_pmd_at(mm, haddr, pmd, entry);
> mm_inc_nr_ptes(mm);
> @@ -1418,7 +1419,9 @@ static vm_fault_t insert_pmd(struct vm_area_struct
> *vma, unsigned long addr,
> if (fop.is_folio) {
> entry = folio_mk_pmd(fop.folio, vma->vm_page_prot);
>
> - if (!is_huge_zero_folio(fop.folio)) {
> + if (is_huge_zero_folio(fop.folio)) {
> + entry = pmd_mkspecial(entry);
> + } else {
> folio_get(fop.folio);
> folio_add_file_rmap_pmd(fop.folio, &fop.folio->page,
> vma);
> add_mm_counter(mm, mm_counter_file(fop.folio),
> HPAGE_PMD_NR);
> @@ -1643,7 +1646,8 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct
> mm_struct *src_mm,
> int ret = -ENOMEM;
>
> pmd = pmdp_get_lockless(src_pmd);
> - if (unlikely(pmd_present(pmd) && pmd_special(pmd))) {
> + if (unlikely(pmd_present(pmd) && pmd_special(pmd) &&
> + !is_huge_zero_pmd(pmd))) {
OK yeah this is new I see from cover letter + ranged-diff.
Yeah this is important actually wow, as otherwise the is_huge_zero_pmd()
branch will not be executed.
Good spot!
> dst_ptl = pmd_lock(dst_mm, dst_pmd);
> src_ptl = pmd_lockptr(src_mm, src_pmd);
> spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
> diff --git a/mm/memory.c b/mm/memory.c
> index 0ba4f6b718471..626caedce35e0 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -555,7 +555,14 @@ static void print_bad_pte(struct vm_area_struct *vma,
> unsigned long addr,
> *
> * "Special" mappings do not wish to be associated with a "struct page"
> (either
> * it doesn't exist, or it exists but they don't want to touch it). In this
> - * case, NULL is returned here. "Normal" mappings do have a struct page.
> + * case, NULL is returned here. "Normal" mappings do have a struct page and
> + * are ordinarily refcounted.
> + *
> + * Page mappings of the shared zero folios are always considered "special",
> as
> + * they are not ordinarily refcounted: neither the refcount nor the mapcount
> + * of these folios is adjusted when mapping them into user page tables.
> + * Selected page table walkers (such as GUP) can still identify mappings of
> the
> + * shared zero folios and work with the underlying "struct page".
Thanks for this.
> *
> * There are 2 broad cases. Firstly, an architecture may define a
> pte_special()
> * pte bit, in which case this function is trivial. Secondly, an architecture
> @@ -585,9 +592,8 @@ static void print_bad_pte(struct vm_area_struct *vma,
> unsigned long addr,
> *
> * VM_MIXEDMAP mappings can likewise contain memory with or without "struct
> * page" backing, however the difference is that _all_ pages with a struct
> - * page (that is, those where pfn_valid is true) are refcounted and
> considered
> - * normal pages by the VM. The only exception are zeropages, which are
> - * *never* refcounted.
> + * page (that is, those where pfn_valid is true, except the shared zero
> + * folios) are refcounted and considered normal pages by the VM.
> *
> * The disadvantage is that pages are refcounted (which can be slower and
> * simply not an option for some PFNMAP users). The advantage is that we
> @@ -667,7 +673,6 @@ struct page *vm_normal_page_pmd(struct vm_area_struct
> *vma, unsigned long addr,
> {
> unsigned long pfn = pmd_pfn(pmd);
>
> - /* Currently it's only used for huge pfnmaps */
> if (unlikely(pmd_special(pmd)))
> return NULL;
>
> --
> 2.50.1
>
