lkml.org 
[lkml]   [2008]   [Nov]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: 2.6.28-rc3 truncates nfsd results
From
On Wed, Nov 05, 2008 at 06:16:28AM -0500, Doug Nazar wrote:
>
>
> > From: J. Bruce Fields [mailto:bfields@fieldses.org]
> > On Tue, Nov 04, 2008 at 01:27:23PM -0500, Doug Nazar wrote:
> > > Commit 8d7c4203 "nfsd: fix failure to set eof in readdir in some situations"
> > > breaks the nfsd server. Bisected it back to this commit and reverting it
> > > fixes the problem.
> > >
> > > However, it only happens on certain machines even with the same kernel &
> > > filesystem (ext3). I've two groups of similar computers, each group running
> > > identical kernels. The ones listing only ~250 files are of course in error.
> > > Eldritch is running 2.6.28-rc3 with that commit reverted. With 2.8.28-rc3 it
> > > showed the incorrect number.
> >
> > Well, that's strange; it must be staring me in the face, but I don't see
> > the problem (and can't reproduce it). Can you watch for the readdir
> > with wireshark and see if it's returning an error on the readdir? Or is
> > it just returning succesfully with eof set after the first ~250
> > entries?
>
> Ok, think I've figured it out.

Awesome, good job working this out.

Anyone know where to find the best documentation of the vfs_readdir
semantics? I obviously didn't understand it as well as I should.

> The computers showing the issue are not using dir_index. This causes
> ext3 to read a block at a time, which then means we can end up with
> buf.full==0 but not finished reading the directory. Before 8d7c4203,
> we'd always get called again because we never set nfserr_eof which
> papered over it.

OK, so if I'm understanding you correctly: there was also (as of
c002a6c797 "Optimise NFS readdir hack slightly"?) a performance
regression which could force the client to do more round trips to the
server to read the whole directory?

> I think the correct solution is to 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 now ensure each buffer is full.
>
> Tested on an empty directory & large directory, eof is properly sent
> and no short buffers.

Thanks, looking....

--b.

>
> 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:
>
>
>


\
 
 \ /
  Last update: 2008-11-05 22:49    [W:0.038 / U:0.832 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site