Messages in this thread Patch in this message |  | | Date | Wed, 5 Nov 2008 16:46:55 -0500 | Subject | Re: 2.6.28-rc3 truncates nfsd results | From | "J. Bruce Fields" <> |
| |
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: > > >
|  |