Hi Michal, On Fri, Aug 21, 2026 at 06:59:26PM +0200, Michal Koutn?? wrote: > 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.
Thanks a lot for looking into this problem. Is there something Noah or I could test? Regards, Salvatore
