Skip to content

Commit

Permalink
cgroup: Include dying leaders with live threads in PROCS iterations
Browse files Browse the repository at this point in the history
CSS_TASK_ITER_PROCS currently iterates live group leaders; however,
this means that a process with dying leader and live threads will be
skipped.  IOW, cgroup.procs might be empty while cgroup.threads isn't,
which is confusing to say the least.

Fix it by making cset track dying tasks and include dying leaders with
live threads in PROCS iteration.

Signed-off-by: Tejun Heo <[email protected]>
Reported-and-tested-by: Topi Miettinen <[email protected]>
Cc: Oleg Nesterov <[email protected]>
  • Loading branch information
htejun committed May 31, 2019
1 parent b636fd3 commit c03cd77
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 7 deletions.
1 change: 1 addition & 0 deletions include/linux/cgroup-defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ struct css_set {
*/
struct list_head tasks;
struct list_head mg_tasks;
struct list_head dying_tasks;

/* all css_task_iters currently walking this cset */
struct list_head task_iters;
Expand Down
1 change: 1 addition & 0 deletions include/linux/cgroup.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ struct css_task_iter {
struct list_head *task_pos;
struct list_head *tasks_head;
struct list_head *mg_tasks_head;
struct list_head *dying_tasks_head;

struct css_set *cur_cset;
struct css_set *cur_dcset;
Expand Down
44 changes: 37 additions & 7 deletions kernel/cgroup/cgroup.c
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@ struct css_set init_css_set = {
.dom_cset = &init_css_set,
.tasks = LIST_HEAD_INIT(init_css_set.tasks),
.mg_tasks = LIST_HEAD_INIT(init_css_set.mg_tasks),
.dying_tasks = LIST_HEAD_INIT(init_css_set.dying_tasks),
.task_iters = LIST_HEAD_INIT(init_css_set.task_iters),
.threaded_csets = LIST_HEAD_INIT(init_css_set.threaded_csets),
.cgrp_links = LIST_HEAD_INIT(init_css_set.cgrp_links),
Expand Down Expand Up @@ -1213,6 +1214,7 @@ static struct css_set *find_css_set(struct css_set *old_cset,
cset->dom_cset = cset;
INIT_LIST_HEAD(&cset->tasks);
INIT_LIST_HEAD(&cset->mg_tasks);
INIT_LIST_HEAD(&cset->dying_tasks);
INIT_LIST_HEAD(&cset->task_iters);
INIT_LIST_HEAD(&cset->threaded_csets);
INIT_HLIST_NODE(&cset->hlist);
Expand Down Expand Up @@ -4399,15 +4401,18 @@ static void css_task_iter_advance_css_set(struct css_task_iter *it)
it->task_pos = NULL;
return;
}
} while (!css_set_populated(cset));
} while (!css_set_populated(cset) && !list_empty(&cset->dying_tasks));

if (!list_empty(&cset->tasks))
it->task_pos = cset->tasks.next;
else
else if (!list_empty(&cset->mg_tasks))
it->task_pos = cset->mg_tasks.next;
else
it->task_pos = cset->dying_tasks.next;

it->tasks_head = &cset->tasks;
it->mg_tasks_head = &cset->mg_tasks;
it->dying_tasks_head = &cset->dying_tasks;

/*
* We don't keep css_sets locked across iteration steps and thus
Expand Down Expand Up @@ -4446,6 +4451,8 @@ static void css_task_iter_skip(struct css_task_iter *it,

static void css_task_iter_advance(struct css_task_iter *it)
{
struct task_struct *task;

lockdep_assert_held(&css_set_lock);
repeat:
if (it->task_pos) {
Expand All @@ -4462,17 +4469,32 @@ static void css_task_iter_advance(struct css_task_iter *it)
if (it->task_pos == it->tasks_head)
it->task_pos = it->mg_tasks_head->next;
if (it->task_pos == it->mg_tasks_head)
it->task_pos = it->dying_tasks_head->next;
if (it->task_pos == it->dying_tasks_head)
css_task_iter_advance_css_set(it);
} else {
/* called from start, proceed to the first cset */
css_task_iter_advance_css_set(it);
}

/* if PROCS, skip over tasks which aren't group leaders */
if ((it->flags & CSS_TASK_ITER_PROCS) && it->task_pos &&
!thread_group_leader(list_entry(it->task_pos, struct task_struct,
cg_list)))
goto repeat;
if (!it->task_pos)
return;

task = list_entry(it->task_pos, struct task_struct, cg_list);

if (it->flags & CSS_TASK_ITER_PROCS) {
/* if PROCS, skip over tasks which aren't group leaders */
if (!thread_group_leader(task))
goto repeat;

/* and dying leaders w/o live member threads */
if (!atomic_read(&task->signal->live))
goto repeat;
} else {
/* skip all dying ones */
if (task->flags & PF_EXITING)
goto repeat;
}
}

/**
Expand Down Expand Up @@ -6009,6 +6031,7 @@ void cgroup_exit(struct task_struct *tsk)
if (!list_empty(&tsk->cg_list)) {
spin_lock_irq(&css_set_lock);
css_set_move_task(tsk, cset, NULL, false);
list_add_tail(&tsk->cg_list, &cset->dying_tasks);
cset->nr_tasks--;

WARN_ON_ONCE(cgroup_task_frozen(tsk));
Expand All @@ -6034,6 +6057,13 @@ void cgroup_release(struct task_struct *task)
do_each_subsys_mask(ss, ssid, have_release_callback) {
ss->release(task);
} while_each_subsys_mask();

if (use_task_css_set_links) {
spin_lock_irq(&css_set_lock);
css_set_skip_task_iters(task_css_set(task), task);
list_del_init(&task->cg_list);
spin_unlock_irq(&css_set_lock);
}
}

void cgroup_free(struct task_struct *task)
Expand Down

0 comments on commit c03cd77

Please sign in to comment.