diff options
author | Tejun Heo <tj@kernel.org> | 2014-02-12 16:07:59 -0500 |
---|---|---|
committer | Tejun Heo <tj@kernel.org> | 2014-02-25 10:04:40 -0500 |
commit | a60bed296ac67b9e2765646dec8e36e3b4d7c395 (patch) | |
tree | 8056af9bec7cc230369ce94514560111bce0ad29 /kernel/cgroup_freezer.c | |
parent | 952aaa125428fae883670a2c2e40ea8044ca1eaa (diff) |
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.c | 28 |
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, | |||
214 | } | 214 | } |
215 | } | 215 | } |
216 | 216 | ||
217 | /** | ||
218 | * freezer_fork - cgroup post fork callback | ||
219 | * @task: a task which has just been forked | ||
220 | * | ||
221 | * @task has just been created and should conform to the current state of | ||
222 | * the cgroup_freezer it belongs to. This function may race against | ||
223 | * freezer_attach(). Losing to freezer_attach() means that we don't have | ||
224 | * to do anything as freezer_attach() will put @task into the appropriate | ||
225 | * state. | ||
226 | */ | ||
217 | static void freezer_fork(struct task_struct *task) | 227 | static void freezer_fork(struct task_struct *task) |
218 | { | 228 | { |
219 | struct freezer *freezer; | 229 | struct freezer *freezer; |
@@ -222,14 +232,26 @@ static void freezer_fork(struct task_struct *task) | |||
222 | freezer = task_freezer(task); | 232 | freezer = task_freezer(task); |
223 | 233 | ||
224 | /* | 234 | /* |
225 | * The root cgroup is non-freezable, so we can skip the | 235 | * The root cgroup is non-freezable, so we can skip locking the |
226 | * following check. | 236 | * freezer. This is safe regardless of race with task migration. |
237 | * If we didn't race or won, skipping is obviously the right thing | ||
238 | * to do. If we lost and root is the new cgroup, noop is still the | ||
239 | * right thing to do. | ||
227 | */ | 240 | */ |
228 | if (!parent_freezer(freezer)) | 241 | if (!parent_freezer(freezer)) |
229 | goto out; | 242 | goto out; |
230 | 243 | ||
244 | /* | ||
245 | * Grab @freezer->lock and freeze @task after verifying @task still | ||
246 | * belongs to @freezer and it's freezing. The former is for the | ||
247 | * case where we have raced against task migration and lost and | ||
248 | * @task is already in a different cgroup which may not be frozen. | ||
249 | * This isn't strictly necessary as freeze_task() is allowed to be | ||
250 | * called spuriously but let's do it anyway for, if nothing else, | ||
251 | * documentation. | ||
252 | */ | ||
231 | spin_lock_irq(&freezer->lock); | 253 | spin_lock_irq(&freezer->lock); |
232 | if (freezer->state & CGROUP_FREEZING) | 254 | if (freezer == task_freezer(task) && (freezer->state & CGROUP_FREEZING)) |
233 | freeze_task(task); | 255 | freeze_task(task); |
234 | spin_unlock_irq(&freezer->lock); | 256 | spin_unlock_irq(&freezer->lock); |
235 | out: | 257 | out: |