All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Kevin Brodsky" <kevin.brodsky@arm.com>
Cc: "Vasily Gorbik" <gor@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	linux-s390@vger.kernel.org,
	"Alexander Gordeev" <agordeev@linux.ibm.com>
Subject: Re: [PATCH 13/22] mm: account page table pages when allocated
Date: Tue, 14 Jul 2026 14:54:45 +0000	[thread overview]
Message-ID: <20260714145446.38A871F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260714-remove_pgtable_cdtor-v1-13-44be8a7685d7@arm.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] NR_PAGETABLE statistic leak on architectures that do not define CONFIG_MMU_GATHER_TABLE_FREE.
- [Medium] Direct calls to pagetable_dtor() followed by generic page freeing functions bypass the NR_PAGETABLE decrement, causing stat leaks on ARM64 memory hot-remove.
- [Medium] SPARC32 SRMMU page tables completely lose NR_PAGETABLE accounting.
- [Medium] Memory hot-remove and vmemmap teardown on x86 will underflow the NR_PAGETABLE counter.
--

--- Patch [13]: [PATCH 13/22] mm: account page table pages when allocated ---
commit 5c303720c0cd77f50e7b180d025a9a9869aab6a2
Author: Kevin Brodsky <kevin.brodsky@arm.com>
mm: account page table pages when allocated

This commit moves memory accounting of pagetable pages from the ctor/dtor
functions to pagetable_alloc() and pagetable_free() in order to improve
accounting where ctor/dtor calls might be missing.

Link: https://lore.kernel.org/r/20251113140448.1814860-3-willy@infradead.org

> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index b336eb32faa05..361a0d7b74c51 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3681,6 +3681,11 @@ static inline struct ptdesc *pagetable_alloc_noprof(gfp_t gfp, unsigned int orde
>  {
>  	struct page *page = alloc_pages_noprof(gfp | __GFP_COMP, order);
>  
> +	if (!page)
> +		return NULL;
> +
> +	lruvec_stat_add_folio(page_folio(page), NR_PAGETABLE);
> +
>  	return page_ptdesc(page);
>  }

[Severity: Medium]
For architectures or code paths that allocate page tables without using
pagetable_alloc_noprof(), will we now lose the NR_PAGETABLE accounting?

For example, on SPARC32, pte_alloc_one() in arch/sparc/mm/srmmu.c allocates
from a pre-allocated pool via srmmu_get_nocache() and previously relied on
pagetable_pte_ctor() to increment the stat:

arch/sparc/mm/srmmu.c:pte_alloc_one() {
    ...
	if (page_ref_inc_return(page) == 2 &&
		!pagetable_pte_ctor(mm, page_ptdesc(page))) {
    ...
}

Similarly, the freeing path mirrors this bypass, using srmmu_free_nocache()
instead of pagetable_free(). Does this change cause SPARC32 to completely
lose user PTE accounting?

> @@ -3689,6 +3694,8 @@ static inline void __pagetable_free(struct ptdesc *pt)
>  {
>  	struct page *page = ptdesc_page(pt);
>  
> +	lruvec_stat_sub_folio(page_folio(page), NR_PAGETABLE);
> +
>  	__free_pages(page, compound_order(page));
>  }

[Severity: Medium]
Will this cause an underflow on x86 during memory hot-remove?

In arch/x86/mm/init_64.c, free_pagetable() calls pagetable_free() (which
now decrements the stat) during hot-remove:

arch/x86/mm/init_64.c:free_pagetable() {
    ...
	if (PageReserved(page))
		free_reserved_page(page);
	else
		pagetable_free(page_ptdesc(page));
}

However, these kernel page tables were originally allocated during hot-add
with spp_getpage() or vmemmap_alloc_block_buf(). Both of these use base
allocators and bypass pagetable_alloc_noprof().

Because the stat is never incremented on allocation but is now decremented
on free, does this cause an underflow?

[ ... ]

> @@ -3797,7 +3804,6 @@ static inline void __pagetable_ctor(struct ptdesc *ptdesc)
>  	struct folio *folio = ptdesc_folio(ptdesc);
>  
>  	__folio_set_pgtable(folio);
> -	lruvec_stat_add_folio(folio, NR_PAGETABLE);
>  }
>  
>  static inline void pagetable_dtor(struct ptdesc *ptdesc)
> @@ -3806,7 +3812,6 @@ static inline void pagetable_dtor(struct ptdesc *ptdesc)
>  
>  	ptlock_free(ptdesc);
>  	__folio_clear_pgtable(folio);
> -	lruvec_stat_sub_folio(folio, NR_PAGETABLE);
>  }

[Severity: Medium]
Will this leak the NR_PAGETABLE counter on architectures that bypass
__pagetable_free() and only call pagetable_dtor()?

On architectures lacking CONFIG_MMU_GATHER_TABLE_FREE (like m68k or xtensa),
the fallback tlb_remove_table() implementation explicitly calls
pagetable_dtor() and then enqueues the page for generic batch freeing:

include/asm-generic/tlb.h:tlb_remove_table() {
	struct ptdesc *ptdesc = (struct ptdesc *)table;

	pagetable_dtor(ptdesc);
	tlb_remove_page(tlb, ptdesc_page(ptdesc));
}

Since the decrement was moved from pagetable_dtor() to __pagetable_free(),
which is bypassed here, will this leak the counter when userspace unmaps
memory?

A similar bypass appears to occur on ARM64 during memory hot-remove:

arch/arm64/mm/mmu.c:free_hotplug_pgtable_page() {
	pagetable_dtor(page_ptdesc(page));
	free_hotplug_page_range(page, PAGE_SIZE, NULL);
}

Since this directly calls pagetable_dtor() and skips __pagetable_free(),
will this also leave the stat un-decremented?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-remove_pgtable_cdtor-v1-0-44be8a7685d7@arm.com?part=13

  reply	other threads:[~2026-07-14 14:54 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14 14:03 [PATCH 00/22] Simplify special kernel page table handling Kevin Brodsky
2026-07-14 14:03 ` Kevin Brodsky
2026-07-14 14:03 ` [PATCH 01/22] mm: drop unused __mm_flags_set_mask_bits_word() Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:03 ` [PATCH 02/22] mm: move mm_flags helpers to mm_types.h Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:03 ` [PATCH 03/22] mm: introduce MMF_KERNEL flag and set it for init_mm Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:47   ` Dave Hansen
2026-07-14 14:47     ` Dave Hansen
2026-07-14 15:04     ` Lorenzo Stoakes (ARM)
2026-07-14 15:04       ` Lorenzo Stoakes (ARM)
2026-07-14 14:03 ` [PATCH 04/22] mm: use mm_is_kernel() in generic page table code Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:28   ` sashiko-bot
2026-07-14 14:03 ` [PATCH 05/22] arm64: mm: use mm_is_kernel() for kernel mm checks Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:03 ` [PATCH 06/22] loongarch: mm: use mm_is_kernel() in switch_mm_irqs_off() Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:03 ` [PATCH 07/22] parisc: mm: use mm_is_kernel() for kernel mm checks Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:03 ` [PATCH 08/22] powerpc: " Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:03 ` [PATCH 09/22] s390: " Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:03 ` [PATCH 10/22] sparc: " Kevin Brodsky
2026-07-14 14:03   ` Kevin Brodsky
2026-07-14 14:04 ` [PATCH 11/22] um: mm: use mm_is_kernel() in TLB sync Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 14:04 ` [PATCH 12/22] x86/mm: use mm_is_kernel() for kernel mm checks Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 15:09   ` Dave Hansen
2026-07-14 15:09     ` Dave Hansen
2026-07-14 14:04 ` [PATCH 13/22] mm: account page table pages when allocated Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 14:54   ` sashiko-bot [this message]
2026-07-14 14:04 ` [PATCH 14/22] mm: set page table page type " Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 14:54   ` sashiko-bot
2026-07-14 15:16   ` Vishal Moola
2026-07-14 15:16     ` Vishal Moola
2026-07-14 14:04 ` [PATCH 15/22] mm: only initialise pt_share_count for user pgtables Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 14:04 ` [PATCH 16/22] efi: mark efi_mm as a kernel mm Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 15:16   ` sashiko-bot
2026-07-14 14:04 ` [PATCH 17/22] mm: pagewalk: drop redundant address check for kernel mm walks Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 14:42   ` sashiko-bot
2026-07-14 14:04 ` [PATCH 18/22] arm64: mm: drop explicit mm_is_efi() check in contpte Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 14:04 ` [PATCH 19/22] x86/tboot: mark tboot_mm as a kernel mm Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 15:19   ` Dave Hansen
2026-07-14 15:19     ` Dave Hansen
2026-07-14 14:04 ` [PATCH 20/22] arm64: mm: drop ctor/dtor calls for kernel page tables Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 14:56   ` sashiko-bot
2026-07-14 14:04 ` [PATCH 21/22] arm: mm: drop ctor call " Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky
2026-07-14 14:04 ` [PATCH 22/22] riscv: mm: drop ctor/dtor calls " Kevin Brodsky
2026-07-14 14:04   ` Kevin Brodsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260714145446.38A871F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=kevin.brodsky@arm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.