Re: [PATCH 5/8] SUNRPC: Don't truncate tail in xdr_inline_pages()

From: Trond Myklebust <trondmy@hammerspace.com>
To: "chuck.lever@oracle.com" <chuck.lever@oracle.com>,
	"trondmy@kernel.org" <trondmy@kernel.org>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 5/8] SUNRPC: Don't truncate tail in xdr_inline_pages()
Date: Mon, 23 Nov 2020 04:29:34 +0000	[thread overview]
Message-ID: <614498c69c40421f8581fd8b25633e8668959581.camel@hammerspace.com> (raw)
In-Reply-To: <0CB9471F-ACC6-42A1-8DCD-8A9E74BAF8F1@oracle.com>

On Sun, 2020-11-22 at 20:24 -0500, Chuck Lever wrote:
> 
> 
> > On Nov 22, 2020, at 3:52 PM, trondmy@kernel.org wrote:
> > 
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > 
> > True that if the length of the pages[] array is not 4-byte aligned,
> > then
> > we will need to store the padding in the tail, but there is no need
> > to
> > truncate the total buffer length here.
> 
> This description confuses me. The existing code reduces the length of
> the tail, not the "total buffer length." And what the removed logic
> is
> doing is taking out the length of the XDR pad for the pages array
> when
> it is not expected to be used.

Why are we bothering to do that? There is nothing problematic with just
ignoring this test and leaving the tail length as it is, nor is there
anything to be gained by applying it.

> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > ---
> > net/sunrpc/xdr.c | 3 ---
> > 1 file changed, 3 deletions(-)
> > 
> > diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
> > index 3ce0a5daa9eb..5a450055469f 100644
> > --- a/net/sunrpc/xdr.c
> > +++ b/net/sunrpc/xdr.c
> > @@ -193,9 +193,6 @@ xdr_inline_pages(struct xdr_buf *xdr, unsigned
> > int offset,
> > 
> >         tail->iov_base = buf + offset;
> >         tail->iov_len = buflen - offset;
> > -       if ((xdr->page_len & 3) == 0)
> > -               tail->iov_len -= sizeof(__be32);
> > -
> >         xdr->buflen += len;
> > }
> > EXPORT_SYMBOL_GPL(xdr_inline_pages);
> > -- 
> > 2.28.0
> > 
> 
> --
> Chuck Lever
> 
> 
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com