Hi Salvatore, thanks for the nice report and sorry for not so prompt response.
On Sat, Aug 15, 2026 at 09:41:14PM +0200, Salvatore Bonaccorso <[email protected]> wrote: > With an additional reproducer provided by Noah, I could bisect the > change down to Good job. > > commit 260fbcb92bbeacfcd050410fdc2d24ab15044400 > Author: Tejun Heo <[email protected]> > Date: Tue Oct 28 20:19:16 2025 -1000 > > cgroup: Move dying_tasks cleanup from cgroup_task_release() to > cgroup_task_free() > > Currently, cgroup_task_exit() adds thread group leaders with live > member > threads to their css_set's dying_tasks list (so cgroup.procs > iteration can > still see the leader), and cgroup_task_release() later removes them > with > list_del_init(&task->cg_list). > > An upcoming patch will defer the dying_tasks list addition, moving > it from > cgroup_task_exit() (called from do_exit()) to a new function called > from > finish_task_switch(). However, release_task() (which calls > cgroup_task_release()) can run either before or after > finish_task_switch(), > creating a race where cgroup_task_release() might try to remove the > task from > dying_tasks before or while it's being added. > > Move the list_del_init() from cgroup_task_release() to > cgroup_task_free() to > fix this race. cgroup_task_free() runs from __put_task_struct(), > which is > always after both paths, making the cleanup safe. > > Cc: Dan Schatzberg <[email protected]> > Cc: Peter Zijlstra <[email protected]> > Signed-off-by: Tejun Heo <[email protected]> > > But there was the suspect that the matching commit might be > d245698d727a ("cgroup: Defer task cgroup unlink until after the task > is done switching out"). I see that after 260fbcb92bbea ("cgroup: Move dying_tasks cleanup from cgroup_task_release() to cgroup_task_free()") it may be possible that tasks on the dying_tasks list may drop their ->usage to zero (since the actual unlinking only happens in __put_task_struct). Most often those would be skipped due to PF_EXITING except for the case of thread group leaders (which the reproducer stresses) whose refcount apparently can drop to zero after task->signal->live > 0 made them iterable :-/ A band-aid fix could be to use tryget_task_struct() in css_task_iter_next() (I got that hint from a LLM) and "skip" zeroed tasks. I see that commit fbe3fb103596b ("sched_ext: Replace tryget_task_struct() with get_task_struct()"), assumes the iterator always succeeds in obtaining the task reference (which was the justification of tryget removal). I expect that sched_ext should still be fine if dying_tasks with zero references are skipped. (What are they? Tasks which literally no one should be interested in and they're only waiting for __put_task_struct_rcu_cb() to be called [*]). (I'm calling that band-aid because it'd resurrect usage of tryget_task_struct() and it keeps the dying_tasks list a weird place to be. If anyone has a better idea?) The commit d245698d727a ("cgroup: Defer task cgroup unlink until after the task is done switching out") seems a reasonable separation of the stages to me. Regards, Michal [*] Except for io_uring_drop_tctx_refs() that calls __put_task_struct() directly (no RCU) but I'd argue the same, that those should not be possibly iterated.
signature.asc
Description: PGP signature
