Re: 2.6.38-rc2... NFS sillyrename is broken...

[Trond Myklebust <Trond.Myklebust@netapp.com>]

On Thu, 2011-01-27 at 10:50 +1100, Nick Piggin wrote: 
> On Thu, Jan 27, 2011 at 7:43 AM, Trond Myklebust
> <Trond.Myklebust@netapp.com> wrote:
> > On Wed, 2011-01-26 at 15:14 -0500, Trond Myklebust wrote:
> >> The alternative would be to add a callback that can be called after
> >> dentry_iput() if DCACHE_NFSFS_RENAMED is true, and that takes the parent
> >> and (negative) dentry as the arguments.
> >> sillyrename doesn't need the inode as an argument, but it definitely
> >> needs the parent dentry so that it can check for races with
> >> ->lookup()...
> >
> > The following (compile tested only!) patch illustrates what I mean.
> 
> We could do this. CEPH also want a way to get d_parent in the inode
> unlink path.
> 
> I think I can actually check for dentry->d_count == 0 rather than
> dentry->d_parent == NULL here, and avoid clearing d_parent
> entirely. That might be the better solution for 2.6.38, because other
> code I've missed might be expecting to use d_parent.

I'm not sure I understand. By the time we hit d_kill() we know that
dentry->d_count == 0. The only thing we need here is the ability to
unlink the file if it has been sillyrenamed previously. The reason for
needing the parent dentry is to allow the filesystem to guard against
races with lookup without requiring the vfs to take the
parent->d_inode->i_mutex in the dput() path...

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com