[PATCH] nfs4: handle situation where dentry wasn't revalidated on open

Previous thread: [PATCH] fuse: return -EAGAIN if the connection has not been init'ed by Josef Bacik on Thursday, November 12, 2009 - 2:01 pm. (12 messages)

Next thread: Union mount of block devices? by Andrea Gelmini on Friday, November 13, 2009 - 6:28 am. (2 messages)
From: Jeff Layton
Date: Friday, November 13, 2009 - 6:23 am

This patch is an alternate approach to fixing this problem that doesn't
rely on VFS changes...

Currently, the NFSv4 client code relies on the ->lookup and
->d_revalidate codepaths to handle the open processing during lookup. In
certain situations (notably LAST_BIND symlinks and file bind mounts)
it's possible for the VFS to skip calling d_revalidate on a dentry that
it finds in the cache. If this is done on an open call, the file doesn't
get opened on the wire, no state is established and stateful operations
fail against it.

This patchset fixes this problem by taking advantage of the fact that we
can pass an open routine to lookup_instantiate_filp. A new open file
operation for NFSv4 is added that should only be called if the filp
wasn't instantiated during lookup. The original open routine is passed
to lookup_instantiate_filp to ensure no change in behavior there.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/file.c           |   42 ++++++++++++++++++++++++++++++++++++++++--
 fs/nfs/inode.c          |   36 +++++++++++++++++++++++++++++++++++-
 fs/nfs/nfs3proc.c       |    1 +
 fs/nfs/nfs4_fs.h        |    1 +
 fs/nfs/nfs4proc.c       |    5 +++--
 fs/nfs/proc.c           |    1 +
 include/linux/nfs_fs.h  |    3 +++
 include/linux/nfs_xdr.h |    1 +
 8 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index f5fdd39..4ffb0c0 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -38,7 +38,7 @@
 
 #define NFSDBG_FACILITY		NFSDBG_FILE
 
-static int nfs_file_open(struct inode *, struct file *);
+static int nfs4_file_open(struct inode *, struct file *);
 static int nfs_file_release(struct inode *, struct file *);
 static loff_t nfs_file_llseek(struct file *file, loff_t offset, int origin);
 static int  nfs_file_mmap(struct file *, struct vm_area_struct *);
@@ -80,6 +80,27 @@ const struct file_operations nfs_file_operations = {
 	.setlease	= nfs_setlease,
 };
 
+#ifdef CONFIG_NFS_V4
+const struct file_operations ...
From: Trond Myklebust
Date: Friday, November 13, 2009 - 6:30 am

So what happens if the file has been renamed/deleted/replaced since the
lookup occurred? You basically end up with a state that corresponds to a
different file than the filp->f_path...

Cheers
  Trond

--

From: Jeff Layton
Date: Friday, November 13, 2009 - 7:13 am

On Fri, 13 Nov 2009 22:30:00 +0900

Good point...

I suppose I could add a check that the state->inode is the right one.
I'm not certain what to do if they aren't though...

I suppose we'd need to unhash the old dentry, and instantiate a new one,
but it is OK to swap out the dentry in the f_path with the new one?
Maybe I should just wait for your overhaul of the open codepath...
-- 
Jeff Layton <jlayton@redhat.com>
--

Previous thread: [PATCH] fuse: return -EAGAIN if the connection has not been init'ed by Josef Bacik on Thursday, November 12, 2009 - 2:01 pm. (12 messages)

Next thread: Union mount of block devices? by Andrea Gelmini on Friday, November 13, 2009 - 6:28 am. (2 messages)