Re: [External] Re: [PATCH] mm: add swappiness=max arg to memory.reclaim for only anon reclaim

[Yosry Ahmed]

On Wed, Mar 19, 2025 at 10:34:54AM +0800, Zhongkun He wrote:
> On Tue, Mar 18, 2025 at 10:10 PM Yosry Ahmed <yosry.ahmed@xxxxxxxxx> wrote:
> >
> > On Tue, Mar 18, 2025 at 09:53:30PM +0800, Zhongkun He wrote:
> > > With this patch 'commit <68cd9050d871> ("mm: add swappiness= arg to
> > > memory.reclaim")', we can submit an additional swappiness=<val> argument
> > > to memory.reclaim. It is very useful because we can dynamically adjust
> > > the reclamation ratio based on the anonymous folios and file folios of
> > > each cgroup. For example,when swappiness is set to 0, we only reclaim
> > > from file folios.
> > >
> > > However,we have also encountered a new issue: when swappiness is set to
> > > the MAX_SWAPPINESS, it may still only reclaim file folios.
> > >
> > > So, we hope to add a new arg 'swappiness=max' in memory.reclaim where
> > > proactive memory reclaim only reclaims from anonymous folios when
> > > swappiness is set to max. The swappiness semantics from a user
> > > perspective remain unchanged.
> > >
> > > For example, something like this:
> > >
> > > echo "2M swappiness=max" > /sys/fs/cgroup/memory.reclaim
> > >
> > > will perform reclaim on the rootcg with a swappiness setting of 'max' (a
> > > new mode) regardless of the file folios. Users have a more comprehensive
> > > view of the application's memory distribution because there are many
> > > metrics available. For example, if we find that a certain cgroup has a
> > > large number of inactive anon folios, we can reclaim only those and skip
> > > file folios, because with the zram/zswap, the IO tradeoff that
> > > cache_trim_mode or other file first logic is making doesn't hold -
> > > file refaults will cause IO, whereas anon decompression will not.
> > >
> > > With this patch, the swappiness argument of memory.reclaim has a new
> > > mode 'max', means reclaiming just from anonymous folios both in traditional
> > > LRU and MGLRU.
> >
> > Is MGLRU handled in this patch?
> 
> Yes, The value of ONLY_ANON_RECLAIM_MODE is 201, and the MGLRU select the
> evictable type like this:
> 
> #define evictable_min_seq(min_seq, swappiness)              \
>     min((min_seq)[!(swappiness)], (min_seq)[(swappiness) <= MAX_SWAPPINESS])
> 
> #define for_each_evictable_type(type, swappiness)           \
>     for ((type) = !(swappiness); (type) <= ((swappiness) <=
> MAX_SWAPPINESS); (type)++)
> 
> if the swappiness=0, the type is LRU_GEN_FILE(1);
> 
> if the swappiness=201 (>MAX_SWAPPINESS),
>   for ((type) = 0; (type) <= 0); (type)++)
> The type is always LRU_GEN_ANON(0).

Zhongkun, I see that you already sent a new version. Please wait until
discussions on a patch are resolved before sending out newer versions,
and allow more time for reviews in general.

I think this is too subtle, and it's easy to miss. Looking at the MGLRU
code it seems like there's a lot of swappiness <= MAX_SWAPPINESS checks,
and I am not sure why these already exist given that swappiness should
never exceed MAX_SWAPPINESS before this change.

Are there other parts of the MGLRU code that are already using
swappiness values > MAX_SWAPPINESS?

Yu, could you help us making things clearer here? I would like to avoid
relying on current implementation details that could easily be missed
when making changes. Ideally we'd explicitly check for
SWAPPINESS_ANON_ONLY.