Hey, Frederic. On Mon, Oct 08, 2012 at 02:48:58PM +0200, Frederic Weisbecker wrote: > Yeah I missed this one. > Now the whole cgroup_attach_task() is clusteracy without the Clusteracy? > threadgroup lock anyway: > > * The PF_EXITING check is racy (we are neither holding tsk->flags nor > threagroup lock). PF_EXITING is *always* protected by threadgroup_change_begin/end(). > * The cgrp == oldcgrp is racy (exit() can change the oldcgrp anytime. So, as long as this happens after PF_EXITING check, it should be safe. > * can_attach / attach / cancel_attach can race against fork/exit (and > post_fork if you consider those interested in cgroup task link like > the freezer. But that is racy in any case already even with > threadgroup lock) Against exit, no. Against forking a new process, can they? If so, we need to fix it. > It has been designed to be called under that lock. So I suspect the Ummm.... threadgroup_lock is a recent addition so things couldn't have been designed to be called under that lock. threadgroup_lock protects the *threadgroup* - creating a new task in the same process or a task of the process exiting. It doesn't do anything about other processes. In fact, the lock itself is per-process. > best, at least for now, is to threadgroup lock from > cgroup_attach_task_all(). And also make cgroup_attach_task() static to > avoid future unsafe callers. Oh, from that call path, sure. Can someone teach me why we need that one at all? I think we're confusing each other here. I was talking about the usual migration path not protected against forking a new process. > There is no harm yet because the only user of it calls that with > current as the "task" parameter, in a place that is > not in the middle of a fork. So no need to worry about some stable backport. > > Also, looking at cgroup_attach_task_all(), what guarantee do we have > that "from" is not concurrently exiting and removing its cgrp. Which > is a separate problem. But we probably need to do some css_set_get() > before playing with it. I really don't know. Why isn't it locking the threadgroup to begin with? Thanks. -- tejun _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
