| Message ID | 20200117151533.12381-3-mkoutny@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] cgroup: Unify css_set task lists | expand |
On Fri, Jan 17, 2020 at 04:15:32PM +0100, Michal Koutný wrote: > PF_EXITING is set earlier than actual removal from css_set when a task > is exitting. This can confuse cgroup.procs readers who see no PF_EXITING > tasks, however, rmdir is checking against css_set membership so it can > transitionally fail with EBUSY. > > Fix this by listing tasks that weren't unlinked from css_set active > lists. > It may happen that other users of the task iterator (without > CSS_TASK_ITER_PROCS) spot a PF_EXITING task before cgroup_exit(). This > is equal to the state before commit c03cd7738a83 ("cgroup: Include dying > leaders with live threads in PROCS iterations") but it may be reviewed > later. Yeah, this looks fine to me. Any chance you can order this before the clean up so that we can mark it for -stable. Thanks.
On Fri, Jan 17, 2020 at 9:28 AM Tejun Heo <tj@kernel.org> wrote: > > On Fri, Jan 17, 2020 at 04:15:32PM +0100, Michal Koutný wrote: > > PF_EXITING is set earlier than actual removal from css_set when a task > > is exitting. This can confuse cgroup.procs readers who see no PF_EXITING > > tasks, however, rmdir is checking against css_set membership so it can > > transitionally fail with EBUSY. > > > > Fix this by listing tasks that weren't unlinked from css_set active > > lists. > > It may happen that other users of the task iterator (without > > CSS_TASK_ITER_PROCS) spot a PF_EXITING task before cgroup_exit(). This > > is equal to the state before commit c03cd7738a83 ("cgroup: Include dying > > leaders with live threads in PROCS iterations") but it may be reviewed > > later. Tested-by: Suren Baghdasaryan <surenb@google.com> > > Yeah, this looks fine to me. Any chance you can order this before the > clean up so that we can mark it for -stable. > +1 for reordering. Makes it easier to backport. Thanks, Suren. > Thanks. > > -- > tejun > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >
On Fri, Jan 17, 2020 at 10:41:29AM -0800, Suren Baghdasaryan <surenb@google.com> wrote: > Tested-by: Suren Baghdasaryan <surenb@google.com> Thanks. > > Yeah, this looks fine to me. Any chance you can order this before the > > clean up so that we can mark it for -stable. > +1 for reordering. Makes it easier to backport. The grounds still need to be prepared for css_task_iter to store additional information. Let me see how the preceding changes can be minimized. Michal
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index b56283e13491..132d258e7172 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -4492,11 +4492,12 @@ static void css_task_iter_advance(struct css_task_iter *it) goto repeat; /* and dying leaders w/o live member threads */ - if (!atomic_read(&task->signal->live)) + if (it->cur_list == CSS_SET_TASKS_DYING && + !atomic_read(&task->signal->live)) goto repeat; } else { /* skip all dying ones */ - if (task->flags & PF_EXITING) + if (it->cur_list == CSS_SET_TASKS_DYING) goto repeat; } }
PF_EXITING is set earlier than actual removal from css_set when a task is exitting. This can confuse cgroup.procs readers who see no PF_EXITING tasks, however, rmdir is checking against css_set membership so it can transitionally fail with EBUSY. Fix this by listing tasks that weren't unlinked from css_set active lists. It may happen that other users of the task iterator (without CSS_TASK_ITER_PROCS) spot a PF_EXITING task before cgroup_exit(). This is equal to the state before commit c03cd7738a83 ("cgroup: Include dying leaders with live threads in PROCS iterations") but it may be reviewed later. Reported-by: Suren Baghdasaryan <surenb@google.com> Fixes: c03cd7738a83 ("cgroup: Include dying leaders with live threads in PROCS iterations") Signed-off-by: Michal Koutný <mkoutny@suse.com> --- kernel/cgroup/cgroup.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)