mm/memcontrol.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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 leads to incorrect reclaim behavior. Specifically,
MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit
is reached. After reclaimation attemps, a subsequent retry may
successfully charge memcg->memsw but fail on the memcg->memory charge.
In this case, swapping should be permitted, but the carried-over state
prevents it.
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>
Reviewed-by: Yosry Ahmed <yosry@kernel.org>
---
v2:
- Dropped other patches.
- Refined commit message to clarify the impact of the leak (Yosry).
- Added Reviewed-by tag.
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-googOn Wed, Mar 18, 2026 at 2:56 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 leads to incorrect reclaim behavior. Specifically,
> MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit
> is reached. After reclaimation attemps, a subsequent retry may
> successfully charge memcg->memsw but fail on the memcg->memory charge.
> In this case, swapping should be permitted, but the carried-over state
> prevents it.
>
> 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")
The Fixes tag is still wrong, see my previous reply.
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 leads to incorrect reclaim behavior. Specifically,
MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit
is reached. After reclaimation attemps, a subsequent retry may
successfully charge memcg->memsw but fail on the memcg->memory charge.
In this case, swapping should be permitted, but the carried-over state
prevents it.
Fix by moving the initialization of 'reclaim_options' inside the
retry loop, ensuring a clean state for every reclaim attempt.
Fixes: 6539cc053869 ("mm: memcontrol: fold mem_cgroup_do_charge()")
Signed-off-by: Bing Jiao <bingjiao@google.com>
Reviewed-by: Yosry Ahmed <yosry@kernel.org>
---
v3:
- Corrected the Fixes tag (Yosry).
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-googOn Wed 18-03-26 22:19:46, Bing Jiao 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 leads to incorrect reclaim behavior. Specifically,
> MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit
> is reached. After reclaimation attemps, a subsequent retry may
> successfully charge memcg->memsw but fail on the memcg->memory charge.
> In this case, swapping should be permitted, but the carried-over state
> prevents it.
Have you noticed this happening in practice or is this based on the code
reading?
> Fix by moving the initialization of 'reclaim_options' inside the
> retry loop, ensuring a clean state for every reclaim attempt.
>
> Fixes: 6539cc053869 ("mm: memcontrol: fold mem_cgroup_do_charge()")
> Signed-off-by: Bing Jiao <bingjiao@google.com>
> Reviewed-by: Yosry Ahmed <yosry@kernel.org>
Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!
> ---
> v3:
> - Corrected the Fixes tag (Yosry).
>
> 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
--
Michal Hocko
SUSE Labs
On Thu, Mar 19, 2026 at 10:29:15AM +0100, Michal Hocko wrote: > On Wed 18-03-26 22:19:46, Bing Jiao 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 leads to incorrect reclaim behavior. Specifically, > > MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit > > is reached. After reclaimation attemps, a subsequent retry may > > successfully charge memcg->memsw but fail on the memcg->memory charge. > > In this case, swapping should be permitted, but the carried-over state > > prevents it. > > Have you noticed this happening in practice or is this based on the code > reading? Hi, Michal, thanks for the ack. This issue was identified during code reading, when I was analyzing the memsw limit behavior in try_charge_memcg(); specifically how retries are handled when demotion is disabled (the demotion patch itself was dropped). Best, Bing
On Fri 20-03-26 03:39:40, Bing Jiao wrote: > On Thu, Mar 19, 2026 at 10:29:15AM +0100, Michal Hocko wrote: > > On Wed 18-03-26 22:19:46, Bing Jiao 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 leads to incorrect reclaim behavior. Specifically, > > > MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit > > > is reached. After reclaimation attemps, a subsequent retry may > > > successfully charge memcg->memsw but fail on the memcg->memory charge. > > > In this case, swapping should be permitted, but the carried-over state > > > prevents it. > > > > Have you noticed this happening in practice or is this based on the code > > reading? > > Hi, Michal, thanks for the ack. > > This issue was identified during code reading, when I was analyzing > the memsw limit behavior in try_charge_memcg(); specifically how > retries are handled when demotion is disabled (the demotion patch > itself was dropped). OK, that is always good to clarify in the changelog. -- Michal Hocko SUSE Labs
On Wed, Mar 18, 2026 at 10:19:46PM +0000, Bing Jiao 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 leads to incorrect reclaim behavior. Specifically,
> MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit
> is reached. After reclaimation attemps, a subsequent retry may
> successfully charge memcg->memsw but fail on the memcg->memory charge.
> In this case, swapping should be permitted, but the carried-over state
> prevents it.
>
> Fix by moving the initialization of 'reclaim_options' inside the
> retry loop, ensuring a clean state for every reclaim attempt.
>
> Fixes: 6539cc053869 ("mm: memcontrol: fold mem_cgroup_do_charge()")
> Signed-off-by: Bing Jiao <bingjiao@google.com>
> Reviewed-by: Yosry Ahmed <yosry@kernel.org>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
On Wed, Mar 18, 2026 at 10:19:46PM +0000, Bing Jiao 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 leads to incorrect reclaim behavior. Specifically,
> MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit
> is reached. After reclaimation attemps, a subsequent retry may
> successfully charge memcg->memsw but fail on the memcg->memory charge.
> In this case, swapping should be permitted, but the carried-over state
> prevents it.
>
> Fix by moving the initialization of 'reclaim_options' inside the
> retry loop, ensuring a clean state for every reclaim attempt.
>
> Fixes: 6539cc053869 ("mm: memcontrol: fold mem_cgroup_do_charge()")
> Signed-off-by: Bing Jiao <bingjiao@google.com>
> Reviewed-by: Yosry Ahmed <yosry@kernel.org>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
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 leads to incorrect reclaim behavior. Specifically,
MEMCG_RECLAIM_MAY_SWAP is cleared when the combined memcg->memsw limit
is reached. After reclaimation attemps, a subsequent retry may
successfully charge memcg->memsw but fail on the memcg->memory charge.
In this case, swapping should be permitted, but the carried-over state
prevents it.
This issue was identified during code reading of try_charge_memcg()
while analyzing memsw limit behavior in tiered-memory systems;
no production failures have been reported yet.
Fix by moving the initialization of 'reclaim_options' inside the
retry loop, ensuring a clean state for every reclaim attempt.
Fixes: 6539cc053869 ("mm: memcontrol: fold mem_cgroup_do_charge()")
Signed-off-by: Bing Jiao <bingjiao@google.com>
Reviewed-by: Yosry Ahmed <yosry@kernel.org>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
---
v4:
- Clarify in the commit message that the issue was found via
code reading (Michal).
- Add ACKs (Michal and Johannes).
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.959.g497ff81fa9-goog© 2016 - 2026 Red Hat, Inc.