Re: [PATCH 14/33] sched/isolation: Flush memcg workqueues on cpuset isolated partition change

Waiman Long
2025-10-21T19:16:54Z

On 10/13/25 4:31 PM, Frederic Weisbecker wrote:
> The HK_TYPE_DOMAIN housekeeping cpumask is now modifyable at runtime. In
> order to synchronize against memcg workqueue to make sure that no
> asynchronous draining is still pending or executing on a newly made
> isolated CPU, the housekeeping susbsystem must flush the memcg
> workqueues.
>
> However the memcg workqueues can't be flushed easily since they are
> queued to the main per-CPU workqueue pool.
>
> Solve this with creating a memcg specific pool and provide and use the
> appropriate flushing API.
>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
> include/linux/memcontrol.h | 4 ++++
> kernel/sched/isolation.c | 2 ++
> kernel/sched/sched.h | 1 +
> mm/memcontrol.c | 12 +++++++++++-
> 4 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 873e510d6f8d..001200df63cf 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -1074,6 +1074,8 @@ static inline u64 cgroup_id_from_mm(struct mm_struct *mm)
> return id;
> }
>
> +void mem_cgroup_flush_workqueue(void);
> +
> extern int mem_cgroup_init(void);
> #else /* CONFIG_MEMCG */
>
> @@ -1481,6 +1483,8 @@ static inline u64 cgroup_id_from_mm(struct mm_struct *mm)
> return 0;
> }
>
> +static inline void mem_cgroup_flush_workqueue(void) { }
> +
> static inline int mem_cgroup_init(void) { return 0; }
> #endif /* CONFIG_MEMCG */
>
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index 95d69c2102f6..9ec365dea921 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -144,6 +144,8 @@ int housekeeping_update(struct cpumask *mask, enum hk_type type)
>
> synchronize_rcu();
>
> + mem_cgroup_flush_workqueue();
> +
> kfree(old);
>
> return 0;
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 8fac8aa451c6..8bfc0b4b133f 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -44,6 +44,7 @@
> #include <linux/lockdep_api.h>
> #include <linux/lockdep.h>
> #include <linux/memblock.h>
> +#include <linux/memcontrol.h>
> #include <linux/minmax.h>
> #include <linux/mm.h>
> #include <linux/module.h>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1033e52ab6cf..1aa14e543f35 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -95,6 +95,8 @@ static bool cgroup_memory_nokmem __ro_after_init;
> /* BPF memory accounting disabled? */
> static bool cgroup_memory_nobpf __ro_after_init;
>
> +static struct workqueue_struct *memcg_wq __ro_after_init;
> +
> static struct kmem_cache *memcg_cachep;
> static struct kmem_cache *memcg_pn_cachep;
>
> @@ -1975,7 +1977,7 @@ static void schedule_drain_work(int cpu, struct work_struct *work)
> {
> guard(rcu)();
> if (!cpu_is_isolated(cpu))
> - schedule_work_on(cpu, work);
> + queue_work_on(cpu, memcg_wq, work);
> }
>
> /*
> @@ -5092,6 +5094,11 @@ void mem_cgroup_sk_uncharge(const struct sock *sk, unsigned int nr_pages)
> refill_stock(memcg, nr_pages);
> }
>
> +void mem_cgroup_flush_workqueue(void)
> +{
> + flush_workqueue(memcg_wq);
> +}
> +
> static int __init cgroup_memory(char *s)
> {
> char *token;
> @@ -5134,6 +5141,9 @@ int __init mem_cgroup_init(void)
> cpuhp_setup_state_nocalls(CPUHP_MM_MEMCQ_DEAD, "mm/memctrl:dead", NULL,
> memcg_hotplug_cpu_dead);
>
> + memcg_wq = alloc_workqueue("memcg", 0, 0);

Should we explicitly mark the memcg_wq as WQ_PERCPU even though I think
percpu is the default. The schedule_work_on() schedules work on the
system_percpu_wq.

Cheers,
Longman

> + WARN_ON(!memcg_wq);
> +
> for_each_possible_cpu(cpu) {
> INIT_WORK(&per_cpu_ptr(&memcg_stock, cpu)->work,
> drain_local_memcg_stock);