summaryrefslogtreecommitdiffstats
path: root/kernel/cgroup_freezer.c
diff options
context:
space:
mode:
authorTejun Heo <tj@kernel.org>2014-02-12 16:07:59 -0500
committerTejun Heo <tj@kernel.org>2014-02-25 10:04:40 -0500
commita60bed296ac67b9e2765646dec8e36e3b4d7c395 (patch)
tree8056af9bec7cc230369ce94514560111bce0ad29 /kernel/cgroup_freezer.c
parent952aaa125428fae883670a2c2e40ea8044ca1eaa (diff)
downloadlinux-a60bed296ac67b9e2765646dec8e36e3b4d7c395.tar.gz
linux-a60bed296ac67b9e2765646dec8e36e3b4d7c395.tar.bz2
linux-a60bed296ac67b9e2765646dec8e36e3b4d7c395.zip
cgroup_freezer: document freezer_fork() subtleties
cgroup_subsys->fork() callback is special in that it's called outside the usual cgroup locking and may race with on-going migration. freezer_fork() currently doesn't consider such race condition; however, it is still correct thanks to the fact that freeze_task() may be called spuriously. This is quite subtle. Let's explain what's going on and add test to detect racing and losing to task migration and skip freeze_task() in such cases for documentation. This doesn't make any behavior difference meaningful to userland. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Li Zefan <lizefan@huawei.com> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Diffstat (limited to 'kernel/cgroup_freezer.c')
-rw-r--r--kernel/cgroup_freezer.c28
1 files changed, 25 insertions, 3 deletions
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 7201a637c405..2ea98b216bff 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -214,6 +214,16 @@ static void freezer_attach(struct cgroup_subsys_state *new_css,
}
}
+/**
+ * freezer_fork - cgroup post fork callback
+ * @task: a task which has just been forked
+ *
+ * @task has just been created and should conform to the current state of
+ * the cgroup_freezer it belongs to. This function may race against
+ * freezer_attach(). Losing to freezer_attach() means that we don't have
+ * to do anything as freezer_attach() will put @task into the appropriate
+ * state.
+ */
static void freezer_fork(struct task_struct *task)
{
struct freezer *freezer;
@@ -222,14 +232,26 @@ static void freezer_fork(struct task_struct *task)
freezer = task_freezer(task);
/*
- * The root cgroup is non-freezable, so we can skip the
- * following check.
+ * The root cgroup is non-freezable, so we can skip locking the
+ * freezer. This is safe regardless of race with task migration.
+ * If we didn't race or won, skipping is obviously the right thing
+ * to do. If we lost and root is the new cgroup, noop is still the
+ * right thing to do.
*/
if (!parent_freezer(freezer))
goto out;
+ /*
+ * Grab @freezer->lock and freeze @task after verifying @task still
+ * belongs to @freezer and it's freezing. The former is for the
+ * case where we have raced against task migration and lost and
+ * @task is already in a different cgroup which may not be frozen.
+ * This isn't strictly necessary as freeze_task() is allowed to be
+ * called spuriously but let's do it anyway for, if nothing else,
+ * documentation.
+ */
spin_lock_irq(&freezer->lock);
- if (freezer->state & CGROUP_FREEZING)
+ if (freezer == task_freezer(task) && (freezer->state & CGROUP_FREEZING))
freeze_task(task);
spin_unlock_irq(&freezer->lock);
out: