Re: Is this nfsd kernel oops known?

[Al Viro <viro@zeniv.linux.org.uk>]

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@oracle.com>

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...