Hi Tigran, On Tue, Dec 8, 2020 at 7:39 AM Mkrtchyan, Tigran <tigran.mkrtchyan@xxxxxxx> wrote: > > > > Hi Anna et al., > > I run some more test and found two issues: > > 1. our server returns on getdeviceinfo notification bitmap of size 2. > Though, only the first element has non zero values, some how this > makes client unhappy, probably due to definition on decode_getdeviceinfo_maxsz > that expects bitmap length to be 1. > > 2. The client uses READ_PLUS to DS despite the fact that flex file > layout stated that DS supports nfs v4.1 > > Network File System, Ops(3): SEQUENCE, PUTFH, READ_PLUS > [Program Version: 4] > [V4 Procedure: COMPOUND (1)] > Tag: <EMPTY> > minorversion: 1 > Operations (count: 3): SEQUENCE, PUTFH, READ_PLUS > Opcode: SEQUENCE (53) > Opcode: PUTFH (22) > Opcode: READ_PLUS (68) > StateID > offset: 0 > count: 10016 > [Main Opcode: READ_PLUS (68)] Thanks for the update! You're right, READ_PLUS to the DS should not be happening. I'll look into why it is. Anna > > > I should re-run some tests with Trond's tree as I was checking the > DS errors in during the test I run last weeks. > > Regards, > Tigran. > > ----- Original Message ----- > > From: "Tigran Mkrtchyan" <tigran.mkrtchyan@xxxxxxx> > > To: "Anna Schumaker" <Anna.Schumaker@xxxxxxxxxx> > > Cc: "schumaker anna" <schumaker.anna@xxxxxxxxx>, "linux-nfs" <linux-nfs@xxxxxxxxxxxxxxx>, "Olga Kornievskaia" > > <aglo@xxxxxxxxx> > > Sent: Monday, 7 December, 2020 16:54:37 > > Subject: Re: [PATCH 0/3] NFS: Disable READ_PLUS by default > > > Sorry, completely confused. It the same commit as before > > c567552612ece787b178e3b147b5854ad422a836 > > > > Tigran. > > > > > > ----- Original Message ----- > >> From: "Tigran Mkrtchyan" <tigran.mkrtchyan@xxxxxxx> > >> To: "Anna Schumaker" <Anna.Schumaker@xxxxxxxxxx> > >> Cc: "schumaker anna" <schumaker.anna@xxxxxxxxx>, "linux-nfs" > >> <linux-nfs@xxxxxxxxxxxxxxx>, "Olga Kornievskaia" > >> <aglo@xxxxxxxxx> > >> Sent: Monday, 7 December, 2020 16:26:05 > >> Subject: Re: [PATCH 0/3] NFS: Disable READ_PLUS by default > > > >> Hi Anna > >> > >> I re-run bisect on your tree with tag nfs-for-5.10-1 as bad and 5.9.0 as good. > >> > >> The bisect point to commit a14a63594cc2e5bdcbb1543d29df945da71e380f, which makes > >> more sense. > >> > >> [centos@os-46-nfs-devel linux-nfs]$ git bisect good > >> c567552612ece787b178e3b147b5854ad422a836 is the first bad commit > >> commit c567552612ece787b178e3b147b5854ad422a836 > >> Author: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx> > >> Date: Wed May 28 13:41:22 2014 -0400 > >> > >> NFS: Add READ_PLUS data segment support > >> > >> This patch adds client support for decoding a single NFS4_CONTENT_DATA > >> segment returned by the server. This is the simplest implementation > >> possible, since it does not account for any hole segments in the reply. > >> > >> Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx> > >> > >> fs/nfs/nfs42xdr.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++ > >> fs/nfs/nfs4client.c | 2 + > >> fs/nfs/nfs4proc.c | 43 +++++++++++++- > >> fs/nfs/nfs4xdr.c | 1 + > >> include/linux/nfs4.h | 2 +- > >> include/linux/nfs_fs_sb.h | 1 + > >> include/linux/nfs_xdr.h | 2 +- > >> 7 files changed, 187 insertions(+), 5 deletions(-) > >> > >> > >> Best regards, > >> Tigran > >> > >> ----- Original Message ----- > >>> From: "Tigran Mkrtchyan" <tigran.mkrtchyan@xxxxxxx> > >>> To: "Olga Kornievskaia" <aglo@xxxxxxxxx> > >>> Cc: "schumaker anna" <schumaker.anna@xxxxxxxxx>, "linux-nfs" > >>> <linux-nfs@xxxxxxxxxxxxxxx>, "Anna Schumaker" > >>> <Anna.Schumaker@xxxxxxxxxx> > >>> Sent: Friday, 4 December, 2020 23:50:27 > >>> Subject: Re: [PATCH 0/3] NFS: Disable READ_PLUS by default > >> > >>> I agree with Olga, especially that disabling doesn't fixes my issue. > >>> Unfortunately I have no idea how kernel's vm works, but I am > >>> ready to test as many times as needed. > >>> > >>> Thanks, > >>> Tigran. > >>> > >>> ----- Original Message ----- > >>>> From: "Olga Kornievskaia" <aglo@xxxxxxxxx> > >>>> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@xxxxxxx> > >>>> Cc: "schumaker anna" <schumaker.anna@xxxxxxxxx>, "linux-nfs" > >>>> <linux-nfs@xxxxxxxxxxxxxxx>, "Anna Schumaker" > >>>> <Anna.Schumaker@xxxxxxxxxx> > >>>> Sent: Friday, 4 December, 2020 21:00:32 > >>>> Subject: Re: [PATCH 0/3] NFS: Disable READ_PLUS by default > >>> > >>>> 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@xxxxxxx> 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@xxxxxxxxx> > >>>>> > To: "linux-nfs" <linux-nfs@xxxxxxxxxxxxxxx> > >>>>> > Cc: "Anna Schumaker" <Anna.Schumaker@xxxxxxxxxx> > >>>>> > Sent: Thursday, 3 December, 2020 21:18:38 > >>>>> > Subject: [PATCH 0/3] NFS: Disable READ_PLUS by default > >>>>> > >>>>> > From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx> > >>>>> > > >>>>> > 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
