Re: Question about open(CLAIM_FH)

[Trond Myklebust <trondmy@hammerspace.com>]

On Tue, 2019-04-30 at 14:44 -0400, Scott Mayhew wrote:
> On Thu, 18 Apr 2019, Trond Myklebust wrote:
> 
> > On Thu, 2019-04-18 at 16:43 -0400, Scott Mayhew wrote:
> > > On Thu, 18 Apr 2019, Trond Myklebust wrote:
> > > 
> > > > Hi Scott,
> > > > 
> > > > On Thu, 2019-04-18 at 09:37 -0400, Scott Mayhew wrote:
> > > > > When the client does an open(CLAIM_FH) and the server already
> > > > > has
> > > > > open
> > > > > state for that open owner and file, what's supposed to
> > > > > happen?
> > > > > Currently the server returns the existing stateid with the
> > > > > seqid
> > > > > bumped,
> > > > > but it looks like the client is expecting a new stateid (I'm
> > > > > seeing
> > > > > the
> > > > > state manager spending a lot of time waiting in
> > > > > nfs_set_open_stateid_locked() due to NFS_STATE_CHANGE_WAIT
> > > > > being
> > > > > set
> > > > > in
> > > > > the state flags by nfs_need_update_open_stateid()).
> > > > > 
> > > > > Looking at rfc5661 section 18.16.3, I see:
> > > > > 
> > > > >    | CLAIM_NULL, CLAIM_FH | For the client, this is a new
> > > > > OPEN
> > > > > request |
> > > > >    |                      | and there is no previous state
> > > > > associated  |
> > > > >    |                      | with the file for the
> > > > > client.  With        |
> > > > >    |                      | CLAIM_NULL, the file is
> > > > > identified by
> > > > > the  |
> > > > >    |                      | current filehandle and the
> > > > > specified       |
> > > > >    |                      | component name.  With CLAIM_FH
> > > > > (new
> > > > > to     |
> > > > >    |                      | NFSv4.1), the file is identified
> > > > > by
> > > > > just   |
> > > > >    |                      | the current filehandle.  
> > > > > 
> > > > > So it seems like maybe the server should be tossing the old
> > > > > state
> > > > > and
> > > > > returning a new stateid?
> > > > > 
> > > > 
> > > > No. As far as the protocol is concerned, the only difference
> > > > between
> > > > CLAIM_NULL and CLAIM_FH is through how the client identifies
> > > > the
> > > > file
> > > > (in the first case, through an implicit lookup, and in the
> > > > second
> > > > case
> > > > through a file handle). The client should be free to intermix
> > > > the
> > > > two
> > > > types of OPEN, and it should expect the resulting stateids to
> > > > depend
> > > > only on whether or not the open_owner matches. If the
> > > > open_owner
> > > > matches an existing stateid, then that stateid is bumped and
> > > > returned.
> > > > 
> > > > I'm not aware of any expectation in the client that this should
> > > > not
> > > > be
> > > > the case, so if you are seeing different behaviour, then
> > > > something
> > > > else
> > > > must be at work here. Is the client perhaps mounting the same
> > > > filesystem in two different places in such a way that the super
> > > > block
> > > > is not being shared?
> > > 
> > > No, it's just a single 4.1 mount w/ the default mount options.
> > > 
> > > For a bit of background, I've been trying to track down a problem
> > > in
> > > RHEL where the SEQ4_STATUS_RECALLABLE_STATE_REVOKED flags is
> > > getting
> > > permanently set because the nfs4_client->cl_revoked list on the
> > > server
> > > is non-empty... yet there's no longer open state on the client. 
> > > 
> > > I can reproduce it pretty easily in RHEL using 2 VMs, each with
> > > 2-4
> > > CPUs
> > > and 4-8G of memory.  The server has 64 nfsd threads and a 15
> > > second
> > > lease time.
> > > 
> > > On the client I'm running the following to add a 10ms delay to
> > > CB_RECALL
> > > replies:
> > > # stap -gve 'global count = 0; probe
> > > module("nfsv4").function("nfs4_callback_recall") { printf("%s:
> > > %d\n",
> > > ppfunc(), ++count); mdelay(10); }'
> > > 
> > > then in another window I open a bunch of files:
> > > # for i in `seq -w 1 5000`; do sleep 2m </mnt/t/dir1/file.$i &
> > > done
> > > 
> > > (Note: I already created the files ahead of time)
> > > 
> > > As soon as the bash prompt returns on the client, I run the
> > > following
> > > on
> > > the server:
> > > # for i in `seq -w 1 5000`; do date >/export/dir1/file.$i & done
> > > 
> > > At that point, any further SEQUENCE ops will have the recallable
> > > state
> > > revoked flag set on the client until the fs is unmounted.
> > > 
> > > If I run the same steps on Fedora clients with recent kernels, I
> > > don't
> > > have the problem with the recallable state revoked flag, but I'm
> > > getting
> > > some other strangeness.  Everything starts out fine with
> > > nfs_reap_expired_delegations() doing TEST_STATEID and
> > > FREE_STATEID,
> > > but
> > > once the state manager starts callings nfs41_open_expired(),
> > > things
> > > sort
> > > of grind to a halt and I see 1 OPEN and 1 or 2 TEST_STATEID ops
> > > every
> > > 5
> > > seconds in wireshark.  It stays that way until the files are
> > > closed
> > > on
> > > the client, when I see a slew of DELEGRETURNs and
> > > FREE_STATEIDs...
> > > but
> > > I'm only seeing 3 or 4 CLOSE ops.  If I poke around in crash on
> > > the
> > > server, I see a ton of open stateids:
> > > 
> > > crash> epython fs/nfsd/print-client-state-info.py
> > > nfsd_net = 0xffff93e473511000
> > >         nfs4_client = 0xffff93e3f7954980
> > >                 nfs4_stateowner = 0xffff93e4058cc360 num_stateids
> > > =
> > > 4997 <---- only 3 CLOSE ops were received
> > >                 num_openowners = 1
> > >                 num_layouts = 0
> > >                 num_delegations = 0
> > >                 num_sessions = 1
> > >                 num_copies = 0
> > >                 num_revoked = 0
> > >                 cl_cb_waitq_qlen = 0
> > > 
> > > Those stateids stick around until the fs is unmounted (and the
> > > DESTROY_STATEID ops return NFS4ERR_CLIENTID_BUSY while doing so).
> > > 
> > > Both VMs are running 5.0.6-200.fc29.x86_64, but the server also
> > > has
> > > the
> > > "nfsd: Don't release the callback slot unless it was actually
> > > held"
> > > patch you sent a few weeks ago as well as the "nfsd: CB_RECALL
> > > can
> > > race
> > > with FREE_STATEID" patch I sent today.
> > 
> > Are the calls to nfs41_open_expired() succeeding? It sounds like
> > they
> > might not be.
> 
> They're succeeding, they're just taking 5 seconds each. 
> 
> To make matters worse, due to the aggressive lease time on the
> server,
> the client was doing a SEQUENCE op periodically while the OPENs were
> occurring.  The SEQUENCE replies also had the
> RECALLABLE_STATE_REVOKED
> flag set, and since they weren't coming from the state manager the
> nfs4_slot->privileged field was unset, so we'd wind up calling
> nfs41_handle_recallable_state_revoked() again, undoing what little
> progress was made.  Bumping the lease time to 20 seconds made that go
> away, but the overall problem is still there.
> 
> Is it really necessary for nfs41_handle_recallable_state_revoked() to
> mark all open state as needing recovery?  After all, the only state
> that
> can be revoked are layouts and delegations.  If I change
> nfs41_handle_recallable_state_revoked(), so that instead of calling
> nfs41_handle_some_state_revoked() we instead call
> nfs_mark_test_expired_all_delegations() and
> nfs4_schedule_state_manager(), then the problem seems to go away.

That would be OK with me. That code probably pre-dates the existence of
nfs_mark_test_expired_all_delegations().

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