Re: [PATCH] Short write in nfsd becomes a full write to the client

Re: [PATCH] Short write in nfsd becomes a full write to the client

[Dale Stimson]

On Thu, 5 Mar 2009 20:16:14 -0500, David Shaw <dshaw-wh+mT2OhP0WF0gnf/s2wvA@public.gmane.org> wrote:
> If a filesystem being written to via NFS returns a short write count
> (as opposed to an error) to nfsd, nfsd treats that as a success for
> the entire write, rather than the short count that actually succeeded.
> 
> For example, given a 8192 byte write, if the underlying filesystem
> only writes 4096 bytes, nfsd will ack back to the nfs client that all
> 8192 bytes were written.  The nfs client does have retry logic for
> short writes, but this is never called as the client is told the
> complete write succeeded.
...
> Here is a patch to properly return the short write count to the
> client.
[patch elided]

I bring this to your attention so you may, if you choose, look into
this further:

Problem synopsis:
An old client (running RHL 9 with kernel "2.4.20-43.9.legacy")
attempts to seek on a file mounted over nfs.  The operation fails
with "Illegal seek" or "Input/Output error".  The server is running
Fedora 11 kernel-PAE-2.6.29.5-191.fc11.i686, which includes the
short write patch.  When this kernel is re-built without the short
write patch, everything works as before.

Detais are at
https://bugzilla.redhat.com/show_bug.cgi?id=508174

Caveats: I am specifically referring to the patch file
"linux-2.6-nfsd-report-short-writes.patch" as newly included in
Fedora's file kernel-2.6.29.5-191.fc11.src.rpm .  I can't vouch
that that patch file is identical to what was posted to this list
or merged for 2.6.30-rc1.

(As an aside, in this case, the client was attempting a simple gcc
compile and link.  The failing programs (invoked by fcc) were the
assember ("as)" and "ld".)

Re: [PATCH] Short write in nfsd becomes a full write to the client

[J. Bruce Fields]

On Fri, Jun 26, 2009 at 09:24:49PM -0700, Dale Stimson wrote:
> On Thu, 5 Mar 2009 20:16:14 -0500, David Shaw <dshaw-wh+mT2OhP0WF0gnf/s2wvA@public.gmane.org> wrote:
> > If a filesystem being written to via NFS returns a short write count
> > (as opposed to an error) to nfsd, nfsd treats that as a success for
> > the entire write, rather than the short count that actually succeeded.
> > 
> > For example, given a 8192 byte write, if the underlying filesystem
> > only writes 4096 bytes, nfsd will ack back to the nfs client that all
> > 8192 bytes were written.  The nfs client does have retry logic for
> > short writes, but this is never called as the client is told the
> > complete write succeeded.
> ...
> > Here is a patch to properly return the short write count to the
> > client.
> [patch elided]
> 
> I bring this to your attention so you may, if you choose, look into
> this further:
> 
> Problem synopsis:
> An old client (running RHL 9 with kernel "2.4.20-43.9.legacy")
> attempts to seek on a file mounted over nfs.  The operation fails
> with "Illegal seek" or "Input/Output error".  The server is running
> Fedora 11 kernel-PAE-2.6.29.5-191.fc11.i686, which includes the
> short write patch.  When this kernel is re-built without the short
> write patch, everything works as before.

Does that server kernel have some version of
a0d24b295aed7a9daf4ca36bd4784e4d40f82303 "nfsd: fix hung up of nfs
client while sync write data to nfs server" applied?

If not, would it be possible to get a network trace?  (Run "tcpdump -s0
-wTMP", then run the test case, then kill tcpdump and mail me a copy of
TMP.)

--b.

> 
> Detais are at
> https://bugzilla.redhat.com/show_bug.cgi?id=508174
> 
> Caveats: I am specifically referring to the patch file
> "linux-2.6-nfsd-report-short-writes.patch" as newly included in
> Fedora's file kernel-2.6.29.5-191.fc11.src.rpm .  I can't vouch
> that that patch file is identical to what was posted to this list
> or merged for 2.6.30-rc1.
> 
> (As an aside, in this case, the client was attempting a simple gcc
> compile and link.  The failing programs (invoked by fcc) were the
> assember ("as)" and "ld".)

Re: [PATCH] Short write in nfsd becomes a full write to the client

[Dale Stimson]

On Mon, Jun 29, 2009 at 10:59:52AM -0400, J. Bruce Fields wrote:
> On Fri, Jun 26, 2009 at 09:24:49PM -0700, Dale Stimson wrote:
> > On Thu, 5 Mar 2009 20:16:14 -0500, David Shaw <dshaw-wh+mT2OhP0WF0gnf/s2wvA@public.gmane.org> wrote:
> > > If a filesystem being written to via NFS returns a short write count
> > > (as opposed to an error) to nfsd, nfsd treats that as a success for
> > > the entire write, rather than the short count that actually succeeded.
> > > 
> > > For example, given a 8192 byte write, if the underlying filesystem
> > > only writes 4096 bytes, nfsd will ack back to the nfs client that all
> > > 8192 bytes were written.  The nfs client does have retry logic for
> > > short writes, but this is never called as the client is told the
> > > complete write succeeded.
> > ...
> > > Here is a patch to properly return the short write count to the
> > > client.
> > [patch elided]
> > 
> > I bring this to your attention so you may, if you choose, look into
> > this further:
> > 
> > Problem synopsis:
> > An old client (running RHL 9 with kernel "2.4.20-43.9.legacy")
> > attempts to seek on a file mounted over nfs.  The operation fails
> > with "Illegal seek" or "Input/Output error".  The server is running
> > Fedora 11 kernel-PAE-2.6.29.5-191.fc11.i686, which includes the
> > short write patch.  When this kernel is re-built without the short
> > write patch, everything works as before.
> 
> Does that server kernel have some version of
> a0d24b295aed7a9daf4ca36bd4784e4d40f82303 "nfsd: fix hung up of nfs
> client while sync write data to nfs server" applied?

No.  As far as I can see that patch has been merged only in 2.6.30 and is not in any of the 2.6.29.? releases.  There is definitely no Fedora-specific version of that patch present.  The only Fedora-specific patch related to NFS applied in kernel-2.6.29.5-191.fc11.src.rpm is linux-2.6-nfsd-report-short-writes.patch.

> If not, would it be possible to get a network trace?  (Run "tcpdump -s0
> -wTMP", then run the test case, then kill tcpdump and mail me a copy of
> TMP.)
> 
> --b.

Capture done, to be emailed off-list.

> > Detais are at
> > https://bugzilla.redhat.com/show_bug.cgi?id=508174
> > 
> > Caveats: I am specifically referring to the patch file
> > "linux-2.6-nfsd-report-short-writes.patch" as newly included in
> > Fedora's file kernel-2.6.29.5-191.fc11.src.rpm .  I can't vouch
> > that that patch file is identical to what was posted to this list
> > or merged for 2.6.30-rc1.
> > 
> > (As an aside, in this case, the client was attempting a simple gcc
> > compile and link.  The failing programs (invoked by fcc) were the
> > assember ("as)" and "ld".)

Re: [PATCH] Short write in nfsd becomes a full write to the client

[J. Bruce Fields]

On Mon, Jun 29, 2009 at 12:29:51PM -0700, Dale Stimson wrote:
> On Mon, Jun 29, 2009 at 10:59:52AM -0400, J. Bruce Fields wrote:
> > On Fri, Jun 26, 2009 at 09:24:49PM -0700, Dale Stimson wrote:
> > > On Thu, 5 Mar 2009 20:16:14 -0500, David Shaw <dshaw-wh+mT2OhP0WF0gnf/s2wvA@public.gmane.org> wrote:
> > > > If a filesystem being written to via NFS returns a short write count
> > > > (as opposed to an error) to nfsd, nfsd treats that as a success for
> > > > the entire write, rather than the short count that actually succeeded.
> > > > 
> > > > For example, given a 8192 byte write, if the underlying filesystem
> > > > only writes 4096 bytes, nfsd will ack back to the nfs client that all
> > > > 8192 bytes were written.  The nfs client does have retry logic for
> > > > short writes, but this is never called as the client is told the
> > > > complete write succeeded.
> > > ...
> > > > Here is a patch to properly return the short write count to the
> > > > client.
> > > [patch elided]
> > > 
> > > I bring this to your attention so you may, if you choose, look into
> > > this further:
> > > 
> > > Problem synopsis:
> > > An old client (running RHL 9 with kernel "2.4.20-43.9.legacy")
> > > attempts to seek on a file mounted over nfs.  The operation fails
> > > with "Illegal seek" or "Input/Output error".  The server is running
> > > Fedora 11 kernel-PAE-2.6.29.5-191.fc11.i686, which includes the
> > > short write patch.  When this kernel is re-built without the short
> > > write patch, everything works as before.
> > 
> > Does that server kernel have some version of
> > a0d24b295aed7a9daf4ca36bd4784e4d40f82303 "nfsd: fix hung up of nfs
> > client while sync write data to nfs server" applied?
> 
> No.  As far as I can see that patch has been merged only in 2.6.30 and is not in any of the 2.6.29.? releases.  There is definitely no Fedora-specific version of that patch present.  The only Fedora-specific patch related to NFS applied in kernel-2.6.29.5-191.fc11.src.rpm is linux-2.6-nfsd-report-short-writes.patch.

That patch fixes a problem with the showrt write patch, so it would be
worth retesting with it applied.

--b.

Re: [PATCH] Short write in nfsd becomes a full write to the client

[J. Bruce Fields]

On Mon, Jun 29, 2009 at 03:47:11PM -0400, bfields wrote:
> On Mon, Jun 29, 2009 at 12:29:51PM -0700, Dale Stimson wrote:
> > On Mon, Jun 29, 2009 at 10:59:52AM -0400, J. Bruce Fields wrote:
> > > On Fri, Jun 26, 2009 at 09:24:49PM -0700, Dale Stimson wrote:
> > > > On Thu, 5 Mar 2009 20:16:14 -0500, David Shaw <dshaw-wh+mT2OhP0WF0gnf/s2wvA@public.gmane.org> wrote:
> > > > > If a filesystem being written to via NFS returns a short write count
> > > > > (as opposed to an error) to nfsd, nfsd treats that as a success for
> > > > > the entire write, rather than the short count that actually succeeded.
> > > > > 
> > > > > For example, given a 8192 byte write, if the underlying filesystem
> > > > > only writes 4096 bytes, nfsd will ack back to the nfs client that all
> > > > > 8192 bytes were written.  The nfs client does have retry logic for
> > > > > short writes, but this is never called as the client is told the
> > > > > complete write succeeded.
> > > > ...
> > > > > Here is a patch to properly return the short write count to the
> > > > > client.
> > > > [patch elided]
> > > > 
> > > > I bring this to your attention so you may, if you choose, look into
> > > > this further:
> > > > 
> > > > Problem synopsis:
> > > > An old client (running RHL 9 with kernel "2.4.20-43.9.legacy")
> > > > attempts to seek on a file mounted over nfs.  The operation fails
> > > > with "Illegal seek" or "Input/Output error".  The server is running
> > > > Fedora 11 kernel-PAE-2.6.29.5-191.fc11.i686, which includes the
> > > > short write patch.  When this kernel is re-built without the short
> > > > write patch, everything works as before.
> > > 
> > > Does that server kernel have some version of
> > > a0d24b295aed7a9daf4ca36bd4784e4d40f82303 "nfsd: fix hung up of nfs
> > > client while sync write data to nfs server" applied?
> > 
> > No.  As far as I can see that patch has been merged only in 2.6.30 and is not in any of the 2.6.29.? releases.  There is definitely no Fedora-specific version of that patch present.  The only Fedora-specific patch related to NFS applied in kernel-2.6.29.5-191.fc11.src.rpm is linux-2.6-nfsd-report-short-writes.patch.
> 
> That patch fixes a problem with the showrt write patch, so it would be
> worth retesting with it applied.

After looking at the trace: yes, looks like the same problem.  (The
(succesful) FILE_SYNC write request returned with a 0 length.)  So
retest with the new patch applied, then poke the Fedora people to apply
it as well.

--b.

[solved] Re: [PATCH] Short write in nfsd becomes a full write to the client

[Dale Stimson]

On Mon, Jun 29, 2009 at 03:49:25PM -0400, J. Bruce Fields wrote:
> On Mon, Jun 29, 2009 at 03:47:11PM -0400, bfields wrote:
> > On Mon, Jun 29, 2009 at 12:29:51PM -0700, Dale Stimson wrote:
> > > On Mon, Jun 29, 2009 at 10:59:52AM -0400, J. Bruce Fields wrote:
> > > > On Fri, Jun 26, 2009 at 09:24:49PM -0700, Dale Stimson wrote:
> > > > > On Thu, 5 Mar 2009 20:16:14 -0500, David Shaw <dshaw@jabberwocky.com> wrote:
> > > > > > If a filesystem being written to via NFS returns a short write count
> > > > > > (as opposed to an error) to nfsd, nfsd treats that as a success for
> > > > > > the entire write, rather than the short count that actually succeeded.
> > > > > > 
> > > > > > For example, given a 8192 byte write, if the underlying filesystem
> > > > > > only writes 4096 bytes, nfsd will ack back to the nfs client that all
> > > > > > 8192 bytes were written.  The nfs client does have retry logic for
> > > > > > short writes, but this is never called as the client is told the
> > > > > > complete write succeeded.
> > > > > ...
> > > > > > Here is a patch to properly return the short write count to the
> > > > > > client.
> > > > > [patch elided]
> > > > > 
> > > > > I bring this to your attention so you may, if you choose, look into
> > > > > this further:
> > > > > 
> > > > > Problem synopsis:
> > > > > An old client (running RHL 9 with kernel "2.4.20-43.9.legacy")
> > > > > attempts to seek on a file mounted over nfs.  The operation fails
> > > > > with "Illegal seek" or "Input/Output error".  The server is running
> > > > > Fedora 11 kernel-PAE-2.6.29.5-191.fc11.i686, which includes the
> > > > > short write patch.  When this kernel is re-built without the short
> > > > > write patch, everything works as before.
> > > > 
> > > > Does that server kernel have some version of
> > > > a0d24b295aed7a9daf4ca36bd4784e4d40f82303 "nfsd: fix hung up of nfs
> > > > client while sync write data to nfs server" applied?
> > > 
> > > No.  As far as I can see that patch has been merged only in 2.6.30 and is not in any of the 2.6.29.? releases.  There is definitely no Fedora-specific version of that patch present.  The only Fedora-specific patch related to NFS applied in kernel-2.6.29.5-191.fc11.src.rpm is linux-2.6-nfsd-report-short-writes.patch.
> > 
> > That patch fixes a problem with the showrt write patch, so it would be
> > worth retesting with it applied.
> 
> After looking at the trace: yes, looks like the same problem.  (The
> (succesful) FILE_SYNC write request returned with a 0 length.)  So
> retest with the new patch applied, then poke the Fedora people to apply
> it as well.
> 
> --b.

Success.

I applied patch a0d24b295aed7a9daf4ca36bd4784e4d40f82303
"nfsd: fix hung up of nfs client while sync write data to nfs server"
to the otherwise unmodified Fedora 11 kernel 2.6.29.5-191.fc11.i686.PAE
and the problem was resolved.

Thank you for your help.  I will update
https://bugzilla.redhat.com/show_bug.cgi?id=508174
and suggest that the patch be applied to future Fedora 2.6.29-based kernels.

Re: [solved] Re: [PATCH] Short write in nfsd becomes a full write to the client

[J. Bruce Fields]

On Tue, Jun 30, 2009 at 08:22:09AM -0700, Dale Stimson wrote:
> On Mon, Jun 29, 2009 at 03:49:25PM -0400, J. Bruce Fields wrote:
> > After looking at the trace: yes, looks like the same problem.  (The
> > (succesful) FILE_SYNC write request returned with a 0 length.)  So
> > retest with the new patch applied, then poke the Fedora people to apply
> > it as well.
> > 
> > --b.
> 
> Success.
> 
> I applied patch a0d24b295aed7a9daf4ca36bd4784e4d40f82303
> "nfsd: fix hung up of nfs client while sync write data to nfs server"
> to the otherwise unmodified Fedora 11 kernel 2.6.29.5-191.fc11.i686.PAE
> and the problem was resolved.
> 
> Thank you for your help.  I will update
> https://bugzilla.redhat.com/show_bug.cgi?id=508174
> and suggest that the patch be applied to future Fedora 2.6.29-based kernels.

Thanks for following up.--b.

[PATCH] Short write in nfsd becomes a full write to the client

[David Shaw]

If a filesystem being written to via NFS returns a short write count
(as opposed to an error) to nfsd, nfsd treats that as a success for
the entire write, rather than the short count that actually succeeded.

For example, given a 8192 byte write, if the underlying filesystem
only writes 4096 bytes, nfsd will ack back to the nfs client that all
8192 bytes were written.  The nfs client does have retry logic for
short writes, but this is never called as the client is told the
complete write succeeded.

There are probably other ways it could happen, but in my case it
happened with a fuse (filesystem in userspace) filesystem which can
rather easily have a partial write.

Here is a patch to properly return the short write count to the
client.

Signed-off-by: David Shaw <dshaw-wh+mT2OhP0WF0gnf/s2wvA@public.gmane.org>
---

 fs/nfsd/nfs3proc.c        |    5 +++--
 fs/nfsd/nfs4proc.c        |    7 +++++--
 fs/nfsd/nfsproc.c         |    3 ++-
 fs/nfsd/vfs.c             |   12 +++++++-----
 include/linux/nfsd/nfsd.h |    2 +-
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 9dbd2eb..9f262b3 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -202,6 +202,7 @@ nfsd3_proc_write(struct svc_rqst *rqstp, struct nfsd3_writeargs *argp,
 					 struct nfsd3_writeres  *resp)
 {
 	__be32	nfserr;
+	unsigned long cnt = argp->len;
 
 	dprintk("nfsd: WRITE(3)    %s %d bytes at %ld%s\n",
 				SVCFH_fmt(&argp->fh),
@@ -214,9 +215,9 @@ nfsd3_proc_write(struct svc_rqst *rqstp, struct nfsd3_writeargs *argp,
 	nfserr = nfsd_write(rqstp, &resp->fh, NULL,
 				   argp->offset,
 				   rqstp->rq_vec, argp->vlen,
-				   argp->len,
+				   &cnt,
 				   &resp->committed);
-	resp->count = argp->count;
+	resp->count = cnt;
 	RETURN_STATUS(nfserr);
 }
 
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9fa60a3..8ed1b93 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -685,6 +685,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct file *filp = NULL;
 	u32 *p;
 	__be32 status = nfs_ok;
+	unsigned long cnt;
 
 	/* no need to check permission - this will be done in nfsd_write() */
 
@@ -703,7 +704,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		return status;
 	}
 
-	write->wr_bytes_written = write->wr_buflen;
+	cnt = write->wr_buflen;
 	write->wr_how_written = write->wr_stable_how;
 	p = (u32 *)write->wr_verifier.data;
 	*p++ = nfssvc_boot.tv_sec;
@@ -711,10 +712,12 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	status =  nfsd_write(rqstp, &cstate->current_fh, filp,
 			     write->wr_offset, rqstp->rq_vec, write->wr_vlen,
-			     write->wr_buflen, &write->wr_how_written);
+			     &cnt, &write->wr_how_written);
 	if (filp)
 		fput(filp);
 
+	write->wr_bytes_written = cnt;
+
 	if (status == nfserr_symlink)
 		status = nfserr_inval;
 	return status;
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 6f7f263..e298e26 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -180,6 +180,7 @@ nfsd_proc_write(struct svc_rqst *rqstp, struct nfsd_writeargs *argp,
 {
 	__be32	nfserr;
 	int	stable = 1;
+	unsigned long cnt = argp->len;
 
 	dprintk("nfsd: WRITE    %s %d bytes at %d\n",
 		SVCFH_fmt(&argp->fh),
@@ -188,7 +189,7 @@ nfsd_proc_write(struct svc_rqst *rqstp, struct nfsd_writeargs *argp,
 	nfserr = nfsd_write(rqstp, fh_copy(&resp->fh, &argp->fh), NULL,
 				   argp->offset,
 				   rqstp->rq_vec, argp->vlen,
-				   argp->len,
+			           &cnt,
 				   &stable);
 	return nfsd_return_attrs(nfserr, resp);
 }
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 6e50aaa..d3c8429 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -960,7 +960,7 @@ static void kill_suid(struct dentry *dentry)
 static __be32
 nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
 				loff_t offset, struct kvec *vec, int vlen,
-	   			unsigned long cnt, int *stablep)
+	   			unsigned long *cnt, int *stablep)
 {
 	struct svc_export	*exp;
 	struct dentry		*dentry;
@@ -974,7 +974,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
 	err = nfserr_perm;
 
 	if ((fhp->fh_export->ex_flags & NFSEXP_MSNFS) &&
-		(!lock_may_write(file->f_path.dentry->d_inode, offset, cnt)))
+		(!lock_may_write(file->f_path.dentry->d_inode, offset, *cnt)))
 		goto out;
 #endif
 
@@ -1006,7 +1006,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
 	host_err = vfs_writev(file, (struct iovec __user *)vec, vlen, &offset);
 	set_fs(oldfs);
 	if (host_err >= 0) {
-		nfsdstats.io_write += cnt;
+		nfsdstats.io_write += host_err;
 		fsnotify_modify(file->f_path.dentry);
 	}
 
@@ -1051,8 +1051,10 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
 	}
 
 	dprintk("nfsd: write complete host_err=%d\n", host_err);
-	if (host_err >= 0)
+	if (host_err >= 0) {
 		err = 0;
+		*cnt = host_err;
+	}
 	else 
 		err = nfserrno(host_err);
 out:
@@ -1095,7 +1097,7 @@ out:
  */
 __be32
 nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
-		loff_t offset, struct kvec *vec, int vlen, unsigned long cnt,
+		loff_t offset, struct kvec *vec, int vlen, unsigned long *cnt,
 		int *stablep)
 {
 	__be32			err = 0;
diff --git a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h
index e19f459..eefa241 100644
--- a/include/linux/nfsd/nfsd.h
+++ b/include/linux/nfsd/nfsd.h
@@ -105,7 +105,7 @@ void		nfsd_close(struct file *);
 __be32 		nfsd_read(struct svc_rqst *, struct svc_fh *, struct file *,
 				loff_t, struct kvec *, int, unsigned long *);
 __be32 		nfsd_write(struct svc_rqst *, struct svc_fh *,struct file *,
-				loff_t, struct kvec *,int, unsigned long, int *);
+				loff_t, struct kvec *,int, unsigned long *, int *);
 __be32		nfsd_readlink(struct svc_rqst *, struct svc_fh *,
 				char *, int *);
 __be32		nfsd_symlink(struct svc_rqst *, struct svc_fh *,

Re: [PATCH] Short write in nfsd becomes a full write to the client

[J. Bruce Fields]

On Thu, Mar 05, 2009 at 08:16:14PM -0500, David Shaw wrote:
> If a filesystem being written to via NFS returns a short write count
> (as opposed to an error) to nfsd, nfsd treats that as a success for
> the entire write, rather than the short count that actually succeeded.
> 
> For example, given a 8192 byte write, if the underlying filesystem
> only writes 4096 bytes, nfsd will ack back to the nfs client that all
> 8192 bytes were written.  The nfs client does have retry logic for
> short writes, but this is never called as the client is told the
> complete write succeeded.
> 
> There are probably other ways it could happen, but in my case it
> happened with a fuse (filesystem in userspace) filesystem which can
> rather easily have a partial write.

Got it.

> 
> Here is a patch to properly return the short write count to the
> client.

Applied for-2.6.30 with one minor style fix, noted below.

Looks good, thanks for your persistance.--b.

> 
> Signed-off-by: David Shaw <dshaw-wh+mT2OhP0WF0gnf/s2wvA@public.gmane.org>
> ---
> 
>  fs/nfsd/nfs3proc.c        |    5 +++--
>  fs/nfsd/nfs4proc.c        |    7 +++++--
>  fs/nfsd/nfsproc.c         |    3 ++-
>  fs/nfsd/vfs.c             |   12 +++++++-----
>  include/linux/nfsd/nfsd.h |    2 +-
>  5 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> index 9dbd2eb..9f262b3 100644
> --- a/fs/nfsd/nfs3proc.c
> +++ b/fs/nfsd/nfs3proc.c
> @@ -202,6 +202,7 @@ nfsd3_proc_write(struct svc_rqst *rqstp, struct nfsd3_writeargs *argp,
>  					 struct nfsd3_writeres  *resp)
>  {
>  	__be32	nfserr;
> +	unsigned long cnt = argp->len;
>  
>  	dprintk("nfsd: WRITE(3)    %s %d bytes at %ld%s\n",
>  				SVCFH_fmt(&argp->fh),
> @@ -214,9 +215,9 @@ nfsd3_proc_write(struct svc_rqst *rqstp, struct nfsd3_writeargs *argp,
>  	nfserr = nfsd_write(rqstp, &resp->fh, NULL,
>  				   argp->offset,
>  				   rqstp->rq_vec, argp->vlen,
> -				   argp->len,
> +				   &cnt,
>  				   &resp->committed);
> -	resp->count = argp->count;
> +	resp->count = cnt;
>  	RETURN_STATUS(nfserr);
>  }
>  
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 9fa60a3..8ed1b93 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -685,6 +685,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	struct file *filp = NULL;
>  	u32 *p;
>  	__be32 status = nfs_ok;
> +	unsigned long cnt;
>  
>  	/* no need to check permission - this will be done in nfsd_write() */
>  
> @@ -703,7 +704,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		return status;
>  	}
>  
> -	write->wr_bytes_written = write->wr_buflen;
> +	cnt = write->wr_buflen;
>  	write->wr_how_written = write->wr_stable_how;
>  	p = (u32 *)write->wr_verifier.data;
>  	*p++ = nfssvc_boot.tv_sec;
> @@ -711,10 +712,12 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  	status =  nfsd_write(rqstp, &cstate->current_fh, filp,
>  			     write->wr_offset, rqstp->rq_vec, write->wr_vlen,
> -			     write->wr_buflen, &write->wr_how_written);
> +			     &cnt, &write->wr_how_written);
>  	if (filp)
>  		fput(filp);
>  
> +	write->wr_bytes_written = cnt;
> +
>  	if (status == nfserr_symlink)
>  		status = nfserr_inval;
>  	return status;
> diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
> index 6f7f263..e298e26 100644
> --- a/fs/nfsd/nfsproc.c
> +++ b/fs/nfsd/nfsproc.c
> @@ -180,6 +180,7 @@ nfsd_proc_write(struct svc_rqst *rqstp, struct nfsd_writeargs *argp,
>  {
>  	__be32	nfserr;
>  	int	stable = 1;
> +	unsigned long cnt = argp->len;
>  
>  	dprintk("nfsd: WRITE    %s %d bytes at %d\n",
>  		SVCFH_fmt(&argp->fh),
> @@ -188,7 +189,7 @@ nfsd_proc_write(struct svc_rqst *rqstp, struct nfsd_writeargs *argp,
>  	nfserr = nfsd_write(rqstp, fh_copy(&resp->fh, &argp->fh), NULL,
>  				   argp->offset,
>  				   rqstp->rq_vec, argp->vlen,
> -				   argp->len,
> +			           &cnt,
>  				   &stable);
>  	return nfsd_return_attrs(nfserr, resp);
>  }
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 6e50aaa..d3c8429 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -960,7 +960,7 @@ static void kill_suid(struct dentry *dentry)
>  static __be32
>  nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
>  				loff_t offset, struct kvec *vec, int vlen,
> -	   			unsigned long cnt, int *stablep)
> +	   			unsigned long *cnt, int *stablep)
>  {
>  	struct svc_export	*exp;
>  	struct dentry		*dentry;
> @@ -974,7 +974,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
>  	err = nfserr_perm;
>  
>  	if ((fhp->fh_export->ex_flags & NFSEXP_MSNFS) &&
> -		(!lock_may_write(file->f_path.dentry->d_inode, offset, cnt)))
> +		(!lock_may_write(file->f_path.dentry->d_inode, offset, *cnt)))
>  		goto out;
>  #endif
>  
> @@ -1006,7 +1006,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
>  	host_err = vfs_writev(file, (struct iovec __user *)vec, vlen, &offset);
>  	set_fs(oldfs);
>  	if (host_err >= 0) {
> -		nfsdstats.io_write += cnt;
> +		nfsdstats.io_write += host_err;
>  		fsnotify_modify(file->f_path.dentry);
>  	}
>  
> @@ -1051,8 +1051,10 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
>  	}
>  
>  	dprintk("nfsd: write complete host_err=%d\n", host_err);
> -	if (host_err >= 0)
> +	if (host_err >= 0) {
>  		err = 0;
> +		*cnt = host_err;
> +	}
>  	else 

That should be } else

--b.

>  		err = nfserrno(host_err);
>  out:
> @@ -1095,7 +1097,7 @@ out:
>   */
>  __be32
>  nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
> -		loff_t offset, struct kvec *vec, int vlen, unsigned long cnt,
> +		loff_t offset, struct kvec *vec, int vlen, unsigned long *cnt,
>  		int *stablep)
>  {
>  	__be32			err = 0;
> diff --git a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h
> index e19f459..eefa241 100644
> --- a/include/linux/nfsd/nfsd.h
> +++ b/include/linux/nfsd/nfsd.h
> @@ -105,7 +105,7 @@ void		nfsd_close(struct file *);
>  __be32 		nfsd_read(struct svc_rqst *, struct svc_fh *, struct file *,
>  				loff_t, struct kvec *, int, unsigned long *);
>  __be32 		nfsd_write(struct svc_rqst *, struct svc_fh *,struct file *,
> -				loff_t, struct kvec *,int, unsigned long, int *);
> +				loff_t, struct kvec *,int, unsigned long *, int *);
>  __be32		nfsd_readlink(struct svc_rqst *, struct svc_fh *,
>  				char *, int *);
>  __be32		nfsd_symlink(struct svc_rqst *, struct svc_fh *,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html