Messages in this thread | | | Date | Thu, 28 Jan 1999 23:29:34 -0500 (EST) | From | David Mansfield <> | Subject | Re: [PATCH] another knfsd reply cache bugfix |
| |
H.J.,
> > --- linux/fs/nfsd/nfscache.c.orig Fri Jan 8 13:04:29 1999 > > +++ linux/fs/nfsd/nfscache.c Tue Jan 12 08:08:56 1999 > > @@ -268,8 +268,9 @@ > > if (!(rp = rqstp->rq_cacherep) || cache_disabled) > > return; > > > > + len = resp->len - (statp - resp->base); > > /* Don't cache excessive amounts of data and XDR failures */ > > - if (!statp || (len = resp->buf - statp) > (256 >> 2)) { > > + if (!statp || len > (256 >> 2)) { > > rp->c_state = RC_UNUSED; > > return; > > } > > > I saw this patch in Linux 2.2.0. But I don't think it is all correct. > Everytime when my NFS client mounts the server, the NFS server > complaints > > nfsd: RC_REPLSTAT/reply len 35! > nfsd: RC_REPLSTAT/reply len 36! > nfsd: RC_REPLSTAT/reply len 20! > nfsd: RC_REPLSTAT/reply len 21! > nfsd: RC_REPLSTAT/reply len 26! > > Any ideas? BTW, do you have a testcase for the bug you tried to fix? > > Thanks. > > > H.J. >
Yes, I think there is a perfectly good explanation for this message (which I also got). In fs/nfsd/nfsproc.c we set up the svc_procedure structure, which configures how the various replies will be cached. There are three choices: RC_NOCACHE (obvious), RC_REPLSTAT (we cache just a status code) and RP_REPLBUFF (we cache the entire reply buffer).
Now, before the patch (which I *also* was surprised to see in 2.2.0!) ALL cached replies would have been seen as length 1. This is true, since in xdr_ressize_check in fs/nfsd/nfsxdr.c we were 'fixing up' the buf->len but not the buf->buf member, which was being used in the length calculation. So the old length calculation was ALWAYS turning up 1. Thus the bug(s) was/were hidden.
Take a look at the svc_procedure struct in fs/nfsd/nfsproc.c. In all cases but one, where the RC_REPLSTAT cachetype is specified, the encode proc is the void. But for:
PROC(readdir, readdirargs, readdirres, none, RC_REPLSTAT),
the encode function is not the void func! It is readdirres So we are guaranteed of having a length > 1. So I think this at least is wrong, and should be changed to RC_REPLBUFF. I should repeat that I know fairly little about this and would love it if you gave it a second look...
As for a testcase: mount the nfs volume from a Solaris (2.5 or 2.6) box with the 'noac' option (tickles the bug better). Make sure you are running on a network that will drop a packet or two, after all the reply cache is only used to retransmit dropped replies. Then run 'bonnie' on the nfs mounted dir. This triggers it EVERY TIME for me. I get:
Jan 8 11:46:20 marvin unix: NFS write error on host spike: I/O error. Jan 8 11:46:20 marvin unix: (file handle: 607e75c0 6780000 1780000 4300000 4300 000 2000000 0 0)
in my Solaris logs, and bonnie terminates with I/O error.
And tcpdump does indeed show a fishy packet, with the same XID as the previous one, but the length of the reply has gone from 96 to 28... I am fairly sure that the patch is just exposing some other problems.
David
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |