On Tue, Aug 05, 2025 at 09:33:10AM -0700, Dave Hansen wrote: > On 8/4/25 05:13, Pankaj Raghav (Samsung) wrote: > > From: Pankaj Raghav <p.raghav@xxxxxxxxxxx> > > > > There are many places in the kernel where we need to zeroout larger > > chunks but the maximum segment we can zeroout at a time by ZERO_PAGE > > is limited by PAGE_SIZE. > ... > > In x86-land, the rules are pretty clear about using imperative voice. > There are quite a few "we's" in the changelog and comments in this series. > > I do think they're generally good to avoid and do lead to more clarity, > but I'm also not sure how important that is in mm-land these days. Yeah, I will change it to imperative to stay consistent. <snip> > > static inline int split_folio_to_list_to_order(struct folio *folio, > > diff --git a/mm/Kconfig b/mm/Kconfig > > index e443fe8cd6cf..366a6d2d771e 100644 > > --- a/mm/Kconfig > > +++ b/mm/Kconfig > > @@ -823,6 +823,27 @@ config ARCH_WANT_GENERAL_HUGETLB > > config ARCH_WANTS_THP_SWAP > > def_bool n > > > > +config ARCH_WANTS_STATIC_HUGE_ZERO_FOLIO > > + def_bool n > > + > > +config STATIC_HUGE_ZERO_FOLIO > > + bool "Allocate a PMD sized folio for zeroing" > > + depends on ARCH_WANTS_STATIC_HUGE_ZERO_FOLIO && TRANSPARENT_HUGEPAGE > > + help > > + Without this config enabled, the huge zero folio is allocated on > > + demand and freed under memory pressure once no longer in use. > > + To detect remaining users reliably, references to the huge zero folio > > + must be tracked precisely, so it is commonly only available for mapping > > + it into user page tables. > > + > > + With this config enabled, the huge zero folio can also be used > > + for other purposes that do not implement precise reference counting: > > + it is still allocated on demand, but never freed, allowing for more > > + wide-spread use, for example, when performing I/O similar to the > > + traditional shared zeropage. > > + > > + Not suitable for memory constrained systems. > > IMNHO, this is written like a changelog, not documentation for end users > trying to make sense of Kconfig options. I'd suggest keeping it short > and sweet: > > config PERSISTENT_HUGE_ZERO_FOLIO > bool "Allocate a persistent PMD-sized folio for zeroing" > ... > help > Enable this option to reduce the runtime refcounting overhead > of the huge zero folio and expand the places in the kernel > that can use huge zero folios. > > With this option enabled, the huge zero folio is allocated > once and never freed. It potentially wastes one huge page > worth of memory. > > Say Y if your system has lots of memory. Say N if you are > memory constrained. > This looks short and to the point. I can fold this in the next version. Thanks. > > config MM_ID > > def_bool n > > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index ff06dee213eb..e117b280b38d 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -75,6 +75,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink, > > static bool split_underused_thp = true; > > > > static atomic_t huge_zero_refcount; > > +atomic_t huge_zero_folio_is_static __read_mostly; > > struct folio *huge_zero_folio __read_mostly; > > unsigned long huge_zero_pfn __read_mostly = ~0UL; > > unsigned long huge_anon_orders_always __read_mostly; > > @@ -266,6 +267,45 @@ void mm_put_huge_zero_folio(struct mm_struct *mm) > > put_huge_zero_folio(); > > } > > > > +#ifdef CONFIG_STATIC_HUGE_ZERO_FOLIO > > + > > +struct folio *__get_static_huge_zero_folio(void) > > +{ > > + static unsigned long fail_count_clear_timer; > > + static atomic_t huge_zero_static_fail_count __read_mostly; > > + > > + if (unlikely(!slab_is_available())) > > + return NULL; > > + > > + /* > > + * If we failed to allocate a huge zero folio, just refrain from > > + * trying for one minute before retrying to get a reference again. > > + */ > > + if (atomic_read(&huge_zero_static_fail_count) > 1) { > > + if (time_before(jiffies, fail_count_clear_timer)) > > + return NULL; > > + atomic_set(&huge_zero_static_fail_count, 0); > > + } > > Any reason that this is an open-coded ratelimit instead of using > 'struct ratelimit_state'? > > I also find the 'huge_zero_static_fail_count' use pretty unintuitive. > This is fundamentally a slow path. Ideally, it's called once. In the > pathological case, it's called once a minute. > > I'd probably just recommend putting a rate limit on this function, then > using a plain old mutex for the actual allocation to keep multiple > threads out. > > Then the function becomes something like this: > > if (__ratelimit(&huge_zero_alloc_ratelimit)) > return; > > guard(mutex)(&huge_zero_mutex); > > if (!get_huge_zero_folio()) > return NULL; > > static_key_enable(&huge_zero_noref_key); > > return huge_zero_folio; > > No atomic, no cmpxchg, no races on allocating. David already reworked this part based on Lorenzo's feedback (he also did not like the ratelimiting part like you). The reworked diff is here[1]. No ratelimiting, etc. > > > ... > > static unsigned long shrink_huge_zero_folio_count(struct shrinker *shrink, > > struct shrink_control *sc) > > { > > @@ -277,7 +317,11 @@ static unsigned long shrink_huge_zero_folio_scan(struct shrinker *shrink, > > struct shrink_control *sc) > > { > > if (atomic_cmpxchg(&huge_zero_refcount, 1, 0) == 1) { > > - struct folio *zero_folio = xchg(&huge_zero_folio, NULL); > > + struct folio *zero_folio; > > + > > + if (WARN_ON_ONCE(atomic_read(&huge_zero_folio_is_static))) > > + return 0; > > + zero_folio = xchg(&huge_zero_folio, NULL); > > BUG_ON(zero_folio == NULL); > > WRITE_ONCE(huge_zero_pfn, ~0UL); > > folio_put(zero_folio); > > This seems like a hack to me. If you don't want the shrinker to run, > then deregister it. Keeping the refcount elevated is fine, but > repeatedly calling the shrinker to do atomic_cmpxchg() when you *know* > it will do nothing seems silly. > The new version[1] deregisters instead of having this condition. :) > If you can't deregister the shrinker, at least use the static_key > approach and check the static key instead of doing futile cmpxchg's forever. -- Pankaj [1] https://lore.kernel.org/linux-mm/70049abc-bf79-4d04-a0a8-dd3787195986@xxxxxxxxxx/