Re: [PATCH RFC 12/15] mm/smap: report workingset folios as referenced

On Mon, Aug 03, 2026 at 09:21:33PM +0800, Johannes Weiner wrote:
> On Tue, Aug 04, 2026 at 03:47:08AM +0800, Kairui Song via B4 Relay wrote:
> > @@ -944,6 +944,22 @@ static void smaps_page_accumulate(struct mem_size_stats *mss,
> >  	}
> >  }
> >  
> > +static bool smap_check_folio_referenced(struct folio *folio)
> > +{
> > +	if (lru_gen_enabled())
> > +		return folio_is_referenced(folio);
> > +	else
> > +		return folio_is_referenced_by_bit(folio);
> > +}
> > +
> > +static void smap_clear_folio_referenced(struct folio *folio)
> > +{
> > +	if (lru_gen_enabled())
> > +		folio_set_lru_refs(folio, 0);
> > +	else
> > +		folio_clear_referenced_by_bit(folio);
> > +}
> 
> I don't really understand this and the preceding 2 patches.
> 
> What is the benefit of a "shared" ref/workingset state implementation
> when you then still have separate APIs for querying & modifying?
> 
> Switching some these callers from folio_test_workingset() to
> folio_is_workingset()? But both functions continue to exist?
> 
> Why not make versions of folio_test_workingset(),
> folio_test_referenced(), folio_clear_referenced() etc. that do the
> arbitration (bit ops on classic, refs ops on mglru) and leave callers
> that are only interested in boolean states, like here, alone?
> 
> folio_test_anon(), folio_test_lazyfree(), folio_test_swapcache() -
> there are many examples where they don't just test a raw bit but
> assess more complex, composed state.
> 

Sound good, I just copied the folio_is/mark_workingset helper from
my previous combined RFC, where I tried to drop PG_workingset hence
used a standalone helper to convert the users one by one for easier
audit and tracking.

I think I can just replace the default helpers and have zero effect
for CLRU. There are actually a few more helpers not used but introduced
here, will drop these in V2. Thanks for the review!