Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191023235600.10880-1-trond.myklebust@hammerspace.com> <CAN-5tyF3hryyjdHjcoNHHPJUDZmgtgxQDureZ+3QQmiwh2CMsA@mail.gmail.com>
In-Reply-To: <CAN-5tyF3hryyjdHjcoNHHPJUDZmgtgxQDureZ+3QQmiwh2CMsA@mail.gmail.com>
From:   Olga Kornievskaia <aglo@umich.edu>
Date:   Thu, 31 Oct 2019 11:49:34 -0400
Message-ID: <CAN-5tyHLGVt4jHZNFSW_5tXFM74KGbaN=PLMb-sjq2+hbcH7YA@mail.gmail.com>
Subject: Re: [PATCH 00/14] Delegation bugfixes
To:     Trond Myklebust <trondmy@gmail.com>
Cc:     linux-nfs <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Thu, Oct 31, 2019 at 11:27 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> Hi Trond,
>
> This patch set produces the following in my testing. Basically what I
> see the client is prevented from using a delegation at all.
>
> After I induce a race of DELEGRETURN/OPEN
> --- the racing OPEN gets a delegation (it returns the same seqid and
> other as the delegation being returned) but the client doesn't use it.
> --- the following (next) OPEN that also gets a delegation immediately
> has the client returning the given delegation.
>
> Disclaimer: in my testing the racing DELEGRETURN doesn't fail with
> OLD_STATEID, NetApp returns OK.

Testing the same against Linux. It prevents the client from using
future delegation stateid. On the induced DELEGRETURN/OPEN race, the
linux server doesn't give a new read delegation. The following open
gets a read delegation and returns it right away.


> On Thu, Oct 24, 2019 at 6:56 AM Trond Myklebust <trondmy@gmail.com> wrote:
> >
> > The following patchset fixes up a number of issues with delegations,
> > but mainly attempts to fix a race condition between open and
> > delegreturn, where the open and the delegreturn get re-ordered so
> > that the delegreturn ends up revoking the delegation that was returned
> > by the open.
> > The root cause is that in certain circumstances, we may currently end
> > up freeing the delegation from delegreturn, so when we later receive
> > the reply to the open, we've lost track of the fact that the seqid
> > predates the one that was returned.
> >
> > This patchset fixes that case by ensuring that we always keep track
> > of the last delegation stateid that was returned for any given inode.
> > That way, if we later see a delegation stateid with the same opaque
> > field, but an older seqid, we know we cannot trust it, and so we
> > ask to replay the OPEN compound.
> >
> > Trond Myklebust (14):
> >   NFSv4: Don't allow a cached open with a revoked delegation
> >   NFSv4: Fix delegation handling in update_open_stateid()
> >   NFSv4: nfs4_callback_getattr() should ignore revoked delegations
> >   NFSv4: Delegation recalls should not find revoked delegations
> >   NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was
> >     revoked
> >   NFS: Rename nfs_inode_return_delegation_noreclaim()
> >   NFSv4: Don't remove the delegation from the super_list more than once
> >   NFSv4: Hold the delegation spinlock when updating the seqid
> >   NFSv4: Clear the NFS_DELEGATION_REVOKED flag in
> >     nfs_update_inplace_delegation()
> >   NFSv4: Update the stateid seqid in nfs_revoke_delegation()
> >   NFSv4: Revoke the delegation on success in nfs4_delegreturn_done()
> >   NFSv4: Ignore requests to return the delegation if it was revoked
> >   NFSv4: Don't reclaim delegations that have been returned or revoked
> >   NFSv4: Fix races between open and delegreturn
> >
> >  fs/nfs/callback_proc.c |   2 +-
> >  fs/nfs/delegation.c    | 109 +++++++++++++++++++++++++++++------------
> >  fs/nfs/delegation.h    |   4 +-
> >  fs/nfs/nfs4proc.c      |  13 ++---
> >  fs/nfs/nfs4super.c     |   4 +-
> >  5 files changed, 88 insertions(+), 44 deletions(-)
> >
> > --
> > 2.21.0
> >