Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201203201841.103294-1-Anna.Schumaker@Netapp.com> <852166252.2305208.1607096860375.JavaMail.zimbra@desy.de>
In-Reply-To: <852166252.2305208.1607096860375.JavaMail.zimbra@desy.de>
From:   Olga Kornievskaia <aglo@umich.edu>
Date:   Fri, 4 Dec 2020 15:00:32 -0500
Message-ID: <CAN-5tyFJeLRyDUdtGkheGkmDE=i-FJprLFiav_mEPA35QeKLQA@mail.gmail.com>
Subject: Re: [PATCH 0/3] NFS: Disable READ_PLUS by default
To:     "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>
Cc:     schumaker anna <schumaker.anna@gmail.com>,
        linux-nfs <linux-nfs@vger.kernel.org>,
        Anna Schumaker <Anna.Schumaker@netapp.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

I object to putting the disable patch in, I think we need to fix the problem.

The current problem is generic/263 is failing because the end of the
buffer is corrupted since we lost the bytes when hole was expanded. I
don't know what the solution is: (1) hole expanding handling needs to
be fixed to not lose data or (2) we shouldnt be reporting that we read
the bytes we lost.

On Fri, Dec 4, 2020 at 10:49 AM Mkrtchyan, Tigran
<tigran.mkrtchyan@desy.de> wrote:
>
> Hi Anna,
>
> I see problems with gedeviceinfo and bisected it to c567552612ece787b178e3b147b5854ad422a836.
> The commit itself doesn't look that can break it, but might
> be can help you find the problem.
>
> What I see, that after xdr_read_pages call the xdr stream points
> to a some random point (or wrong page)
>
> Regards,
>    Tigran.
>
>
> ----- Original Message -----
> > From: "schumaker anna" <schumaker.anna@gmail.com>
> > To: "linux-nfs" <linux-nfs@vger.kernel.org>
> > Cc: "Anna Schumaker" <Anna.Schumaker@Netapp.com>
> > Sent: Thursday, 3 December, 2020 21:18:38
> > Subject: [PATCH 0/3] NFS: Disable READ_PLUS by default
>
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >
> > I've been scratching my head about what's going on with xfstests generic/091
> > and generic/263, but I'm not sure what else to look at at this point.
> > This patchset disables READ_PLUS by default by marking it as a
> > developer-only kconfig option.
> >
> > I also included a couple of patches fixing some other issues that were
> > noticed while inspecting the code. These patches don't help the tests
> > pass, but they do fail later on after applying so it does feel like
> > progress.
> >
> > I'm hopeful the remaning issues can be worked out in the future.
> >
> > Thanks,
> > Anna
> >
> >
> > Anna Schumaker (3):
> >  NFS: Disable READ_PLUS by default
> >  NFS: Allocate a scratch page for READ_PLUS
> >  SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes
> >
> > fs/nfs/Kconfig          |  9 +++++++++
> > fs/nfs/nfs42xdr.c       |  2 ++
> > fs/nfs/nfs4proc.c       |  2 +-
> > fs/nfs/read.c           | 13 +++++++++++--
> > include/linux/nfs_xdr.h |  1 +
> > net/sunrpc/xdr.c        |  3 ++-
> > 6 files changed, 26 insertions(+), 4 deletions(-)
> >
> > --
> > 2.29.2