From: Baolin Wang <baolin.wang@linux.alibaba.com>
To: "Barry Song (Xiaomi)" <baohua@kernel.org>
Cc: akpm@linux-foundation.org, axelrasmussen@google.com,
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 19:26:25 +0800 [thread overview]
Message-ID: <fc302ca3-6bf6-4bde-b3e8-8d2e93a4e4ac@linux.alibaba.com> (raw)
In-Reply-To: <20261022102726.56682-1-baohua@kernel.org>
On 10/22/26 6:27 PM, Barry Song (Xiaomi) wrote:
> 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.
Ah, yes, just a quick sample to show what I had in mind. :). Sould also
consider reclaimable_anon_is_low() for the GFP_NOIO case.
>> 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.
No, we have real use cases where 'vm_swappiness' is set to 0, which
means the workload never reclaims anon folios. So in this case we don't
need to age anon folios.
> 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?
OK, still look good to me.
> 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;
I've considered this, but we'd still need to add some code in
get_nr_to_scan()/lruvec_is_sizable() to filter out the anon folios for
GFP_NOIO case. Rather than spreading the logic across multiple places,
I'd prefer adding a helper with proper comments to handle the aging
issue, like the sample code I showed.
prev parent reply other threads:[~2026-10-22 11:26 UTC|newest]
Thread overview: 6+ 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)
2026-10-22 11:26 ` Baolin Wang [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=fc302ca3-6bf6-4bde-b3e8-8d2e93a4e4ac@linux.alibaba.com \
--to=baolin.wang@linux.alibaba.com \
--cc=akpm@linux-foundation.org \
--cc=axelrasmussen@google.com \
--cc=baohua@kernel.org \
--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®