On Sun, Sep 11, 2022 at 06:36:22PM +0000, Chuck Lever III wrote:
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 9f486b788ed0..b16aed158ba6 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -846,10 +846,14 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
> > struct splice_desc *sd)
> > {
> > struct svc_rqst *rqstp = sd->u.data;
> > -
> > - svc_rqst_replace_page(rqstp, buf->page);
> > - if (rqstp->rq_res.page_len == 0)
> > - rqstp->rq_res.page_base = buf->offset;
> > + struct page *page = buf->page; // may be a compound one
> > + unsigned offset = buf->offset;
> > +
> > + page += offset / PAGE_SIZE;
>
> Nit: I see "offset / PAGE_SIZE" is used in the iter code base,
> but in the NFS stack, we prefer "offset >> PAGE_SIZE" and
> "offset & ~PAGE_MASK" (below).
*shrug*
If a C compiler is too dumb to recognize division by a power of two
constant...
Anyway, your codebase, your rules.
>
> > + for (int i = sd->len; i > 0; i -= PAGE_SIZE)
> > + svc_rqst_replace_page(rqstp, page++);
> > + if (rqstp->rq_res.page_len == 0) // first call
> > + rqstp->rq_res.page_base = offset % PAGE_SIZE;
> > rqstp->rq_res.page_len += sd->len;
> > return sd->len;
> > }
>
> I could take this through the nfsd for-rc tree, but that's based
> on 5.19-rc7 so it doesn't have f0f6b614f83d. I don't think will
> break functionality, but I'm wondering if it would be better for
> you to take this through your tree to improve bisect-ability.
>
> If you agree and Ben reports a Tested-by:, then here's my
>
> Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
OK, I'll wait for Tested-by and send it to Linus. Should be safe
for backports - with non-compound pages we are going to have
offset < PAGE_SIZE and sd->len <= PAGE_SIZE, so this is equivalent
to the mainline variant of the function for those...
