Hello Suren. On Fri, Jan 10, 2020 at 01:47:19PM -0800, Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote: > > On Fri, Jun 07, 2019 at 07:09:53PM +0200, Michal Koutný wrote: > > > Wouldn't it make more sense to call > > > css_set_move_task(tsk, cset, NULL, false); > > > in cgroup_release instead of cgroup_exit then? > > > > > > css_set_move_task triggers the cgroup emptiness notification so if we > > > list group leaders with running siblings as members of the cgroup (IMO > > > correct), is it consistent to deliver the (un)populated event > > > that early? > > > By moving to cgroup_release we would also make this notification > > > analogous to SIGCHLD delivery. > > > > So, the current behavior is mostly historical and I don't think this > > is a better behavior. That said, I'm not sure about the cost benefit > > ratio of changing the behavior at this point given how long the code > > has been this way. Another aspect is that it'd expose zombie tasks > > and possibly migrations of them to each controller. > > Sorry for reviving an old discussion Since you reply to my remark, I have to share that I found myself wrong later wrt the emptiness notifications. Moving the task in cgroup_exit doesn't matter if thread group contains other live tasks, the unpopulated notification will be raised when the last task of thread group calls group_exit, i.e. it is similar to SIGHLD. Now to your issue. > but I got a bug report from a customer about Android What kernel version is that? Can be it considered equal to the current Linux? > In my testing I was able to reproduce the failure in > which case rmdir() fails with EBUSY from cgroup_destroy_locked() > because cgroup_is_populated() returns true. That'd mean that not all tasks in the cgroup did exit() (cgroup_exit()), i.e. they're still running. Can you see them in cgroup.threads/tasks? > cgroup_is_populated() depends on cgrp->nr_populated_xxx counters which > IIUC will not be updated until cgroup_update_populated() is called > from cgroup_exit() and that might get delayed... Why do you think it's delayed? > So from user space's point of view the cgroup is empty and can be > removed but rmdir() fails to do so. As Tejun says, cgroup with only dead _tasks_ should be removable (and if I'm not mistaken it is in the current kernel). Unless you do individual threads migration when a thread would be separated from its (dead) leader. Does your case include such migrations? Michal
Attachment:
signature.asc
Description: Digital signature
