On Tue, 07 Jun 2022, Chuck Lever III wrote:
> > On Jun 6, 2022, at 10:35 PM, Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote:
> >
> >> On Jun 6, 2022, at 8:59 PM, NeilBrown <neilb@xxxxxxx> wrote:
> >> We then assign frag{1,2}bytes and have another chunk of code that looks
> >> wrong to me. I'd like
> >>
> >> if (xdr->iov) {
> >> xdr->iov->iov_len += frag1bytes;
> >> xdr->iov = NULL;
> >> } else {
> >> xdr->buf->page_len += frag1bytes;
> >> xdr->page_ptr++;
> >> }
> >>
> >> Note that this changes the code NOT to increment pagE_ptr if iov was not
> >> NULL. I cannot see how it would be correct to do that. Presumably this
> >> code is never called with iov != NULL ???
> >
> > That strikes me as a good change. I will add it to this series as
> > another patch.
> >
> > Yes, this code is called by the server's READDIR encoder with iov = NULL.
> > See nfsd3_init_dirlist_pages().
>
> This change breaks READDIR, looks like.
Thanks for testing...
I looked again, and I see that svcxdr_init_encode() initialises
->page_ptr to "buf->page - 1". So we need a +1 when moving from the
'head' to the pages.
Commit 05638dc73af2 ("nfsd4: simplify server xdr->next_page use")
explains why. I'm not sure I completely agree with the reasoning (head
really is a special case), but it probably isn't worth "fixing".
Thanks,
NeilBrown
>
> --
> Chuck Lever
>
>
>
>
