Re: [PATCH bpf-next v12 1/2] mm/bpf: Add bpf_proactive_reclaim kfunc

[Barry Song]

On Sun, Sep 20, 2026 at 6:36 AM Barry Song <baohua@xxxxxxxxxx> wrote:
[...]
> > > Related to the previous comment, you treat the special values as part of
> > > the range. It works currently, but creates a layout dependency on the
> > > enum. I think it would be more future-proof if you did:
> > >
> > > if (swappiness != -1 && swappiness != SWAPPINESS_ANON_ONLY) {
> > >       if (swappiness < MIN_SWAPPINESS || swappiness > MAX_SWAPPINESS)
> > >               return -EINVAL;
> > > }
> > >
> > > The previous comments I brought up are now resolved, so assuming you'll
> > > make the changes above you can include:
> > >
> > > Reviewed-by: JP Kobryn <jp.kobryn@xxxxxxxxx>
> >
> > Hui, please make these changes and just send this patch in next version.
>
> Hi JP and Shakeel,
>
> This feels a bit weird to me, since 0–201 are all valid
> swappiness values.
>
> 201 (`SWAPPINESS_ANON_ONLY`) is just as valid a swappiness value
> as 200. In contrast, -1 is a different story, as it indicates
> completely different semantics.
>
> If 201 is treated as a special value, then 0 (`MIN_SWAPPINESS`)
> should be treated the same way for MGLRU, since 0 means file-only
> reclaim there. This isn't the case for CLRU, where swappiness 0
> can still reclaim anon folios.
>
> If possible, I'd vote for keeping Hui's current code :-)

On second thought, you might be right, as there could theoretically be a
case where `SWAPPINESS_ANON_ONLY` is not defined as
`MAX_SWAPPINESS + 1`, but as, for example, 400 or some other value.

That said, I find `MAX_SWAPPINESS` and `SWAPPINESS_ANON_ONLY` really
weird. Somehow, I feel `MAX_SWAPPINESS` should be
`SWAPPINESS_ANON_ONLY`, since 201 is actually the maximum valid
swappiness value, rather than 200.

For the change suggested by JP, I think it is right if we don't clean up
`MAX_SWAPPINESS` and `SWAPPINESS_ANON_ONLY` properly.

With the change suggested by JP,

Reviewed-by: Barry Song <baohua@xxxxxxxxxx>