Re: [PATCH v2] NFSv4: fairly test all delegations on a SEQ4_ revocation

[Trond Myklebust <trondmy@hammerspace.com>]

On Mon, 2023-09-18 at 16:36 -0400, Benjamin Coddington wrote:
> On 14 Sep 2023, at 9:18, Benjamin Coddington wrote:
> 
> > On 24 Aug 2023, at 14:52, Benjamin Coddington wrote:
> > 
> > > When the client is required to use TEST_STATEID to discover which
> > > delegation(s) have been revoked, it may continually test
> > > delegations at the
> > > head of the list if the server continues to be unsatisfied and
> > > send
> > > SEQ4_STATUS_RECALLABLE_STATE_REVOKED.  For a large number of
> > > delegations
> > > this behavior is prone to live-lock because the client may never
> > > be able to
> > > test and free revoked state at the end of the list since the
> > > SEQ4_STATUS_RECALLABLE_STATE_REVOKED will cause us to flag
> > > delegations at
> > > the head of the list to be tested.  This problem is further
> > > exacerbated by
> > > the state manager's willingness to be scheduled out on a busy
> > > system while
> > > testing the list of delegations.
> > > 
> > > Keep a generation counter for each attempt to test all
> > > delegations, and
> > > skip delegations that have already been tested in the current
> > > pass.
> > > 
> > > Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> > 
> > This one went through the ringer in an environment that saw
> > multiple clients
> > live-locking, and resolves the problem for them.  They asked me to
> > add:
> > 
> > Tested-by: Torkil Svensgaard <torkil@drcmr.dk>
> > Tested-by: Ruben Vestergaard <rubenv@drcmr.dk>
> > 
> > Ben
> 
> Did this one get rejected with a reason?  This fix could also be
> implemented
> with flag (as I mentioned in a reply on v2).
> 
> Ben
> 

Speaking only for myself, I haven't yet had time to properly review it.
So not yet rejected by me.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com