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@xxxxxxxxxx> thanks, C. _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers