lkml.org 
[lkml]   [2008]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: Linux 2.6.26-rc4
    From
    Date

    On Tue, 2008-06-03 at 08:01 -0700, Linus Torvalds wrote:
    >
    > On Tue, 3 Jun 2008, Ian Kent wrote:
    > > >
    > > > > I think it must be autofs4 doing something weird. Like this in
    > > > > autofs4_lookup_unhashed():
    > > > >
    > > > > /*
    > > > > * Make the rehashed dentry negative so the VFS
    > > > > * behaves as it should.
    > > > > */
    > > > > if (inode) {
    > > > > dentry->d_inode = NULL;
    >
    > Uhhuh. Yeah, that's not allowed.
    >
    > A dentry inode can start _out_ as NULL, but it can never later become NULL
    > again until it is totally unused.
    >
    > > > Lovely. If we ever step into that with somebody else (no matter who)
    > > > holding a reference to that dentry, we are certainly well and truly
    > > > buggered. It's not just mount(2) - everything in the tree assumes that
    > > > holding a reference to positive dentry guarantees that it remains
    > > > positive.
    >
    > Indeed. Things like regular file ops won't even test the inode, since they
    > know that "open()" will only open a dentry with a positive entry, so they
    > know that the dentry->inode is non-NULL.
    >
    > [ Although some code-paths do test - but that is just because people are
    > so used to testign that pointers are non-NULL. ]
    >
    > > The intent here is that, the dentry above is unhashed at this point, and
    > > if hasn't been reclaimed by the VFS, it is made negative and replaces
    > > the unhashed negative dentry passed to ->lookup(). The reference count
    > > is incremented to account for the reference held by the path walk.
    > >
    > > What am I doing wrong here?
    >
    > What's wrong is that you can't do that "dentry->d_inode = NULL". EVER.

    OK.

    >
    > Why would you want to? If the dentry is already unhashed, then no _new_
    > lookups will ever find it anyway, so it's effectively unfindable anyway.
    > Except by people who *have* to find it, ie the people who already hold it
    > open (because, for example, they opened it earlier, or because they
    > chdir()'ed into a subdirectory).

    The code we're talking about deals with a race between expiring and
    mounting an autofs mount point at the same time.

    I'll have a closer look and see if I can make it work without turning
    the dentry negative.

    >
    > So why don't you just return a NULL dentry instead, for a unhashed dentry?
    > Or do the "goto next" thing?

    That just won't work for the case this is meant to deal with.

    Ian




    \
     
     \ /
      Last update: 2008-06-03 18:13    [W:2.243 / U:0.480 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site