Messages in this thread | | | From | Paul Menage <> | Date | Thu, 20 Oct 2011 03:12:37 -0700 | Subject | Re: [PATCH 1/6] cgroup: add cgroup.isolation_root flag entry to the cgroup filesystem |
| |
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?
> + > +static int cgroup_isolation_root_write(struct cgroup *cgrp, > + struct cftype *cft, > + u64 val) > +{ > + if (cgrp->parent == NULL) > + return -EBUSY;
EPERM or EINVAL would be more appropriate here, I think.
> + if (atomic_read(&cgrp->count)) > + return -EBUSY;
Also need to check for child cgroups, and return -EBUSY?
> + if (val) > + set_bit(CGRP_ISOLATION_ROOT, &cgrp->flags); > + else > + clear_bit(CGRP_ISOLATION_ROOT, &cgrp->flags); > + 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 :-)
Paul -- 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/
| |