Re: [PATCH v2 2/4] mm/mglru: add PUD-level Bloom filter state and generic helpers

[Baoquan He <baoquan.he@linux.dev>]

Hi Kairui,

On 10/17/26 at 02:39am, Kairui Song wrote:
> On Tue, Sep 01, 2026 at 02:37:53PM +0800, Baoquan He wrote:
> > Rename the existing PMD-level filter array (filters -> pmd_filters) and
> > add a second, coarser pair of PUD-level Bloom filters (pud_filters).
> > The PUL filsters operate at 1GB (PUD) granularity, whose 512 PMDs would
> > all fail the PMD-level filter - a general suppression of unnecessary page
> > table walks. Cross-node empty walks are the most visible case: a foreign
> > mm's PUDs have no young entries for this lruvec, so the entire PMD
> > iteration is pure waste.
> > 
> > Generalize the Bloom filter helpers (test/update/reset_bloom_filter ->
> > __test/update/reset_bloom_filter) to operate on a generic filters array,
> > and add thin wrappers (test/update/reset_pmd_bloom_filter) that pass the
> > PMD-level array, so the PUD-level pair can reuse the same hash,
> > double-buffering and reset.
> 
> Hi Baoquan,
> 
> This is really nice idea, thanks! And sorry for the late response.

Thanks a lot for careful reviewing, and I am sorry I just missed it.
Please see the reply at the bottom.

> 
> > 
> > No behavior change yet; the PUD filters are populated and used by later
> > patches.
> > 
> > Signed-off-by: Baoquan He <hebaoquan@kylinos.cn>
> > ---
> >  include/linux/mmzone.h |  6 ++++--
> >  mm/vmscan.c            | 45 ++++++++++++++++++++++++++++--------------
> >  2 files changed, 34 insertions(+), 17 deletions(-)
> > 
> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > index f75daa6cd02d..faf28d35065d 100644
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -612,8 +612,10 @@ struct lru_gen_mm_state {
> >  	struct list_head *head;
> >  	/* where the last iteration ended before */
> >  	struct list_head *tail;
> > -	/* Bloom filters flip after each iteration */
> > -	unsigned long *filters[NR_BLOOM_FILTERS];
> > +	/* PMD-level Bloom filters flip after each iteration */
> > +	unsigned long *pmd_filters[NR_BLOOM_FILTERS];
> > +	/* PUD-level Bloom filters flip after each iteration */
> > +	unsigned long *pud_filters[NR_BLOOM_FILTERS];
> 
> Or could we have macros like:
> BLOOM_FILTER_LEVELS 2
> BLOOM_FILTER_PUD 0
> BLOOM_FILTER_PMD 1
> 
> (just personal flavor).

Very great flavor, have taken it in my v3.

> 
> >  	/* the mm stats for debugging */
> >  	unsigned long stats[NR_HIST_GENS][NR_MM_STATS];
> >  };
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 55d43ab54459..cd09c5b8af8c 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -2842,14 +2842,13 @@ static void get_item_key(void *item, int *key)
> >  	key[1] = hash >> BLOOM_FILTER_SHIFT;
> >  }
> >  
> > -static bool test_bloom_filter(struct lru_gen_mm_state *mm_state, unsigned long seq,
> > -			      void *item)
> > +static bool __test_bloom_filter(unsigned long **filters, unsigned long seq, void *item)
> >  {
> >  	int key[2];
> >  	unsigned long *filter;
> >  	int gen = filter_gen_from_seq(seq);
> >  
> > -	filter = READ_ONCE(mm_state->filters[gen]);
> > +	filter = READ_ONCE(filters[gen]);
> >  	if (!filter)
> >  		return true;
> >  
> > @@ -2858,14 +2857,13 @@ static bool test_bloom_filter(struct lru_gen_mm_state *mm_state, unsigned long s
> >  	return test_bit(key[0], filter) && test_bit(key[1], filter);
> >  }
> >  
> > -static void update_bloom_filter(struct lru_gen_mm_state *mm_state, unsigned long seq,
> > -				void *item)
> > +static void __update_bloom_filter(unsigned long **filters, unsigned long seq, void *item)
> >  {
> >  	int key[2];
> >  	unsigned long *filter;
> >  	int gen = filter_gen_from_seq(seq);
> >  
> > -	filter = READ_ONCE(mm_state->filters[gen]);
> > +	filter = READ_ONCE(filters[gen]);
> >  	if (!filter)
> >  		return;
> >  
> > @@ -2877,12 +2875,12 @@ static void update_bloom_filter(struct lru_gen_mm_state *mm_state, unsigned long
> >  		set_bit(key[1], filter);
> >  }
> >  
> > -static void reset_bloom_filter(struct lru_gen_mm_state *mm_state, unsigned long seq)
> > +static void __reset_bloom_filter(unsigned long **filters, unsigned long seq)
> >  {
> >  	unsigned long *filter;
> >  	int gen = filter_gen_from_seq(seq);
> >  
> > -	filter = mm_state->filters[gen];
> > +	filter = filters[gen];
> >  	if (filter) {
> >  		bitmap_clear(filter, 0, BIT(BLOOM_FILTER_SHIFT));
> >  		return;
> > @@ -2890,7 +2888,24 @@ static void reset_bloom_filter(struct lru_gen_mm_state *mm_state, unsigned long
> >  
> >  	filter = bitmap_zalloc(BIT(BLOOM_FILTER_SHIFT),
> >  			       __GFP_HIGH | __GFP_NOMEMALLOC | __GFP_NOWARN);
> > -	WRITE_ONCE(mm_state->filters[gen], filter);
> > +	WRITE_ONCE(filters[gen], filter);
> 
> So the helpers will be something like
> test_bloom_filter(mm_state, level, ...), seems a bit cleaner?

Yes, it makes code much cleaner. I wil use it in v3. Thanks again.

Thanks
Baoquan