Re: [PATCH v3] cgroup: avoid flushing global workqueue in cgroup1_pidlist_destroy_all

["Michal Koutný" <mkoutny@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 3099 bytes --]

Hi Junnan.

(Sorry for late response, I sketched some notes and then didn't get down
to sent them. Now they're below.)

On Fri, Aug 14, 2026 at 06:20:52PM +0800, Junnan Zhang <zhangjn_dev@163.com> wrote:
> From: Junnan Zhang <zhangjn11@chinatelecom.cn>
> 
> cgroup1_pidlist_destroy_all() flushes the global
> cgroup_pidlist_destroy_wq while destroying a cgroup. Because all cgroup
> v1 pidlist destruction work items are queued on the same shared workqueue,
> a single slow or stuck work item (e.g. waiting for pidlist_mutex held by a
> user-space reader) blocks every concurrent cgroup destruction path.
> 
> This can lead to kworker tasks stuck in flush_workqueue() for over
> hung_task_timeout seconds, as observed on busy systems running Docker or
> Kubernetes workloads.

Since the cgroup_pidlist_destroy_wq is already a dedicated workqueue (no
other conteders), the pursuit of pidlist_mutex holder is a feasible
theory. However, that would also mean:
a) a single reader taking more than hung_task_timeout_secs (that'd be
   a softlockup earlier),
b) starvation of cgroup1_pidlist_destroy_all() by many (queued)
   cgroup_pidlist_start() callers which goes against the second-long
   caching of pidlists,
c) there is large number of nr_cgroups * nr_pidnses which makes the
   caching ineffective

> 
>   INFO: task kworker/0:1:1438499 blocked for more than 120 seconds.
>   "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>   kworker/0:1 D 0 1438499 2 0x80000080
>   Workqueue: cgroup_destroy css_free_rwork_fn
>   ? __schedule+0x296/0x900
>   schedule+0x28/0x80
>   schedule_timeout+0x1ee/0x3a0
>   ? kvm_sched_clock_read+0xd/0x20
>   wait_for_completion+0x12c/0x190
>   ? wake_up_q+0x70/0x70
>   flush_workqueue+0x132/0x430
>   ? cgroup1_pidlist_destroy_all+0x7c/0xa0
>   cgroup1_pidlist_destroy_all+0x7c/0xa0
>   css_free_rwork_fn+0xb5/0x390
>   process_one_work+0x195/0x3e0
>   worker_thread+0x30/0x390
>   ? process_one_work+0x3e0/0x3e0
>   kthread+0x113/0x130
>   ? kthread_create_worker_on_cpu+0x70/0x70
>   ret_from_fork+0x1f/0x40
> 
> Fix it by moving the cgroup's pidlists to a local orphan list under
> pidlist_mutex, clearing their ->owner pointer, and then cancelling each
> pidlist's delayed work outside the lock. The destroy work function now
> checks ->owner and skips freeing orphaned pidlists, so
> cgroup1_pidlist_destroy_all() can free them safely without flushing the
> whole shared workqueue.

What's the point of the workqueue after this change? (Mainly the
expiration + having process context for the handler.)

The flushing isn't necessary if there's a way how to ensure pidlists
head won't be used after cgrp removal, which the fix should achieve.


So I'd say, the narrow-focused cancellation may work, no need to wait
for other cgroups. OTOH, I'm surprised this v1-issue popped up only now
and whether such a long contention can happen over pidlist_mutex as your
commit message implies. What nr_cgroups, nr_pidnses could cause this in
your theory?

Thanks,
Michal


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]