All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Doug Nazar <nazard@dragoninc.ca>
Cc: "'David Woodhouse'" <David.Woodhouse@intel.com>,
	"'Al Viro'" <viro@zeniv.linux.org.uk>,
	linux-kernel@vger.kernel.org
Subject: Re: 2.6.28-rc3 truncates nfsd results
Date: Wed, 5 Nov 2008 18:07:15 -0500	[thread overview]
Message-ID: <20081105230715.GP1455@fieldses.org> (raw)
In-Reply-To: <20081105214655.GK1455@fieldses.org>

Seems good to me; I cleaned up commit message, as below, and I'll submit
in a few days.  If you didn't mind me adding a signed-off-by for you,
that'd also be good.  I hope we've got it right this time!

--b.

commit 19e1e96e0b51f9ad4cb0329e6b32a742cf9db713
Author: Doug Nazar <nazard@dragoninc.ca>
Date:   Wed Nov 5 06:16:28 2008 -0500

    Fix nfsd truncation of readdir results
    
    Commit 8d7c4203 "nfsd: fix failure to set eof in readdir in some
    situations" introduced a bug: on a directory in an exported ext3
    filesystem with dir_index unset, a READDIR will only return about 250
    entries, even if the directory was larger.
    
    Bisected it back to this commit; reverting it fixes the problem.
    
    It turns out that in this case ext3 reads a block at a time, then
    returns from readdir, which means we can end up with buf.full==0 but
    with more entries in the directory still to be read.  Before 8d7c4203
    (but after c002a6c797 "Optimise NFS readdir hack slightly"), this would
    cause us to return the READDIR result immediately, but with the eof bit
    unset.  That could cause a performance regression (because the client
    would need more roundtrips to the server to read the whole directory),
    but no loss in correctness, since the cleared eof bit caused the client
    to send another readdir.  After 8d7c4203, the setting of the eof bit
    made this a correctness problem.
    
    So, move nfserr_eof into the loop and remove the buf.full check so that
    we loop until buf.used==0.  The following seems to do the right thing
    and reduces the network traffic since we don't return a READDIR result
    until the buffer is full.
    
    Tested on an empty directory & large directory; eof is properly sent and
    there are no more short buffers.
    
    Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 848a03e..4433c8f 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1875,11 +1875,11 @@ static int nfsd_buffered_readdir(struct file *file, filldir_t func,
 		return -ENOMEM;
 
 	offset = *offsetp;
-	cdp->err = nfserr_eof; /* will be cleared on successful read */
 
 	while (1) {
 		unsigned int reclen;
 
+		cdp->err = nfserr_eof; /* will be cleared on successful read */
 		buf.used = 0;
 		buf.full = 0;
 
@@ -1912,9 +1912,6 @@ static int nfsd_buffered_readdir(struct file *file, filldir_t func,
 			de = (struct buffered_dirent *)((char *)de + reclen);
 		}
 		offset = vfs_llseek(file, 0, SEEK_CUR);
-		cdp->err = nfserr_eof;
-		if (!buf.full)
-			break;
 	}
 
  done:

  reply	other threads:[~2008-11-05 23:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-04 18:27 2.6.28-rc3 truncates nfsd results Doug Nazar
2008-11-04 18:43 ` Jeff Garzik
2008-11-04 19:03   ` David Woodhouse
2008-11-04 19:32     ` Doug Nazar
2008-11-04 20:08       ` David Woodhouse
2008-11-09 19:09   ` J. Bruce Fields
2008-11-04 19:36 ` J. Bruce Fields
2008-11-04 22:30 ` J. Bruce Fields
2008-11-05 11:16   ` Doug Nazar
2008-11-05 21:46     ` J. Bruce Fields
2008-11-05 23:07       ` J. Bruce Fields [this message]
2008-11-06  5:21       ` Doug Nazar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20081105230715.GP1455@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=David.Woodhouse@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nazard@dragoninc.ca \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.