Hi Trond, Now that I recall the reason the Ontap server gives out the same delegation was to deal with the linux client behaviour who sends an open even though it holds a delegation (according to the server's view). So what server does is it gives out the same delegation. This patch series changes the semantics. Can you describe what you expect the server is supposed to do in this case? On Thu, Oct 31, 2019 at 11:49 AM Olga Kornievskaia <aglo@xxxxxxxxx> wrote: > > On Thu, Oct 31, 2019 at 11:27 AM Olga Kornievskaia <aglo@xxxxxxxxx> 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@xxxxxxxxx> 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 > > >
