On Fri, Mar 31, 2023 at 10:50:45AM -0400, Waiman Long <longman@xxxxxxxxxx> wrote: > The current cpuset code uses the global cpuset_attach_old_cs variable > to store the old cpuset value between consecutive cpuset_can_attach() > and cpuset_attach() calls. Since a caller of cpuset_can_attach() may > not need to hold the global cgroup_threadgroup_rwsem, parallel cpuset > attach operations are possible. Do I understand correctly this consequence of the cpuset_attach_task() on the clone path? In that particular case (with CLONE_INTO_CGROUP) cgroup_mutex is taken, so the access the the old_cs variable should still be synchronized with regular migrations that are also under cgroup_mutex. > When there are concurrent cpuset attach operations in progress, > cpuset_attach() may fetch the wrong value from cpuset_attach_old_cs > causing incorrect result. To avoid this problem while still allowing > certain level of parallelism, drop cpuset_attach_old_cs and use a > per-cpuset attach_old_cs value. Also restrict to at most one active > attach operation per cpuset to avoid corrupting the value of the > per-cpuset attach_old_cs value. Secondly, semantically wouldn't a `void *ss_priv[CGROUP_SUBSYS_COUNT]` in struct cgroup_taskset make it simpler wrt the exclusivity guarantees? Thirdly, if my initial assumptino is right -- I'd suggest ordering this before the patch `cgroup/cpuset: Make cpuset_fork() handle CLONE_INTO_CGROUP properly` to spare backporters possible troubles if this is would be a fixup to that. Thanks, Michal
Attachment:
signature.asc
Description: PGP signature
