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

[Barry Song]

On Thu, Jul 23, 2026 at 10:00 AM Ridong Chen <ridong.chen@xxxxxxxxx> wrote:
>
>
>
> On 7/21/2026 3:45 PM, Barry Song wrote:
> > 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.
> >
> Eh, shrink_node handles MGLRU global reclaim via a special branch, so we'd have
> to add the warning at the beginning of shrink_node, but that feels a bit
> jarring. Let's just go with adding some comments instead.

Yep. I also failed to find the best place to add the WARN_ON earlier in
a common path shared by both MGLRU and the active/inactive LRU. Maybe
just adding some more comments would help clarify the intent :-)

Best Regards
Barry