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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html