Messages in this thread | | | From | Witold Krecicki <> | Subject | Re: [PATCH 1/6] cgroup: add cgroup.isolation_root flag entry to the cgroup filesystem | Date | Thu, 20 Oct 2011 15:20:44 +0200 |
| |
Dnia czwartek, 20 października 2011 o 12:12:37 Paul Menage napisał(a): > On Fri, Sep 30, 2011 at 4:36 AM, Witold Krecicki <wpk@culm.net> wrote: > > + */ > > +static struct cgroup *cgroup_get_isolation_root(struct cgroup *cgrp) > > +{ > > + for (;;) { > > + if (!cgrp) > > + return NULL; > > + if (isolation_root(cgrp)) > > + return cgrp; > > + cgrp = cgrp->parent; > > + } > > + return NULL; > > +} > > What are the locking requirements for cgroup_get_isolation_root?
IMHO none - this function is performed only if there is a task in a cgroup, so cgroup cannot be removed (and a task cannot leave cgroup) - also, you cannot change isolation_root flag if the isolation_root is busy.
In here we really need locking, after fixes: static int cgroup_isolation_root_write(struct cgroup *cgrp, struct cftype *cft, u64 val) { if (cgrp->parent == NULL) return -EPERM; cgroup_lock(); if (atomic_read(&cgrp->count) || !list_empty(&cgrp->children)) { cgroup_unlock(); return -EBUSY; } if (val) set_bit(CGRP_ISOLATION_ROOT, &cgrp->flags); else clear_bit(CGRP_ISOLATION_ROOT, &cgrp->flags); cgroup_unlock(); return 0; } > Arguably we need to take a lock in these two functions, both to guard > against racing with a creation of a child cgroup or moving in a task > while trying to set its isolation root flag, and to synchronize > readers and writers of the flag. But to be honest I think we can say > that this is one of those cases where we can say that the sysadmin is > dumb enough to have races between his container setup code and his > container population code the result is undefined, as long as we don't > actually crash or leak :-) I guess that fixes that problem? -- WK
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |