All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ridong Chen <ridong.chen@linux.dev>
To: Barry Song <baohua@kernel.org>
Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, david@kernel.org,
	mhocko@kernel.org, qi.zheng@linux.dev, shakeel.butt@linux.dev,
	ljs@kernel.org, kasong@tencent.com, axelrasmussen@google.com,
	yuanchu@google.com, weixugc@google.com,
	hezhongkun.hzk@bytedance.com, muchun.song@linux.dev,
	dave@stgolabs.net, roman.gushchin@linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, Ridong Chen <chenridong@xiaomi.com>
Subject: Re: [PATCH v2 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
Date: Mon, 20 Jul 2026 12:42:32 +0800	[thread overview]
Message-ID: <bcace78f-987f-4908-9e65-bf0b9a52855c@linux.dev> (raw)
In-Reply-To: <CAGsJ_4yYEaoiTY0vFtqJK5fsXZW2PTkMu-qZ=X7=cqmMVa3YCg@mail.gmail.com>



On 7/18/2026 9:34 PM, Barry Song wrote:
> On Sat, Jul 18, 2026 at 5:54 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> As Qi mentioned [1], when swappiness=max (SWAPPINESS_ANON_ONLY) is set,
>> the reclaim logic is expected to reclaim anonymous pages exclusively.
>> However, due to the current ordering of checks in get_scan_count(),
>> file pages may still be evicted if can_reclaim_anon_pages() returns
>> false, which contradicts the semantics of SWAPPINESS_ANON_ONLY.
>>
>> Reproducer in a cgroup holding 64M of file cache, with no swap configured:
>>
>>    Before (file cache is wrongly evicted):
>>      # cat memory.stat
>>      anon 196608
>>      file 67178496
>>      pgscan_proactive 0
>>      # echo "64M swappiness=max" > memory.reclaim
>>      # cat memory.stat
>>      anon 208896
>>      file 4096                 <- page cache evicted
>>      pgsteal_proactive 16400
>>      pgscan_proactive 16400
>>
>>    After (file cache is left intact):
>>      # cat memory.stat
>>      anon 200704
>>      file 67178496
>>      pgscan_proactive 0
>>      # echo "64M swappiness=max" > memory.reclaim
>>      -bash: echo: write error: Resource temporarily unavailable
>>      # cat memory.stat
>>      anon 208896
>>      file 67178496             <- page cache untouched
>>      pgsteal_proactive 0
>>      pgscan_proactive 0
>>
>> Fix this by bailing out early when SWAPPINESS_ANON_ONLY is set and no
>> anonymous pages are reclaimable, before falling back to file reclaim.
>>
>> [1] https://lore.kernel.org/cgroups/7ddf3eee-5fe2-45f7-8614-c8936a039e04@linux.dev/
>>
>> Fixes: 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")
>> Suggested-by: Qi Zheng <qi.zheng@linux.dev>
>> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
>> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>> Reviewed-by: Muchun Song <muchun.song@linux.dev>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> 
> Reviewed-by: Barry Song <baohua@kernel.org>
> 
>> ---
>>   mm/vmscan.c | 18 +++++++++++-------
>>   1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 35c3bb15ae96..d6b383d96a0c 100644
>> --- 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.

-- 
Best regards
Ridong



  reply	other threads:[~2026-07-20  4:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-18  9:52 [PATCH v2 0/4] mm/vmscan: fix swappiness=max and clean up per-node proactive reclaim Ridong Chen
2026-07-18  9:52 ` [PATCH v2 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max Ridong Chen
2026-07-18 13:34   ` Barry Song
2026-07-20  4:42     ` Ridong Chen [this message]
2026-07-21  7:45       ` Barry Song
2026-07-20  2:27   ` Qi Zheng
2026-07-18  9:52 ` [PATCH v2 2/4] mm: vmscan: propagate real error code from per-node proactive reclaim Ridong Chen
2026-07-20  2:32   ` Qi Zheng
2026-07-21  7:32   ` Barry Song
2026-07-18  9:52 ` [PATCH v2 3/4] mm: vmscan: drop unused gfp_mask parameter from __node_reclaim() Ridong Chen
2026-07-18 13:12   ` Barry Song
2026-07-20  2:34   ` Qi Zheng
2026-07-18  9:52 ` [PATCH v2 4/4] mm/mglru: fix anon-only reclaim evicting file pages when swappiness=max Ridong Chen
2026-07-18 13:43   ` Barry Song
2026-07-20  2:48     ` Qi Zheng
2026-07-20  6:03       ` Ridong Chen

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=bcace78f-987f-4908-9e65-bf0b9a52855c@linux.dev \
    --to=ridong.chen@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=baohua@kernel.org \
    --cc=chenridong@xiaomi.com \
    --cc=dave@stgolabs.net \
    --cc=david@kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=hezhongkun.hzk@bytedance.com \
    --cc=kasong@tencent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=muchun.song@linux.dev \
    --cc=qi.zheng@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=weixugc@google.com \
    --cc=yuanchu@google.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.