Al Viro wrote: > On Mon, Feb 09, 2009 at 11:03:48AM +0000, Al Viro wrote: >> BTW, a trivial note - kfree(root) in your ->kill_sb() is done >> earlier than it's nice to do. Shouldn't affect the problem, though. > Do you mean kfree(root) should be called after kill_litter_super()? I don't see the point here.. > Other probably irrelevant notes: > > memcpy(start, cgrp->dentry->d_name.name, len); > cgrp = cgrp->parent; > if (!cgrp) > break; > dentry = rcu_dereference(cgrp->dentry); > > in cgroup_path(). Why don't we need rcu_dereference on both? > Moreover, shouldn't that be > memcpy(start, dentry->d_name.name, len); > anyway, seeing that we'd just looked at dentry->d_name.len? We are right, dentry-> but not cgrp->dentry-> should be used. > > In cgroup_rmdir(): > spin_lock(&cgrp->dentry->d_lock); > d = dget(cgrp->dentry); > spin_unlock(&d->d_lock); > > cgroup_d_remove_dir(d); > dput(d); > Er? Comments, please... Unless something very unusual is going on, > either that d_lock is pointless or dget() is rather unsafe. > The code was inherited from cpuset. I doubted it's redundant, but I was not confident enough to remove it. > cgroups_clone() > /* Now do the VFS work to create a cgroup */ > inode = parent->dentry->d_inode; > > /* Hold the parent directory mutex across this operation to > * stop anyone else deleting the new cgroup */ > mutex_lock(&inode->i_mutex); > Can the parent be in process of getting deleted by somebody else? If yes, > we are in trouble here. > > BTW, that thing in cgroup_path()... What guarantees that cgroup_rename() > won't hit between getting len and doing memcpy()? > cgroup_path() was inherited from cpuset's cpuset_path(), and I think it's true it races with rename. > That said, cgroup seems to be completely agnostic wrt anything happening > on vfsmount level, so I really don't see how it gets to that WARN_ON(). > Hell knows; I really want to see the sequence of events - it might be > something like fscking up ->s_active handling with interesting results > (cgroup code is certainly hitting it in not quite usual ways), it may be > genuine VFS-only race. Need more data... > _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers