linux-kernel - Re: [RFC 2/3] mm: add STATIC_PMD_ZERO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5dv5hsfvbdwyjlkxaeo2g43v6n4xe6ut7pjf6igrv7b25y2m5a@blllpcht5euu>
Date: Tue, 27 May 2025 20:00:50 +0200
From: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Pankaj Raghav <p.raghav@...sung.com>, 
	Suren Baghdasaryan <surenb@...gle.com>, Ryan Roberts <ryan.roberts@....com>, 
	Vlastimil Babka <vbabka@...e.cz>, Baolin Wang <baolin.wang@...ux.alibaba.com>, 
	Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>, 
	"H . Peter Anvin" <hpa@...or.com>, Zi Yan <ziy@...dia.com>, Mike Rapoport <rppt@...nel.org>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Michal Hocko <mhocko@...e.com>, 
	David Hildenbrand <david@...hat.com>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Thomas Gleixner <tglx@...utronix.de>, 
	Nico Pache <npache@...hat.com>, Dev Jain <dev.jain@....com>, 
	"Liam R . Howlett" <Liam.Howlett@...cle.com>, Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, linux-block@...r.kernel.org, willy@...radead.org, x86@...nel.org, 
	linux-fsdevel@...r.kernel.org, "Darrick J . Wong" <djwong@...nel.org>, mcgrof@...nel.org, 
	gost.dev@...sung.com, hch@....de
Subject: Re: [RFC 2/3] mm: add STATIC_PMD_ZERO_PAGE config option

On Tue, May 27, 2025 at 09:37:50AM -0700, Dave Hansen wrote:
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 055204dc211d..96f99b4f96ea 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -152,6 +152,7 @@ config X86
> >  	select ARCH_WANT_OPTIMIZE_HUGETLB_VMEMMAP	if X86_64
> >  	select ARCH_WANT_HUGETLB_VMEMMAP_PREINIT if X86_64
> >  	select ARCH_WANTS_THP_SWAP		if X86_64
> > +	select ARCH_WANTS_STATIC_PMD_ZERO_PAGE if X86_64
> 
> I don't think this should be the default. There are lots of little
> x86_64 VMs sitting around and 2MB might be significant to them.

This is the feedback I wanted. I will make it optional.

> > +config ARCH_WANTS_STATIC_PMD_ZERO_PAGE
> > +	bool
> > +
> > +config STATIC_PMD_ZERO_PAGE
> > +	def_bool y
> > +	depends on ARCH_WANTS_STATIC_PMD_ZERO_PAGE
> > +	help
> > +	  Typically huge_zero_folio, which is a PMD page of zeroes, is allocated
> > +	  on demand and deallocated when not in use. This option will always
> > +	  allocate huge_zero_folio for zeroing and it is never deallocated.
> > +	  Not suitable for memory constrained systems.
> 
> "Static" seems like a weird term to use for this. I was really expecting
> to see a 2MB object that gets allocated in .bss or something rather than
> a dynamically allocated page that's just never freed.

My first proposal was along those lines[0] (sorry I messed up version
while sending the patches). David Hilderbrand suggested to leverage the
infrastructure we already have in huge_memory.

> 
> >  menuconfig TRANSPARENT_HUGEPAGE
> >  	bool "Transparent Hugepage Support"
> >  	depends on HAVE_ARCH_TRANSPARENT_HUGEPAGE && !PREEMPT_RT
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 11edc4d66e74..ab8c16d04307 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -203,9 +203,17 @@ static void put_huge_zero_page(void)
> >  	BUG_ON(atomic_dec_and_test(&huge_zero_refcount));
> >  }
> >  
> > +/*
> > + * If STATIC_PMD_ZERO_PAGE is enabled, @mm can be NULL, i.e, the huge_zero_folio
> > + * is not associated with any mm_struct.
> > +*/
> 
> I get that callers have to handle failure. But isn't this pretty nasty
> for mm==NULL callers to be *guaranteed* to fail? They'll generate code
> for the success case that will never even run.
> 

The idea was to still have dynamic allocation possible even if this
config was disabled.

You are right that if this config is disabled, the callers with NULL mm
struct are guaranteed to fail, but we are not generating extra code
because there are still users who want dynamic allocation.

Do you think it is better to have the code with inside an #ifdef instead
of using the IS_ENABLED primitive?

[1] https://lore.kernel.org/linux-fsdevel/20250516101054.676046-2-p.raghav@samsung.com/

--
Pankaj