lkml.org 
[lkml]   [2009]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [cgroup or VFS ?] WARNING: at fs/namespace.c:636 mntput_no_expire+0xac/0xf2()
    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...
    >




    \
     
     \ /
      Last update: 2009-02-10 06:51    [W:3.222 / U:0.204 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site