Re: [PATCH 1/8] mm/mglru: consolidate common code for retrieving evitable size

[Barry Song]

On Wed, Mar 18, 2026 at 3:11 AM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
>
> From: Kairui Song <kasong@xxxxxxxxxxx>
>
> Merge commonly used code for counting evictable folios in a lruvec.
>
> No behavior change.
>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>

Reviewed-by: Barry Song <baohua@xxxxxxxxxx>

[...]
>  static long get_nr_to_scan(struct lruvec *lruvec, struct scan_control *sc, int swappiness)
>  {
> -       bool success;
> +       bool need_aging;

Nice! Many times, I’ve been in the process of submitting a patch
to rename this `success`, as its current name is completely
unreadable and unclear in meaning.

Another `success`also needs some cleanup.
I mean this one:

static int shrink_one(struct lruvec *lruvec, struct scan_control *sc)
{
        bool success;
         ...
         success = try_to_shrink_lruvec(lruvec, sc);
}

yet:

static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
{
        ...
        /* whether this lruvec should be rotated */
        return nr_to_scan < 0;
}

I really can't see the connection between "should be rotated"
and "success".

>         unsigned long nr_to_scan;
>         struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>         DEFINE_MAX_SEQ(lruvec);
> @@ -4962,7 +4954,7 @@ static long get_nr_to_scan(struct lruvec *lruvec, struct scan_control *sc, int s
>         if (mem_cgroup_below_min(sc->target_mem_cgroup, memcg))
>                 return -1;
>
> -       success = should_run_aging(lruvec, max_seq, swappiness, &nr_to_scan);
> +       need_aging = should_run_aging(lruvec, max_seq, swappiness, &nr_to_scan);
>
>         /* try to scrape all its memory if this memcg was deleted */
>         if (nr_to_scan && !mem_cgroup_online(memcg))
> @@ -4971,7 +4963,7 @@ static long get_nr_to_scan(struct lruvec *lruvec, struct scan_control *sc, int s
>         nr_to_scan = apply_proportional_protection(memcg, sc, nr_to_scan);
>
>         /* try to get away with not aging at the default priority */
> -       if (!success || sc->priority == DEF_PRIORITY)
> +       if (!need_aging || sc->priority == DEF_PRIORITY)
>                 return nr_to_scan >> sc->priority;
>
>         /* stop scanning this lruvec as it's low on cold folios */
>
Thanks
Barry