Re: [PATCH v2 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max

[Barry Song]

On Mon, Jul 20, 2026 at 12:43 PM Ridong Chen <ridong.chen@xxxxxxxxx> wrote:
[...]

> >> --- a/mm/vmscan.c
> >> +++ b/mm/vmscan.c
> >> @@ -2501,6 +2501,17 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
> >>          enum scan_balance scan_balance;
> >>          enum lru_list lru;
> >>
> >> +       /* Proactive reclaim initiated by userspace for anonymous memory only */
> >> +       if (swappiness == SWAPPINESS_ANON_ONLY) {
> >> +               WARN_ON_ONCE(!sc->proactive);
> >
> > I feel it's a bit odd that the WARN_ON_ONCE() is placed here. Maybe
> > adding a short comment would make the intent clearer. For example:
> > "SWAPPINESS_ANON_ONLY is only allowed for proactive reclaim."
>
> Hi Barry,
>
> I didn't change the original logic in this patch. I agree that adding a
> comment would help clarify the intent.
>
> I'll include that in the next version.
>
> > I'm not sure whether it would be better to issue this warning earlier,
> > for example in try_to_free_mem_cgroup_pages(). That said, I don't
> > feel strongly about this.
>
> Since patch 4 will address the MGLRU issue and could also include this
> warning, I think that triggering the warning earlier makes sense and
> would be more general. Would it be acceptable to add a separate patch
> (e.g., patch 5) for moving the warning? That way, each patch stays
> focused on a single logical change.

I'm not quite sure where the best place would be to move it earlier. We
already have two places that perform proactive reclaim:
try_to_free_mem_cgroup_pages() and __node_reclaim(). There are also
other reclaim paths that are not proactive. Since they all eventually call
shrink_node(), would it make more sense to handle it there?

If we can't find a suitable common path, I guess simply adding some comments
to the current patches would be good enough.

Thanks
Barry