Re: [PATCH] another knfsd reply cache bugfix

David Mansfield (david@cobite.com)
Thu, 28 Jan 1999 23:29:34 -0500 (EST)


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/