Paul Menage wrote: > On Fri, Jan 9, 2009 at 8:50 PM, Li Zefan <lizf@xxxxxxxxxxxxxx> wrote: >>> For checking the "children" list, you can just lock >>> ipaddr_subsys.hierarchy_mutex. >>> >> Unfortunately hierarchy_mutex can't be used here, since hierarchy_mutex >> doesn't protect subsys's create() method, and the create() will access >> parent cgroup's data. >> > > But that can be solved by putting a spinlock in the ipaddr_cgroup > structure and taking it in the write handler (and the connect/bind > handlers, which should also be using RCU), and taking the parent > structure's lock before copying from it in the create callback. No > need for something as heavy as cgroup_lock(). > Still won't work. :( ================ lock(ipaddr_subsys.hierarchy_mutex); if (list_empty(&cgrp->children) && parent->ipv4_addr == INADDR_ANY) create() parent->spin_lock(); cgrp->ipv4_addr = parent->ipv4_addr; parent->spin_unlock(); ipcgroup->spin_lock(); ipcgroup->ipv4_addr = new_addr; ipcgroup->spin_unlock(); unlock(ipaddr_subsys.hierarchy_mutex); list_add(&cgrp->sibling, &cgrp->parent->children); ================ And neither can it work to hold hierarchy_mutex in subsys's create(). I think the only way to make hierarchy_mutex works for this issue is: @@ -2403,16 +2403,18 @@ static long cgroup_create(struct cgroup *parent, struct if (notify_on_release(parent)) set_bit(CGRP_NOTIFY_ON_RELEASE, &cgrp->flags); + cgroup_lock_hierarchy(root); + for_each_subsys(root, ss) { struct cgroup_subsys_state *css = ss->create(ss, cgrp); if (IS_ERR(css)) { + cgroup_unlock_hierarchy(root); err = PTR_ERR(css); goto err_destroy; } init_cgroup_css(css, ss, cgrp); } - cgroup_lock_hierarchy(root); list_add(&cgrp->sibling, &cgrp->parent->children); cgroup_unlock_hierarchy(root); root->number_of_cgroups++; _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers