[PATCH] nfsd: no need to open the file on a COMMIT call

[Jeff Layton]

There's no need to call nfsd_open when a COMMIT call comes in over the
wire. It doesn't really buy us anything to pass a filp into
vfs_fsync_range. Trond also pointed out that the write access check
for the open is wrong as well.

It's also problematic to do so. If the client doing the COMMIT call is
holding a delegation on the file, then that causes the server to recall
the delegation before it will proceed. The client (correctly) won't
return the delegation until the COMMIT succeeds. This leads to a
deadlock of sorts that can cause the client and server to stall out
and end up in state renewal.

The gory details of this situation are written up here:

https://bugzilla.redhat.com/show_bug.cgi?id=551028#c10

Fix this by not opening the file on a COMMIT call and simply pass
a NULL file pointer to vfs_fsync_range.

Signed-off-by: Jeff Layton <jlayton at redhat.com>
---
fs/nfsd/vfs.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index a11b0e8..2cdd9c0 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1156,7 +1156,6 @@ __be32
nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp,
loff_t offset, unsigned long count)
{
- struct file *file;
loff_t end = LLONG_MAX;
__be32 err = nfserr_inval;

@@ -1168,11 +1167,8 @@ nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp,
goto out;
}

- err = nfsd_open(rqstp, fhp, S_IFREG, NFSD_MAY_WRITE, &file);
- if (err)
- goto out;
if (EX_ISSYNC(fhp->fh_export)) {
- int err2 = vfs_fsync_range(file, file->f_path.dentry,
+ int err2 = vfs_fsync_range(NULL, fhp->fh_dentry,
offset, end, 0);

if (err2 != -EINVAL)
@@ -1180,8 +1176,6 @@ nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp,
else
err = nfserr_notsupp;
}
-
- nfsd_close(file);
out:
return err;
}

[Christoph Hellwig]

Post by Jeff Layton
There's no need to call nfsd_open when a COMMIT call comes in over the
wire. It doesn't really buy us anything to pass a filp into
vfs_fsync_range.

Yes, it does. Having the braindead calling convention with a possible
NULL file pointer is a real pain for all lower levels, and I have
patches ready to remove the last occurance of that, so please do not
introduce a new one.