Skip to content
/ linux Public

Commit b636fd3

Browse files
committed
cgroup: Implement css_task_iter_skip()
When a task is moved out of a cset, task iterators pointing to the task are advanced using the normal css_task_iter_advance() call. This is fine but we'll be tracking dying tasks on csets and thus moving tasks from cset->tasks to (to be added) cset->dying_tasks. When we remove a task from cset->tasks, if we advance the iterators, they may move over to the next cset before we had the chance to add the task back on the dying list, which can allow the task to escape iteration. This patch separates out skipping from advancing. Skipping only moves the affected iterators to the next pointer rather than fully advancing it and the following advancing will recognize that the cursor has already been moved forward and do the rest of advancing. This ensures that when a task moves from one list to another in its cset, as long as it moves in the right direction, it's always visible to iteration. This doesn't cause any visible behavior changes. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Oleg Nesterov <oleg@redhat.com>
1 parent 6b115bf commit b636fd3

File tree

2 files changed

+39
-24
lines changed

2 files changed

+39
-24
lines changed

include/linux/cgroup.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
/* walk all threaded css_sets in the domain */
4444
#define CSS_TASK_ITER_THREADED (1U << 1)
4545

46+
/* internal flags */
47+
#define CSS_TASK_ITER_SKIPPED (1U << 16)
48+
4649
/* a css_task_iter should be treated as an opaque object */
4750
struct css_task_iter {
4851
struct cgroup_subsys *ss;

kernel/cgroup/cgroup.c

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ static struct cftype cgroup_base_files[];
215215

216216
static int cgroup_apply_control(struct cgroup *cgrp);
217217
static void cgroup_finalize_control(struct cgroup *cgrp, int ret);
218-
static void css_task_iter_advance(struct css_task_iter *it);
218+
static void css_task_iter_skip(struct css_task_iter *it,
219+
struct task_struct *task);
219220
static int cgroup_destroy_locked(struct cgroup *cgrp);
220221
static struct cgroup_subsys_state *css_create(struct cgroup *cgrp,
221222
struct cgroup_subsys *ss);
@@ -843,6 +844,21 @@ static void css_set_update_populated(struct css_set *cset, bool populated)
843844
cgroup_update_populated(link->cgrp, populated);
844845
}
845846

847+
/*
848+
* @task is leaving, advance task iterators which are pointing to it so
849+
* that they can resume at the next position. Advancing an iterator might
850+
* remove it from the list, use safe walk. See css_task_iter_skip() for
851+
* details.
852+
*/
853+
static void css_set_skip_task_iters(struct css_set *cset,
854+
struct task_struct *task)
855+
{
856+
struct css_task_iter *it, *pos;
857+
858+
list_for_each_entry_safe(it, pos, &cset->task_iters, iters_node)
859+
css_task_iter_skip(it, task);
860+
}
861+
846862
/**
847863
* css_set_move_task - move a task from one css_set to another
848864
* @task: task being moved
@@ -868,22 +884,9 @@ static void css_set_move_task(struct task_struct *task,
868884
css_set_update_populated(to_cset, true);
869885

870886
if (from_cset) {
871-
struct css_task_iter *it, *pos;
872-
873887
WARN_ON_ONCE(list_empty(&task->cg_list));
874888

875-
/*
876-
* @task is leaving, advance task iterators which are
877-
* pointing to it so that they can resume at the next
878-
* position. Advancing an iterator might remove it from
879-
* the list, use safe walk. See css_task_iter_advance*()
880-
* for details.
881-
*/
882-
list_for_each_entry_safe(it, pos, &from_cset->task_iters,
883-
iters_node)
884-
if (it->task_pos == &task->cg_list)
885-
css_task_iter_advance(it);
886-
889+
css_set_skip_task_iters(from_cset, task);
887890
list_del_init(&task->cg_list);
888891
if (!css_set_populated(from_cset))
889892
css_set_update_populated(from_cset, false);
@@ -4430,10 +4433,19 @@ static void css_task_iter_advance_css_set(struct css_task_iter *it)
44304433
list_add(&it->iters_node, &cset->task_iters);
44314434
}
44324435

4433-
static void css_task_iter_advance(struct css_task_iter *it)
4436+
static void css_task_iter_skip(struct css_task_iter *it,
4437+
struct task_struct *task)
44344438
{
4435-
struct list_head *next;
4439+
lockdep_assert_held(&css_set_lock);
4440+
4441+
if (it->task_pos == &task->cg_list) {
4442+
it->task_pos = it->task_pos->next;
4443+
it->flags |= CSS_TASK_ITER_SKIPPED;
4444+
}
4445+
}
44364446

4447+
static void css_task_iter_advance(struct css_task_iter *it)
4448+
{
44374449
lockdep_assert_held(&css_set_lock);
44384450
repeat:
44394451
if (it->task_pos) {
@@ -4442,15 +4454,15 @@ static void css_task_iter_advance(struct css_task_iter *it)
44424454
* consumed first and then ->mg_tasks. After ->mg_tasks,
44434455
* we move onto the next cset.
44444456
*/
4445-
next = it->task_pos->next;
4446-
4447-
if (next == it->tasks_head)
4448-
next = it->mg_tasks_head->next;
4457+
if (it->flags & CSS_TASK_ITER_SKIPPED)
4458+
it->flags &= ~CSS_TASK_ITER_SKIPPED;
4459+
else
4460+
it->task_pos = it->task_pos->next;
44494461

4450-
if (next == it->mg_tasks_head)
4462+
if (it->task_pos == it->tasks_head)
4463+
it->task_pos = it->mg_tasks_head->next;
4464+
if (it->task_pos == it->mg_tasks_head)
44514465
css_task_iter_advance_css_set(it);
4452-
else
4453-
it->task_pos = next;
44544466
} else {
44554467
/* called from start, proceed to the first cset */
44564468
css_task_iter_advance_css_set(it);

0 commit comments

Comments
 (0)