Re: [PATCH v3 1/5] mm/sparse-vmemmap: provide generic vmemmap_set_pmd() and vmemmap_check_pmd()

From: "Oscar Salvador (SUSE)" <osalvador@kernel.org>
To: Muchun Song <muchun.song@linux.dev>
Cc: "David Hildenbrand (Arm)" <david@kernel.org>,
	Muchun Song <songmuchun@bytedance.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <pjw@kernel.org>,
	Huacai Chen <chenhuacai@kernel.org>,
	Andreas Larsson <andreas@gaisler.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, loongarch@lists.linux.dev,
	sparclinux@vger.kernel.org, Alexandre Ghiti <alex@ghiti.fr>,
	Albert Ou <aou@eecs.berkeley.edu>,
	WANG Xuerui <kernel@xen0n.name>, Lorenzo Stoakes <ljs@kernel.org>,
	"Liam R. Howlett" <liam@infradead.org>,
	Vlastimil Babka <vbabka@kernel.org>,
	Mike Rapoport <rppt@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>
Subject: Re: [PATCH v3 1/5] mm/sparse-vmemmap: provide generic vmemmap_set_pmd() and vmemmap_check_pmd()
Date: Tue, 2 Jun 2026 06:40:24 +0200	[thread overview]
Message-ID: <ah5euKU9l3pP_8Ty@localhost.localdomain> (raw)
In-Reply-To: <0D4AF42F-37C5-4A76-9703-44ACC6374C48@linux.dev>

On Mon, Jun 01, 2026 at 08:37:42PM +0800, Muchun Song wrote:
> 
> 
> > On Jun 1, 2026, at 20:22, David Hildenbrand (Arm) <david@kernel.org> wrote:
> > 
> > On 6/1/26 10:48, Muchun Song wrote:
> >> The two weak functions are currently no-ops on every architecture,
> >> forcing each platform that needs them to duplicate the same handful
> >> of lines.  Provide a generic implementation:
> >> 
> >> - vmemmap_set_pmd() simply sets a huge PMD with PAGE_KERNEL protection.
> >> 
> >> - vmemmap_check_pmd() verifies that the PMD is present and leaf,
> >>  then calls the existing vmemmap_verify() helper.
> >> 
> >> Architectures that need special handling can continue to override the
> >> weak symbols; everyone else gets the standard version for free.
> >> 
> >> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> >> ---
> >> v2->v3:
> >> - Replace BUG_ON() with WARN_ON_ONCE() in vmemmap_set_pmd()
> >> ---
> >> mm/sparse-vmemmap.c | 7 ++++++-
> >> 1 file changed, 6 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> >> index 112ccf9c71ca..99e2be39671b 100644
> >> --- a/mm/sparse-vmemmap.c
> >> +++ b/mm/sparse-vmemmap.c
> >> @@ -386,12 +386,17 @@ int __meminit vmemmap_populate_hvo(unsigned long addr, unsigned long end,
> >> void __weak __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
> >>       unsigned long addr, unsigned long next)
> >> {
> >> + 	WARN_ON_ONCE(!pmd_set_huge(pmd, virt_to_phys(p), PAGE_KERNEL));
> > 
> > 
> > Not sure if a VM_WARN_ON_ONCE() would be appropriate. (then, we have to move the
> > pmd_set_huge() out of the statement).
> 
> I think it might be better to keep WARN_ON_ONCE here. This way, we can still
> monitor for warnings in production while keeping the code simple.

IIRC there was a discussion about this some time ago, given the fact that quite
some people out there tend to have 'panic_on_warn', but then again, if we fail to
set a PMD here it means something pretty nasty happened, so I am ok with that.

-- 
Oscar Salvador
SUSE Labs