History

cgroup: remove cgroup_enable_task_cg_lists() optimization

cgroup_enable_task_cg_lists() is used to lazyily initialize task cgroup associations on the first use to reduce fork / exit overheads on systems which don't use cgroup. Unfortunately, locking around it has never been actually correct and its value is dubious given how the vast majority of systems use cgroup right away from boot. This patch removes the optimization. For now, replace the cg_list based branches with WARN_ON_ONCE()'s to be on the safe side. We can simplify the logic further in the future.

Tejun Heo committed 7 years ago

Commit 5153faa1 parent a713af33 files · +39 −148Browse files

Unified · Split

3 files
  1. include/linux/cgroup.h +0 −1
  2. kernel/cgroup/cgroup.c +39 −145
  3. kernel/cgroup/cpuset.c +0 −2
include/linux/cgroup.h+0−1
@@ -150,7 +150,6 @@ struct task_struct *cgroup_taskset_first(struct cgroup_taskset *tset, · 149 unmodified lines
150150 struct task_struct *cgroup_taskset_next(struct cgroup_taskset *tset,
151151 struct cgroup_subsys_state **dst_cssp);
152152
153−void cgroup_enable_task_cg_lists(void);
154153 void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags,
155154 struct css_task_iter *it);
156155 struct task_struct *css_task_iter_next(struct css_task_iter *it);
kernel/cgroup/cgroup.c+39−145
@@ -1883,65 +1883,6 @@ static int cgroup_reconfigure(struct fs_context *fc) · 1882 unmodified lines
18831883 return 0;
18841884 }
18851885
1886−/*
1887− * To reduce the fork() overhead for systems that are not actually using
1888− * their cgroups capability, we don't maintain the lists running through
1889− * each css_set to its tasks until we see the list actually used - in other
1890− * words after the first mount.
1891− */
1892−static bool use_task_css_set_links __read_mostly;
1893−
1894−void cgroup_enable_task_cg_lists(void)
1895−{
1896− struct task_struct *p, *g;
1897−
1898− /*
1899− * We need tasklist_lock because RCU is not safe against
1900− * while_each_thread(). Besides, a forking task that has passed
1901− * cgroup_post_fork() without seeing use_task_css_set_links = 1
1902− * is not guaranteed to have its child immediately visible in the
1903− * tasklist if we walk through it with RCU.
1904− */
1905− read_lock(&tasklist_lock);
1906− spin_lock_irq(&css_set_lock);
1907−
1908− if (use_task_css_set_links)
1909− goto out_unlock;
1910−
1911− use_task_css_set_links = true;
1912−
1913− do_each_thread(g, p) {
1914− WARN_ON_ONCE(!list_empty(&p->cg_list) ||
1915− task_css_set(p) != &init_css_set);
1916−
1917− /*
1918− * We should check if the process is exiting, otherwise
1919− * it will race with cgroup_exit() in that the list
1920− * entry won't be deleted though the process has exited.
1921− * Do it while holding siglock so that we don't end up
1922− * racing against cgroup_exit().
1923− *
1924− * Interrupts were already disabled while acquiring
1925− * the css_set_lock, so we do not need to disable it
1926− * again when acquiring the sighand->siglock here.
1927− */
1928− spin_lock(&p->sighand->siglock);
1929− if (!(p->flags & PF_EXITING)) {
1930− struct css_set *cset = task_css_set(p);
1931−
1932− if (!css_set_populated(cset))
1933− css_set_update_populated(cset, true);
1934− list_add_tail(&p->cg_list, &cset->tasks);
1935− get_css_set(cset);
1936− cset->nr_tasks++;
1937− }
1938− spin_unlock(&p->sighand->siglock);
1939− } while_each_thread(g, p);
1940−out_unlock:
1941− spin_unlock_irq(&css_set_lock);
1942− read_unlock(&tasklist_lock);
1943−}
1944−
19451886 static void init_cgroup_housekeeping(struct cgroup *cgrp)
19461887 {
19471888 struct cgroup_subsys *ss;
@@ -2187,13 +2128,6 @@ static int cgroup_init_fs_context(struct fs_context *fc) · 239 unmodified lines
21872128 if (!ctx)
21882129 return -ENOMEM;
21892130
2190− /*
2191− * The first time anyone tries to mount a cgroup, enable the list
2192− * linking each css_set to its tasks and fix up all existing tasks.
2193− */
2194− if (!use_task_css_set_links)
2195− cgroup_enable_task_cg_lists();
2196−
21972131 ctx->ns = current->nsproxy->cgroup_ns;
21982132 get_cgroup_ns(ctx->ns);
21992133 fc->fs_private = &ctx->kfc;
@@ -2371,9 +2305,8 @@ static void cgroup_migrate_add_task(struct task_struct *task, · 171 unmodified lines
23712305 if (task->flags & PF_EXITING)
23722306 return;
23732307
2374− /* leave @task alone if post_fork() hasn't linked it yet */
2375− if (list_empty(&task->cg_list))
2376− return;
2308+ /* cgroup_threadgroup_rwsem protects racing against forks */
2309+ WARN_ON_ONCE(list_empty(&task->cg_list));
23772310
23782311 cset = task_css_set(task);
23792312 if (!cset->mg_src_cgrp)
@@ -4586,9 +4519,6 @@ repeat: · 2206 unmodified lines
45864519 void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags,
45874520 struct css_task_iter *it)
45884521 {
4589− /* no one should try to iterate before mounting cgroups */
4590− WARN_ON_ONCE(!use_task_css_set_links);
4591−
45924522 memset(it, 0, sizeof(*it));
45934523
45944524 spin_lock_irq(&css_set_lock);
@@ -6022,62 +5952,38 @@ void cgroup_cancel_fork(struct task_struct *child) · 1427 unmodified lines
60225952 void cgroup_post_fork(struct task_struct *child)
60235953 {
60245954 struct cgroup_subsys *ss;
5955+ struct css_set *cset;
60255956 int i;
60265957
5958+ spin_lock_irq(&css_set_lock);
5959+
5960+ WARN_ON_ONCE(!list_empty(&child->cg_list));
5961+ cset = task_css_set(current); /* current is @child's parent */
5962+ get_css_set(cset);
5963+ cset->nr_tasks++;
5964+ css_set_move_task(child, NULL, cset, false);
5965+
60275966 /*
6028− * This may race against cgroup_enable_task_cg_lists(). As that
6029− * function sets use_task_css_set_links before grabbing
6030− * tasklist_lock and we just went through tasklist_lock to add
6031− * @child, it's guaranteed that either we see the set
6032− * use_task_css_set_links or cgroup_enable_task_cg_lists() sees
6033− * @child during its iteration.
6034− *
6035− * If we won the race, @child is associated with %current's
6036− * css_set. Grabbing css_set_lock guarantees both that the
6037− * association is stable, and, on completion of the parent's
6038− * migration, @child is visible in the source of migration or
6039− * already in the destination cgroup. This guarantee is necessary
6040− * when implementing operations which need to migrate all tasks of
6041− * a cgroup to another.
6042− *
6043− * Note that if we lose to cgroup_enable_task_cg_lists(), @child
6044− * will remain in init_css_set. This is safe because all tasks are
6045− * in the init_css_set before cg_links is enabled and there's no
6046− * operation which transfers all tasks out of init_css_set.
5967+ * If the cgroup has to be frozen, the new task has too. Let's set
5968+ * the JOBCTL_TRAP_FREEZE jobctl bit to get the task into the
5969+ * frozen state.
60475970 */
6048− if (use_task_css_set_links) {
6049− struct css_set *cset;
6050−
6051− spin_lock_irq(&css_set_lock);
6052− cset = task_css_set(current); /* current is @child's parent */
6053− if (list_empty(&child->cg_list)) {
6054− get_css_set(cset);
6055− cset->nr_tasks++;
6056− css_set_move_task(child, NULL, cset, false);
6057− }
5971+ if (unlikely(cgroup_task_freeze(child))) {
5972+ spin_lock(&child->sighand->siglock);
5973+ WARN_ON_ONCE(child->frozen);
5974+ child->jobctl |= JOBCTL_TRAP_FREEZE;
5975+ spin_unlock(&child->sighand->siglock);
60585976
60595977 /*
6060− * If the cgroup has to be frozen, the new task has too.
6061− * Let's set the JOBCTL_TRAP_FREEZE jobctl bit to get
6062− * the task into the frozen state.
5978+ * Calling cgroup_update_frozen() isn't required here,
5979+ * because it will be called anyway a bit later from
5980+ * do_freezer_trap(). So we avoid cgroup's transient switch
5981+ * from the frozen state and back.
60635982 */
6064− if (unlikely(cgroup_task_freeze(child))) {
6065− spin_lock(&child->sighand->siglock);
6066− WARN_ON_ONCE(child->frozen);
6067− child->jobctl |= JOBCTL_TRAP_FREEZE;
6068− spin_unlock(&child->sighand->siglock);
6069−
6070− /*
6071− * Calling cgroup_update_frozen() isn't required here,
6072− * because it will be called anyway a bit later
6073− * from do_freezer_trap(). So we avoid cgroup's
6074− * transient switch from the frozen state and back.
6075− */
6076− }
6077−
6078− spin_unlock_irq(&css_set_lock);
60795983 }
60805984
5985+ spin_unlock_irq(&css_set_lock);
5986+
60815987 /*
60825988 * Call ss->fork(). This must happen after @child is linked on
60835989 * css_set; otherwise, @child might change state between ->fork()
@@ -6101,29 +6007,19 @@ void cgroup_exit(struct task_struct *tsk) · 17 unmodified lines
61016007 struct css_set *cset;
61026008 int i;
61036009
6104− /*
6105− * Unlink from @tsk from its css_set. As migration path can't race
6106− * with us (thanks to cgroup_threadgroup_rwsem), we can check css_set
6107− * and cg_list without synchronization.
6108− */
6109− cset = task_css_set(tsk);
6010+ spin_lock_irq(&css_set_lock);
61106011
6111− if (!list_empty(&tsk->cg_list)) {
6112− spin_lock_irq(&css_set_lock);
6113− css_set_move_task(tsk, cset, NULL, false);
6114− list_add_tail(&tsk->cg_list, &cset->dying_tasks);
6115− cset->nr_tasks--;
6012+ WARN_ON_ONCE(list_empty(&tsk->cg_list));
6013+ cset = task_css_set(tsk);
6014+ css_set_move_task(tsk, cset, NULL, false);
6015+ list_add_tail(&tsk->cg_list, &cset->dying_tasks);
6016+ cset->nr_tasks--;
61166017
6117− WARN_ON_ONCE(cgroup_task_frozen(tsk));
6118− if (unlikely(cgroup_task_freeze(tsk)))
6119− cgroup_update_frozen(task_dfl_cgroup(tsk));
6018+ WARN_ON_ONCE(cgroup_task_frozen(tsk));
6019+ if (unlikely(cgroup_task_freeze(tsk)))
6020+ cgroup_update_frozen(task_dfl_cgroup(tsk));
61206021
6121− spin_unlock_irq(&css_set_lock);
6122− } else {
6123− /* Take reference to avoid freeing init_css_set in cgroup_free,
6124− * see cgroup_fork(). */
6125− get_css_set(cset);
6126− }
6022+ spin_unlock_irq(&css_set_lock);
61276023
61286024 /* see cgroup_post_fork() for details */
61296025 do_each_subsys_mask(ss, i, have_exit_callback) {
@@ -6140,12 +6036,10 @@ void cgroup_release(struct task_struct *task) · 10 unmodified lines
61406036 ss->release(task);
61416037 } while_each_subsys_mask();
61426038
6143− if (use_task_css_set_links) {
6144− spin_lock_irq(&css_set_lock);
6145− css_set_skip_task_iters(task_css_set(task), task);
6146− list_del_init(&task->cg_list);
6147− spin_unlock_irq(&css_set_lock);
6148− }
6039+ spin_lock_irq(&css_set_lock);
6040+ css_set_skip_task_iters(task_css_set(task), task);
6041+ list_del_init(&task->cg_list);
6042+ spin_unlock_irq(&css_set_lock);
61496043 }
61506044
61516045 void cgroup_free(struct task_struct *task)
kernel/cgroup/cpuset.c+0−2
@@ -928,8 +928,6 @@ static void rebuild_root_domains(void) · 927 unmodified lines
928928 lockdep_assert_cpus_held();
929929 lockdep_assert_held(&sched_domains_mutex);
930930
931− cgroup_enable_task_cg_lists();
932−
933931 rcu_read_lock();
934932
935933 /*

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t