On Wed, Dec 9, 2020 at 12:22 PM Olga Kornievskaia <aglo@xxxxxxxxx> wrote:
>
> On Wed, Dec 9, 2020 at 12:12 PM Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote:
> >
> > On Wed, 2020-12-09 at 12:07 -0500, Olga Kornievskaia wrote:
> > > On Wed, Dec 9, 2020 at 11:59 AM Trond Myklebust
> > > <trondmy@xxxxxxxxxxxxxxx> wrote:
> > > >
> > > > On Fri, 2020-12-04 at 15:00 -0500, Olga Kornievskaia wrote:
> > > > > I object to putting the disable patch in, I think we need to fix
> > > > > the
> > > > > problem.
> > > >
> > > > I can't see the problem is fixable in 5.10. There are way too many
> > > > changes required, and we're in the middle of the week of the last -
> > > > rc
> > > > for 5.10. Furthermore, there are no regressions introduced by just
> > > > disabling the functionality, because READ_PLUS has only just been
> > > > merged in this release cycle.
> > > >
> > > > I therefore strongly suggest we just send [PATCH 1/3] NFS: Disable
> > > > READ_PLUS by default and then fix the rest in 5.11.
> > >
> > > Sure, but shouldn't there be more ifdefs inside of the xdr code to
> > > turn it off completely?
> >
> > AFAICT, those functions are not called by anything else, so as long as
> > the READ_PLUS client functionality is disabled, they should be
> > harmless.
>
> Is it benign that in the normal read path sunrpc will be calling a new
> function of xdr_realign_pages()? Non readplus code didn't have it.
It should be. All I did was pull out some code from xdr_align_pages()
and put it into a new function. `git show --diff-algorithm=histogram`
says this is what I did:
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index 909920fab93b..d93bcad5ba9f 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -997,10 +997,25 @@ __be32 * xdr_inline_decode(struct xdr_stream
*xdr, size_t nbytes)
}
EXPORT_SYMBOL_GPL(xdr_inline_decode);
+static void xdr_realign_pages(struct xdr_stream *xdr)
+{
+ struct xdr_buf *buf = xdr->buf;
+ struct kvec *iov = buf->head;
+ unsigned int cur = xdr_stream_pos(xdr);
+ unsigned int copied, offset;
+
+ /* Realign pages to current pointer position */
+ if (iov->iov_len > cur) {
+ offset = iov->iov_len - cur;
+ copied = xdr_shrink_bufhead(buf, offset);
+ trace_rpc_xdr_alignment(xdr, offset, copied);
+ xdr->nwords = XDR_QUADLEN(buf->len - cur);
+ }
+}
+
static unsigned int xdr_align_pages(struct xdr_stream *xdr, unsigned int len)
{
struct xdr_buf *buf = xdr->buf;
- struct kvec *iov;
unsigned int nwords = XDR_QUADLEN(len);
unsigned int cur = xdr_stream_pos(xdr);
unsigned int copied, offset;
@@ -1008,15 +1023,7 @@ static unsigned int xdr_align_pages(struct
xdr_stream *xdr, unsigned int len)
if (xdr->nwords == 0)
return 0;
- /* Realign pages to current pointer position */
- iov = buf->head;
- if (iov->iov_len > cur) {
- offset = iov->iov_len - cur;
- copied = xdr_shrink_bufhead(buf, offset);
- trace_rpc_xdr_alignment(xdr, offset, copied);
- xdr->nwords = XDR_QUADLEN(buf->len - cur);
- }
-
+ xdr_realign_pages(xdr);
if (nwords > xdr->nwords) {
nwords = xdr->nwords;
len = nwords << 2;
>
> >
> > --
> > Trond Myklebust
> > Linux NFS client maintainer, Hammerspace
> > trond.myklebust@xxxxxxxxxxxxxxx
> >
> >
