Re: [PATCH] nfs: don't queue synchronous NFSv4 close rpc_release to nfsiod

[Jeff Layton <jlayton@redhat.com>]

On Thu, 17 Feb 2011 16:37:41 -0500
Jeff Layton <jlayton@redhat.com> wrote:

> On Thu, 17 Feb 2011 11:47:24 -0800
> Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> 
> > On Thu, 2011-02-17 at 10:10 -0500, Jeff Layton wrote: 
> > > On Thu, 17 Feb 2011 08:40:52 -0500
> > > Jeff Layton <jlayton@redhat.com> wrote:
> > > 
> > > > 
> > > > Looks like it finally failed on the 39th pass:
> > > > 
> > > > second check for lost reply on non-idempotent requests
> > > > testing 50 idempotencies in directory "testdir"
> > > > rmdir 1: Directory not empty
> > > > special tests failed
> > > > 
> > > > When I look in the directory (several hours after it failed), the
> > > > silly-renamed file is still there:
> > > > 
> > > > -rw---x--x. 1 root root   30 Feb 16 15:04 .nfs000000000000002d00000090
> > > > 
> > > > ...so I'm not sure what exactly is wrong yet, but it looks like the
> > > > silly delete just never happened. Maybe there's a dentry refcount leak
> > > > of some sort? There are no queued RPC's.
> > > > 
> > > > I'll keep looking at it but if you have ideas as to what it could be,
> > > > let me know.
> > > > 
> > > 
> > > I walked down the directory tree in crash on the live kernel and found
> > > the dentry. The d_count is 0x0, so I'm not clear on why it didn't get
> > > cleaned up:
> > > 
> > > crash> struct dentry.d_flags,d_count,d_name 0xffff880017d46a80
> > >   d_flags = 0xc000, 
> > >   d_count = 0x0, 
> > >   d_name = {
> > >     hash = 0xe08ab5c8, 
> > >     len = 0x1c, 
> > >     name = 0xffff880017d46ab8 ".nfs000000000000002d00000090"
> > >   }, 
> > > 
> > > 
> > > The d_flags are:
> > > 
> > > #define DCACHE_OP_REVALIDATE    0x4000
> > > #define DCACHE_OP_DELETE        0x8000
> > > 
> > > ...very odd. I'd have expected to see this one set too:
> > > 
> > > #define DCACHE_NFSFS_RENAMED  0x0002
> > > 
> > > I suppose the async sillyrename call could have failed and we ended up
> > > calling nfs_cancel_async_unlink? I'll stick in some printk's around
> > > that area and see if I can figure out what's going on...
> > > 
> > 
> > Perhaps I missed a call site that needs an rpc_put_task_async()?
> > 
> 
> I'm not sure that would explain what we're seeing here, unless I'm just
> missing something. I do know that nfs_cancel_async_unlink was never
> called in my latest test run, so that does not seem to be it.
> 
> Another anomaly -- d_fsdata is NULL. How do we get into that state with
> a dentry that has been silly-renamed? FWIW, here's the entire dentry
> struct with my latest reproducer. Let me know if anything stands out to
> you...
> 

My current thinking is that the problems I'm seeing here are not
directly related to the bug we're trying to fix. It looks like
sillyrename is still just plain broken a'la your email entitled:

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

...I'll plan to keep an eye out for the fix from Nick, and re-test this
when it hits.

Thanks,
-- 
Jeff Layton <jlayton@redhat.com>