mm/memcontrol: fix stuck FLUSHING_CACHED_CHARGE bit on isolated cpus

PATCH Superseded Rik van Riel 3 participants 7 messages Aug 28

Subsystems (4)

Stable branch Memory management cgroup memcg

lore

Share & tools

lore

Rik van Riel

author

2026-08-28 13:46 UTC #0 lore raw

cover mm/memcontrol: fix stuck FLUSHING_CACHED_CHARGE bit on isolated cpus

drain_all_stock() can leave FLUSHING_CACHED_CHARGE set after the work is dropped. It sets the bit before checking isolation and schedule_drain_work() checks isolation and queues in a separate RCU critical section, so housekeeping_update()'s synchronize_rcu() can race the second check.

drain_local_stock() only clears the bit for work that ran, so the bit remains set and the stock is never drained again.

Patch 1 extracts the stock drain predicates into helpers.

Patch 2 reorganizes the drain_all_stock() loop, reducing nesting, splitting out local vs remote cpu handling, and skipping everything on isolated cpus, which solves the stuck FLUSHING_CACHED_CHARGE flag.

Link https://sashiko.dev/#/patchset/20260827124211.3b94b103@fangorn

Rik van Riel (2):
mm/memcontrol: extract stock drain predicates into helpers mm/memcontrol: fix stuck FLUSHING_CACHED_CHARGE bit on isolated cpus

 mm/memcontrol.c | 61 ++++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 26 deletions(-)

--
2.55.0

Rik van Riel

author

2026-08-28 13:46 UTC #1 lore raw

PATCH 1/1 mm/memcontrol: extract stock drain predicates into helpers

Wrap the FLUSHING check plus is_memcg_drain_needed() / obj_stock_flush_required() and the test_and_set into memcg_stock_should_drain() and obj_stock_should_drain().

No functional change.

Cc [email protected]

Assisted-by Hermes:muse-spark-1.2

Signed-off-by Rik van Riel <[email protected]>

mm/memcontrol.c +26 -8

index 1271d390b617..a660ea0f820b 100644
@@ -2306,6 +2306,30 @@ static bool is_memcg_drain_needed(struct memcg_stock_pcp *stock,
 	return flush;
 }
 
+static bool memcg_stock_should_drain(struct memcg_stock_pcp *stock,
+				       struct mem_cgroup *root_memcg)
+{
+	if (test_bit(FLUSHING_CACHED_CHARGE, &stock->flags))
+		return false;
+	if (!is_memcg_drain_needed(stock, root_memcg))
+		return false;
+	if (test_and_set_bit(FLUSHING_CACHED_CHARGE, &stock->flags))
+		return false;
+	return true;
+}
+
+static bool obj_stock_should_drain(struct obj_stock_pcp *stock,
+				  struct mem_cgroup *root_memcg)
+{
+	if (test_bit(FLUSHING_CACHED_CHARGE, &stock->flags))
+		return false;
+	if (!obj_stock_flush_required(stock, root_memcg))
+		return false;
+	if (test_and_set_bit(FLUSHING_CACHED_CHARGE, &stock->flags))
+		return false;
+	return true;
+}
+
 static void schedule_drain_work(int cpu, struct work_struct *work)
 {
 	/*

@@ -2342,20 +2366,14 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 		struct memcg_stock_pcp *memcg_st = &per_cpu(memcg_stock, cpu);
 		struct obj_stock_pcp *obj_st = &per_cpu(obj_stock, cpu);
 
-		if (!test_bit(FLUSHING_CACHED_CHARGE, &memcg_st->flags) &&
-		    is_memcg_drain_needed(memcg_st, root_memcg) &&
-		    !test_and_set_bit(FLUSHING_CACHED_CHARGE,
-				      &memcg_st->flags)) {
+		if (memcg_stock_should_drain(memcg_st, root_memcg)) {
 			if (cpu == curcpu)
 				drain_local_memcg_stock(&memcg_st->work);
 			else
 				schedule_drain_work(cpu, &memcg_st->work);
 		}
 
-		if (!test_bit(FLUSHING_CACHED_CHARGE, &obj_st->flags) &&
-		    obj_stock_flush_required(obj_st, root_memcg) &&
-		    !test_and_set_bit(FLUSHING_CACHED_CHARGE,
-				      &obj_st->flags)) {
+		if (obj_stock_should_drain(obj_st, root_memcg)) {
 			if (cpu == curcpu)
 				drain_local_obj_stock(&obj_st->work);
 			else

Rik van Riel

author

2026-08-28 13:46 UTC #2 lore raw

PATCH 1/1 mm/memcontrol: avoid stuck FLUSHING_CACHED_CHARGE bit on isolated cpus

In reply to Rik van Riel

drain_local_stock() only clears the bit for work that ran, so the bit remains set and the stock is never drained again.

Reorganize the drain_all_stock() loop, reducing nesting, splitting out local vs remote cpu handling, and skipping everything on isolated cpus, which solves the stuck FLUSHING_CACHED_CHARGE flag.

Fixes 6a792697a53a ("memcg: do not drain charge pcp caches on remote isolated cpus")

Cc [email protected]

Assisted-by Hermes:muse-spark-1.2

Signed-off-by Rik van Riel <[email protected]>

mm/memcontrol.c +14 ~~-23~~

index a660ea0f820b..b0d8ec042d48 100644
@@ -2330,19 +2330,6 @@ static bool obj_stock_should_drain(struct obj_stock_pcp *stock,
 	return true;
 }
 
-static void schedule_drain_work(int cpu, struct work_struct *work)
-{
-	/*
-	 * Protect housekeeping cpumask read and work enqueue together
-	 * in the same RCU critical section so that later cpuset isolated
-	 * partition update only need to wait for an RCU GP and flush the
-	 * pending work on newly isolated CPUs.
-	 */
-	guard(rcu)();
-	if (!cpu_is_isolated(cpu))
-		queue_work_on(cpu, memcg_wq, work);
-}
-
 /*
  * Drains all per-CPU charge caches for given root_memcg resp. subtree
  * of the hierarchy under it.

@@ -2366,19 +2353,23 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 		struct memcg_stock_pcp *memcg_st = &per_cpu(memcg_stock, cpu);
 		struct obj_stock_pcp *obj_st = &per_cpu(obj_stock, cpu);
 
-		if (memcg_stock_should_drain(memcg_st, root_memcg)) {
-			if (cpu == curcpu)
+		if (cpu == curcpu) {
+			if (memcg_stock_should_drain(memcg_st, root_memcg))
 				drain_local_memcg_stock(&memcg_st->work);
-			else
-				schedule_drain_work(cpu, &memcg_st->work);
-		}
-
-		if (obj_stock_should_drain(obj_st, root_memcg)) {
-			if (cpu == curcpu)
+			if (obj_stock_should_drain(obj_st, root_memcg))
 				drain_local_obj_stock(&obj_st->work);
-			else
-				schedule_drain_work(cpu, &obj_st->work);
+			continue;
 		}
+
+		/* Pairs with RCU barrier in housekeeping_update(). */
+		guard(rcu)();
+		if (cpu_is_isolated(cpu))
+			continue;
+
+		if (memcg_stock_should_drain(memcg_st, root_memcg))
+			queue_work_on(cpu, memcg_wq, &memcg_st->work);
+		if (obj_stock_should_drain(obj_st, root_memcg))
+			queue_work_on(cpu, memcg_wq, &obj_st->work);
 	}
 	migrate_enable();
 	mutex_unlock(&percpu_charge_mutex);

Michal Hocko

2026-08-28 14:25 UTC #3 lore raw

Re: [PATCH] mm/memcontrol: avoid stuck FLUSHING_CACHED_CHARGE bit on isolated cpus

Quoted 6 lines · Rik van Riel

drain_all_stock() can leave FLUSHING_CACHED_CHARGE set after the work is dropped. It sets the bit before checking isolation and schedule_drain_work() checks isolation and queues in a separate RCU critical section, so housekeeping_update()'s synchronize_rcu() can race the second check.

Quoted 6 lines

drain_local_stock() only clears the bit for work that ran, so the bit remains set and the stock is never drained again.
Reorganize the drain_all_stock() loop, reducing nesting, splitting out local vs remote cpu handling, and skipping everything on isolated cpus, which solves the stuck FLUSHING_CACHED_CHARGE flag.

Is there any reason why we cannot simply clear the flag if the work is not scheduled?
---

mm/memcontrol.c +13 -7

index 69b37f63a307..907ac63e5067 100644
@@ -2261,8 +2261,10 @@ static bool is_memcg_drain_needed(struct memcg_stock_pcp *stock,
 	return flush;
 }
 
-static void schedule_drain_work(int cpu, struct work_struct *work)
+static bool schedule_drain_work(int cpu, struct work_struct *work)
 {
+	int ret = false;
+
 	/*
 	 * Protect housekeeping cpumask read and work enqueue together
 	 * in the same RCU critical section so that later cpuset isolated

@@ -2270,8 +2272,12 @@ static void schedule_drain_work(int cpu, struct work_struct *work)
 	 * pending work on newly isolated CPUs.
 	 */
 	guard(rcu)();
-	if (!cpu_is_isolated(cpu))
-		queue_work_on(cpu, memcg_wq, work);
+	if (!cpu_is_isolated(cpu)) {
+		queue_work_on(cpu, memcg_wq, &memcg_st->work);
+		ret = true;
+	}
+
+	return ret;
 }
 
 /*

@@ -2303,8 +2309,8 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 				      &memcg_st->flags)) {
 			if (cpu == curcpu)
 				drain_local_memcg_stock(&memcg_st->work);
-			else
-				schedule_drain_work(cpu, &memcg_st->work);
+			else if (!schedule_drain_work(cpu, &memcg_st->work))
+				clear_bit(FLUSHING_CACHED_CHARGE, &memcg_st->flags)
 		}
 
 		if (!test_bit(FLUSHING_CACHED_CHARGE, &obj_st->flags) &&

@@ -2313,8 +2319,8 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 				      &obj_st->flags)) {
 			if (cpu == curcpu)
 				drain_local_obj_stock(&obj_st->work);
-			else
-				schedule_drain_work(cpu, &obj_st->work);
+			else if (!schedule_drain_work(cpu, &obj_st->work))
+				clear_bit(FLUSHING_CACHED_CHARGE, &obj_st->flags);
 		}
 	}
 	migrate_enable();

Signature · 2 lines

Michal Hocko
SUSE Labs

Rik van Riel

author

2026-08-28 14:56 UTC #4 lore raw

Re: [PATCH] mm/memcontrol: avoid stuck FLUSHING_CACHED_CHARGE bit on isolated cpus

Quoted 11 lines · Michal Hocko

Quoted 10 lines · Rik van Riel
drain_all_stock() can leave FLUSHING_CACHED_CHARGE set after the work is dropped. It sets the bit before checking isolation and schedule_drain_work() checks isolation and queues in a separate RCU critical section, so housekeeping_update()'s synchronize_rcu() can race the second check.
drain_local_stock() only clears the bit for work that ran, so the bit
remains set and the stock is never drained again.

Quoted 9 lines

Quoted 4 lines
Reorganize the drain_all_stock() loop, reducing nesting, splitting out local vs remote cpu handling, and skipping everything on isolated
cpus, which solves the stuck FLUSHING_CACHED_CHARGE flag.
Is there any reason why we cannot simply clear the flag if the work is
not scheduled?

That should work, too.

I'm fine with whatever variant people prefer.

Signature · 1 line

All Rights Reversed.

Shakeel Butt

maintainer

2026-08-28 15:41 UTC #5 lore raw

Re: [PATCH] mm/memcontrol: avoid stuck FLUSHING_CACHED_CHARGE bit on isolated cpus

In reply to Michal Hocko

Quoted 36 lines · Michal Hocko

Quoted 6 lines · Rik van Riel
drain_all_stock() can leave FLUSHING_CACHED_CHARGE set after the work is dropped. It sets the bit before checking isolation and schedule_drain_work() checks isolation and queues in a separate RCU critical section, so housekeeping_update()'s synchronize_rcu() can race the second check.
Quoted 6 lines
drain_local_stock() only clears the bit for work that ran, so the bit remains set and the stock is never drained again.
Reorganize the drain_all_stock() loop, reducing nesting, splitting out local vs remote cpu handling, and skipping everything on isolated cpus, which solves the stuck FLUSHING_CACHED_CHARGE flag.
Is there any reason why we cannot simply clear the flag if the work is not scheduled?
---
mm/memcontrol.c +3 -1
index 69b37f63a307..907ac63e5067 100644
@@ -2261,8 +2261,10 @@ static bool is_memcg_drain_needed(struct memcg_stock_pcp *stock,
 	return flush;
 }
 
-static void schedule_drain_work(int cpu, struct work_struct *work)
+static bool schedule_drain_work(int cpu, struct work_struct *work)
 {
+	int ret = false;
+
 	/*
 	 * Protect housekeeping cpumask read and work enqueue together
 	 * in the same RCU critical section so that later cpuset isolated
@@ -2270,8 +2272,12 @@ static void schedule_drain_work(int cpu, struct work_struct *work)
 	 * pending work on newly isolated CPUs.

mm/memcontrol.c static void schedule_drain_work(int cpu, struct work_struct *work) · line 2280

Quoted 10 lines

mm/memcontrol.c +6 -2

 	 */
 	guard(rcu)();
-	if (!cpu_is_isolated(cpu))
-		queue_work_on(cpu, memcg_wq, work);
+	if (!cpu_is_isolated(cpu)) {
+		queue_work_on(cpu, memcg_wq, &memcg_st->work);
+		ret = true;
+	}
+
+	return ret;

Let's go with this patch. We can simplify above by inversing the check:

	if (cpu_is_isolated(cpu))
		return false;
	
	queue_work_on(cpu, memcg_wq, &memcg_st->work);
	return true;

Quoted 28 lines

mm/memcontrol.c +4 -4

 }
 
 /*

@@ -2303,8 +2309,8 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 				      &memcg_st->flags)) {
 			if (cpu == curcpu)
 				drain_local_memcg_stock(&memcg_st->work);
-			else
-				schedule_drain_work(cpu, &memcg_st->work);
+			else if (!schedule_drain_work(cpu, &memcg_st->work))
+				clear_bit(FLUSHING_CACHED_CHARGE, &memcg_st->flags)
 		}
 
 		if (!test_bit(FLUSHING_CACHED_CHARGE, &obj_st->flags) &&

@@ -2313,8 +2319,8 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 				      &obj_st->flags)) {
 			if (cpu == curcpu)
 				drain_local_obj_stock(&obj_st->work);
-			else
-				schedule_drain_work(cpu, &obj_st->work);
+			else if (!schedule_drain_work(cpu, &obj_st->work))
+				clear_bit(FLUSHING_CACHED_CHARGE, &obj_st->flags);
 		}
 	}
 	migrate_enable();

Signature · 2 lines

Michal Hocko
SUSE Labs

Rik van Riel

author

2026-08-28 16:13 UTC #6 lore raw

Re: [PATCH] mm/memcontrol: avoid stuck FLUSHING_CACHED_CHARGE bit on isolated cpus

Quoted 7 lines · Shakeel Butt

Let's go with this patch. We can simplify above by inversing the check:
	if (cpu_is_isolated(cpu))
		return false;

At that point, we could also clear the bit here, and have the function return void again, so there's no duplication of that check and clear in drain_all_stock()

Quoted 3 lines

	
	queue_work_on(cpu, memcg_wq, &memcg_st->work);
	return true;

Let me send that.

Signature · 1 line

All Rights Reversed.