mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Barry Song (Xiaomi)" <baohua@kernel.org>
To: baolin.wang@linux.alibaba.com
Cc: akpm@linux-foundation.org, axelrasmussen@google.com,
	baohua@kernel.org, baoquan.he@linux.dev, david@kernel.org,
	hannes@cmpxchg.org, kasong@tencent.com,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	liuxiang.277@bytedance.com, ljs@kernel.org, mhocko@kernel.org,
	qi.zheng@linux.dev, shakeel.butt@linux.dev, weixugc@google.com,
	yuanchu@google.com, zhangbo56@xiaomi.com
Subject: Re: [RFC PATCH v2] mm: vmscan: avoid anon scanning for GFP_NOIO with low swapcache in MGLRU
Date: Thu, 22 Oct 2026 18:27:26 +0800	[thread overview]
Message-ID: <20261022102726.56682-1-baohua@kernel.org> (raw)
In-Reply-To: <0cbd219d-f69d-4f72-aec5-43e1f2d978de@linux.alibaba.com>

On Thu, Oct 22, 2026 at 4:49 PM Baolin Wang <baolin.wang@linux.alibaba.com> wrote:
>
>
>
> On 10/22/26 5:55 AM, Barry Song wrote:
> > On Wed, Oct 21, 2026 at 3:28 PM Baolin Wang
> > <baolin.wang@linux.alibaba.com> wrote:
> > [...]
> >
> >>>    static bool lruvec_is_sizable(struct lruvec *lruvec, struct scan_control *sc)
> >>>    {
> >>>        unsigned long total;
> >>>        int swappiness = get_swappiness(lruvec, sc);
> >>>        struct mem_cgroup *memcg = lruvec_memcg(lruvec);
> >>>
> >>> +     swappiness = get_scan_swappiness(lruvec, sc, swappiness);
> >>> +
> >>>        total = lruvec_evictable_size(lruvec, swappiness);
> >>>
> >>>        /* whether the size is big enough to be helpful */
> >>> @@ -5249,9 +5265,10 @@ static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
> >>>        bool need_rotate = false, should_age = false;
> >>>        long nr_batch, nr_to_scan;
> >>>        int swappiness = get_swappiness(lruvec, sc);
> >>> +     int scan_swappiness = get_scan_swappiness(lruvec, sc, swappiness);
> >>
> >> IMO, 'swappiness' and 'scan_swappiness' are really confusing. Why move
> >> this logic into get_swappiness()? In get_swappiness(), we already have
> >> some similar logic to adjust swappiness when there is no available swap
> >> space.
> >>
> >
> > Yes. It is very confusing. we can't do that in get_swappiness() as otherwise,
> > we will get cold/hot inversion due to GFP_NOIO. for a real swappiness=0,
> > we allow cold/hot inversion for anon, but for GFP_NOIO, this is not the
> > case.
>
> But can_reclaim_anon_pages() doesn't only handle the GFP_NOIO case, it
> also covers the swap space limitation scenario. The latter is what
> concerns me more.
>
> If we're only talking about the GFP_NOIO case, I'd agree with your
> concern. However, we could introduce a helper to force anon aging and
> add comments explaining why we need this. That would look more
> maintainable than 2 confusing swappiness variables, no?
>
> Just my 2 cents.
>
> Something like below (untested):
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index daa7e37c2236..7c1d26e6b9d7 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2890,7 +2890,7 @@ static int get_swappiness(struct lruvec *lruvec,
> struct scan_control *sc)
>          if (swappiness == SWAPPINESS_ANON_ONLY)
>                  return swappiness;
>
> -       if (!sc->may_swap)
> +       if (!sc->may_swap || !(sc->gfp_mask & __GFP_IO))
>                  return 0;

For GFP_NOIO, we should also check whether swapcache is low, since
GFP_NOIO can reclaim swapcache. We already have
reclaimable_anon_is_low() for this.

>
>          if (!can_demote(pgdat->node_id, sc, memcg) &&
> @@ -4049,7 +4049,7 @@ static inline void flush_lru_batch(struct
> list_head *head, struct list_head **ba
>          *batch_end = NULL;
>   }
>
> -static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
> +static bool inc_min_seq(struct lruvec *lruvec, int type, int
> swappiness, bool force_anon_aging)
>   {
>          int zone;
>          int remaining = MAX_LRU_BATCH;
> @@ -4063,7 +4063,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int
> type, int swappiness)
>                  goto done;
>
>          /* For anon type, skip the check if swappiness is zero (file
> only) */
> -       if (!type && !swappiness)
> +       if (!type && !swappiness && !force_anon_aging)
>                  goto done;
>
>          VM_WARN_ON_ONCE(get_nr_gens(lruvec, type) != MAX_NR_GENS);
> @@ -4183,7 +4183,20 @@ static void try_to_inc_min_seq(struct lruvec
> *lruvec, int swappiness)
>          }
>   }
>
> -static bool inc_max_seq(struct lruvec *lruvec, unsigned long seq, int
> swappiness)
> +static bool is_force_anon_aging(struct scan_control *sc, struct lruvec
> *lruvec,
> +               int swappiness)
> +{
> +       if (swappiness || (sc->gfp_mask & __GFP_IO))
> +               return false;

As above, GFP_NOIO doesn't imply swappiness=0 if there are many
swapcache folios to reclaim. So we should check
reclaimable_anon_is_low() here.

> +
> +       /*
> +        * Explain why we neeed to force increase anon min seq.
> +        * xxxxxx
> +        */
> +       return swappiness != sc_swappiness(sc, lruvec_memcg(lruvec));
> +}
> +
> +static bool inc_max_seq(struct lruvec *lruvec, unsigned long seq, int
> swappiness, bool force_anon_aging)
>   {
>          bool success;
>          int prev, next;
> @@ -4205,7 +4218,7 @@ static bool inc_max_seq(struct lruvec *lruvec,
> unsigned long seq, int swappiness
>                  if (get_nr_gens(lruvec, type) != MAX_NR_GENS)
>                          continue;
>
> -               if (inc_min_seq(lruvec, type, swappiness))
> +               if (inc_min_seq(lruvec, type, swappiness, force_anon_aging))
>                          continue;
>
>                  lruvec_unlock_irq(lruvec);
> @@ -4249,7 +4262,7 @@ static bool inc_max_seq(struct lruvec *lruvec,
> unsigned long seq, int swappiness
>   }
>
>   static bool try_to_inc_max_seq(struct lruvec *lruvec, unsigned long seq,
> -                              int swappiness, bool force_scan)
> +                              int swappiness, bool force_scan, bool
> force_anon_aging)
>   {
>          bool success;
>          struct lru_gen_mm_walk *walk;
> @@ -4260,7 +4273,7 @@ static bool try_to_inc_max_seq(struct lruvec
> *lruvec, unsigned long seq,
>          VM_WARN_ON_ONCE(seq > READ_ONCE(lrugen->max_seq));
>
>          if (!mm_state)
> -               return inc_max_seq(lruvec, seq, swappiness);
> +               return inc_max_seq(lruvec, seq, swappiness,
> force_anon_aging);
>
>          /* see the comment in iterate_mm_list() */
>          if (seq <= READ_ONCE(mm_state->seq))
> @@ -4295,7 +4308,7 @@ static bool try_to_inc_max_seq(struct lruvec
> *lruvec, unsigned long seq,
>          } while (mm);
>   done:
>          if (success) {
> -               success = inc_max_seq(lruvec, seq, swappiness);
> +               success = inc_max_seq(lruvec, seq, swappiness,
> force_anon_aging);
>                  WARN_ON_ONCE(!success);
>          }
>
> @@ -5262,7 +5275,9 @@ static bool try_to_shrink_lruvec(struct lruvec
> *lruvec, struct scan_control *sc)
>                  }
>
>                  if (should_run_aging(lruvec, max_seq, sc, swappiness)) {
> -                       if (try_to_inc_max_seq(lruvec, max_seq,
> swappiness, false))

This is quite tricky. If we pass swappiness=0, anon aging will be
skipped. Do we want that for GFP_NOIO cases? I mean, these are not
real cases where anon aging should be disabled.

static int should_skip_vma(unsigned long start, unsigned long end, struct mm_walk *args)
{
        ...  
        if (vma == get_gate_vma(vma->vm_mm))
                return true;
        ...  
}

> +                       bool force_anon_aging = is_force_anon_aging(sc,
> lruvec, swappiness);

Adding a force_anon_aging argument to inc_max_seq() seems like
quite a burden. Maybe a cleaner approach is to pass sc all the
way to inc_max_seq() and let it detect GFP_NOIO cases itself?

BTW, I guess we don't need to worry about swappiness at all. We can
simply bypass anon reclaim?

diff --git a/mm/vmscan.c b/mm/vmscan.c
index fc823fe3bd1f..fa6d9860156d 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4903,6 +4903,8 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	int skipped = 0;
 	unsigned long remaining = nr_to_scan;
 	struct lru_gen_folio *lrugen = &lruvec->lrugen;
+	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
+	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
 
 	VM_WARN_ON_ONCE(nr_to_scan > MAX_LRU_BATCH);
 	VM_WARN_ON_ONCE(!list_empty(list));
@@ -4910,6 +4912,9 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	if (get_nr_gens(lruvec, type) == MIN_NR_GENS)
 		return 0;
 
+	if (!type && reclaimable_anon_is_low(memcg, pgdat->node_id, sc))
+		return 0;
+
 	gen = lru_gen_from_seq(lrugen->min_seq[type]);
 
 	for (i = MAX_NR_ZONES; i > 0; i--) {
@@ -5164,12 +5169,19 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 static bool should_run_aging(struct lruvec *lruvec, unsigned long max_seq,
 			     struct scan_control *sc, int swappiness)
 {
+	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
+	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
 	DEFINE_MIN_SEQ(lruvec);
 
 	/* have to run aging, since eviction is not possible anymore */
 	if (evictable_min_seq(min_seq, swappiness) + MIN_NR_GENS > max_seq)
 		return true;
 
+	/* run aging when GFP_NOIO limits anon reclaim with few swapcache folios */
+	if (swappiness && min_seq[LRU_GEN_FILE] + MIN_NR_GENS > max_seq &&
+	    reclaimable_anon_is_low(memcg, pgdat->node_id, sc))
+		return true;
+
 	/* try to avoid aging, do gentle reclaim at the default priority */
 	if (sc->priority == DEF_PRIORITY)
 		return false;


      reply	other threads:[~2026-10-22 10:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 15:18 Xiang Liu
2026-10-21  7:28 ` Baolin Wang
2026-10-21 21:55   ` Barry Song
2026-10-22  8:49     ` Baolin Wang
2026-10-22 10:27       ` Barry Song (Xiaomi) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261022102726.56682-1-baohua@kernel.org \
    --to=baohua@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=baoquan.he@linux.dev \
    --cc=david@kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kasong@tencent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=liuxiang.277@bytedance.com \
    --cc=ljs@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=qi.zheng@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=weixugc@google.com \
    --cc=yuanchu@google.com \
    --cc=zhangbo56@xiaomi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®