In try_charge_memcg(), the 'reclaim_options' variable is initialized
once at the start of the function. However, the function contains a
retry loop. If reclaim_options were modified during an iteration
(e.g., by encountering a memsw limit), the modified state would
persist into subsequent retries.

This could lead to incorrect reclaim behavior, such as anon pages
cannot be reclaimed if memsw has quotas after retries.

Fix by moving the initialization of 'reclaim_options' inside the
retry loop, ensuring a clean state for every reclaim attempt.

Fixes: 73b73bac90d9 ("mm: vmpressure: don't count proactive reclaim in vmpressure")
Signed-off-by: Bing Jiao <bingjiao@google.com>
---
 mm/memcontrol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a47fb68dd65f..303ac622d22d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2558,7 +2558,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 	struct page_counter *counter;
 	unsigned long nr_reclaimed;
 	bool passed_oom = false;
-	unsigned int reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
+	unsigned int reclaim_options;
 	bool drained = false;
 	bool raised_max_event = false;
 	unsigned long pflags;
@@ -2572,6 +2572,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 		/* Avoid the refill and flush of the older stock */
 		batch = nr_pages;

+	reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
 	if (!do_memsw_account() ||
 	    page_counter_try_charge(&memcg->memsw, batch, &counter)) {
 		if (page_counter_try_charge(&memcg->memory, batch, &counter))
--
2.53.0.851.ga537e3e6e9-goog

On Tue, Mar 17, 2026 at 4:07 PM Bing Jiao <bingjiao@google.com> wrote:
>
> In try_charge_memcg(), the 'reclaim_options' variable is initialized
> once at the start of the function. However, the function contains a
> retry loop. If reclaim_options were modified during an iteration
> (e.g., by encountering a memsw limit), the modified state would
> persist into subsequent retries.
>
> This could lead to incorrect reclaim behavior, such as anon pages
> cannot be reclaimed if memsw has quotas after retries.
>
> Fix by moving the initialization of 'reclaim_options' inside the
> retry loop, ensuring a clean state for every reclaim attempt.
>
> Fixes: 73b73bac90d9 ("mm: vmpressure: don't count proactive reclaim in vmpressure")

Before this commit, we had the same logic with 'may_swap' being
initialized to true and set to false in the retry loop. Before that,
it was 'flags' and 'MEM_CGROUP_RECLAIM_NOSWAP'.

I think initializing whether to swap or not outside the retry loop
started by commit 6539cc053869 ("mm: memcontrol: fold
mem_cgroup_do_charge()") 12 years ago, so I don't think it's a problem
in practice.

Practically speaking, we clear MEMCG_RECLAIM_MAY_SWAP if we hit the
combined memcg->memsw limit. I guess it's theoretically possible (but
probably unlikely) that we try to charge memcg->memsw, fail, reclaim
and/or OOM, then try again, succeed in charging memcg->memsw, but fail
charging memcg->memory. In this case, we should indeed attempt to
swap.

All that being said, this looks correct with the right 'Fixes' tag:

Reviewed-by: Yosry Ahmed <yosry@kernel.org>

> Signed-off-by: Bing Jiao <bingjiao@google.com>
> ---
>  mm/memcontrol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a47fb68dd65f..303ac622d22d 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2558,7 +2558,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
>         struct page_counter *counter;
>         unsigned long nr_reclaimed;
>         bool passed_oom = false;
> -       unsigned int reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
> +       unsigned int reclaim_options;
>         bool drained = false;
>         bool raised_max_event = false;
>         unsigned long pflags;
> @@ -2572,6 +2572,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
>                 /* Avoid the refill and flush of the older stock */
>                 batch = nr_pages;
>
> +       reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
>         if (!do_memsw_account() ||
>             page_counter_try_charge(&memcg->memsw, batch, &counter)) {
>                 if (page_counter_try_charge(&memcg->memory, batch, &counter))
> --
> 2.53.0.851.ga537e3e6e9-goog
>