Messages in this thread | | | Date | Tue, 21 Oct 2008 11:47:40 +0200 | From | Cedric Le Goater <> | Subject | Re: [PATCH 2/4] freezer_cg: remove redundant check in freezer_can_attach() |
| |
Li Zefan wrote: > Cedric Le Goater wrote: >> Li Zefan wrote: >>> It is sufficient to check if @task is frozen, and no need to check if >>> the original freezer is frozen. >> hmm, a task being frozen does not mean that its freezer cgroup is >> frozen. > > If a task has being frozen, then can_attach() returns -EBUSY at once. > If a task isn't frozen, then we have orig_freezer->state == THAWED. > > So we need to check the state of the task only. > >> So the extra check seems valid but looking at the comment : >> >> /* >> * The call to cgroup_lock() in the freezer.state write method prevents >> * a write to that file racing against an attach, and hence the >> * can_attach() result will remain valid until the attach completes. >> */ >> static int freezer_can_attach(struct cgroup_subsys *ss, >> >> how do we know that the task_freezer(task), which is not protected by >> the cgroup_lock(), is not going to change its state to CGROUP_FROZEN >> it looks racy. >> > > Since freezer_can_attach() is called by cgroup core with cgroup_lock held, there is > no race with task attaching and state changing.
ok I see. cgroup_mutex is global, I thought it had changed and that we were only locking the cgroup the task was being attached to.
Acked-by: Cedric Le Goater <clg@fr.ibm.com>
thanks,
C.
| |