All of lore.kernel.org
 help / color / mirror / Atom feed
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.


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

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 15:18 [RFC PATCH v2] mm: vmscan: avoid anon scanning for GFP_NOIO with low swapcache in MGLRU 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]
2026-10-22 21:19           ` Barry Song
2026-10-23  1:36             ` Baolin Wang
2026-10-23  1:40               ` Barry Song
2026-10-27  5:14                 ` Xiang Liu

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.