race in state manager leads to failed recovery

[Olga Kornievskaia <aglo@umich.edu>]

An application can fail with EIO when the server reboots and the
client while doing the recovery is getting into the following
situation where it incorrectly chooses "nograce" recovery instead of
"reboot" recovery. The necessary trigger is a pnfs mount and 2 DS
reads on the same file fail with BAD_STATEID each read uses its own
lock stateid (and MDS server reboots losing state). Since  server
rebooted it will fail recovery ops with BAD_SESSION and then also
STALE_ID triggering session and clientid recovery.

1. Thread1 gets BAD_STATEID initiates stateid recovery calls
nfs4_state_mark_reclaim_no_grace() which sets the cl_state->flags and
state->flags respective _NOGRACE flags.

2. State manager gets to run and it clears the _NOGRACE from the
cl_state. Calls nfs4_do_reclaim() which sends TEST_STATEID which gets
a BAD_SESSION.

3. Thread2 now gets control and it also processing its BAD_STATEID and
calls nfs4_state_mark_reclaim_no_grace() and it again sets the
state/cl_state->flags to have _NOGRACE.

4. State manager runs and continues with state recovery by sending
DESTROY_SESSION, and then CREATE_SESSION which fails with
STALE_CLIENTID. Now we are recovering the lease.

5. State manager in nfs4_recovery_handle_error for STALE_CLIENTID,
calls nfs4_state_start_reclaim_reboot() calls
nfs4_state_mark_reclaim_reboot() which has a check

/* Don't recover state that expired before the reboot */
if (test_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags)) {
  clear_bit(NFS_STATE_RECLAIM_REBOOT, &state->flags);
  return 0;
}

Because the _NOGRACE was set again in Step3, we end up clearing
_RECLAIM_REBOOT and not setting _RECLAIM_REBOOT in the cl_state->flags
and choose to do "nograce" operation instead of recovery.

Removing this check removes the problem.

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 06274e3..2c668a0 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1321,11 +1321,6 @@ static int
nfs4_state_mark_reclaim_reboot(struct nfs_client *clp, struct nfs4_st
  if (!nfs4_valid_open_stateid(state))
  return 0;
  set_bit(NFS_STATE_RECLAIM_REBOOT, &state->flags);
- /* Don't recover state that expired before the reboot */
- if (test_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags)) {
- clear_bit(NFS_STATE_RECLAIM_REBOOT, &state->flags);
- return 0;
- }
  set_bit(NFS_OWNER_RECLAIM_REBOOT, &state->owner->so_flags);
  set_bit(NFS4CLNT_RECLAIM_REBOOT, &clp->cl_state);
  return 1;

I'm confused by the comment why would we not what to recover state?

When both threads execute before the state manager starts this problem
doesn't exist.

If helpful here are two annotated (**) snippet from var log message
from the non-working and then one without the check:

Jun  6 12:06:42 ipa18 kernel: nfs4_state_mark_reclaim_nograce ffff880072f50=
c00
   ** first thread sets the nograce recovery
Jun  6 12:06:42 ipa18 kernel: AGLO: nfs4_state_manager reclaim_nograce
cleared NOGRACE from client state ffff88006fc23d28
  ** this is from the state manager=E2=80=99s loop from after
test_and_clear(NFS4CLNT_RECLAIM_NO_GRACE, cl_state)
Jun  6 12:06:42 ipa18 kernel: AGLO: nfs4_do_reclaim start
Jun  6 12:06:42 ipa18 kernel: NFS call  test_stateid ffff880077ac0af0
Jun  6 12:06:42 ipa18 kernel: nfs4_state_mark_reclaim_nograce ffff880072f50=
c00
   ** 2nd thread sets the nograce recovery
Jun  6 12:06:42 ipa18 kernel: AGLO: nfs4_do_reclaim status end=3D-11
Jun  6 12:06:42 ipa18 kernel: AGLO: nfs4_begin_drain_session
Jun  6 12:06:42 ipa18 kernel: AGLO: nfs4_state_mark_reclaim_reboot
clears RECLAIM_REBOOT from state=3Dffff880072f50c00
   ** this is from nfs4_state_mark_reclaim_reboot() and ends up not
setting the _RECLAIM_REBOOT flag in cl_state and clears it from the
state->flags
Jun  6 12:06:42 ipa18 kernel: AGLO: nfs4_begin_drain_session
Jun  6 12:06:43 ipa18 kernel: AGLO: nfs4_begin_drain_session
Jun  6 12:06:43 ipa18 kernel: AGLO: nfs4_reclaim_lease client
state=3Dffff88006fc23d28 flag has NOGRACE
Jun  6 12:06:43 ipa18 kernel: AGLO: nfs4_state_manager reclaim_nograce
cleared NOGRACE from client state ffff88006fc23d28
   ** this is in the state manager loop after the "reclaim_nograce" is
chosen and clearing the _NOGRACE flag
Jun  6 12:06:43 ipa18 kernel: AGLO: nfs4_do_reclaim start
Jun  6 12:06:43 ipa18 kernel: NFS call  test_stateid ffff880077ac0af0
Jun  6 12:06:43 ipa18 kernel: NFS call  test_stateid ffff880077ac02f0
Jun  6 12:06:43 ipa18 kernel: NFS call  test_stateid ffff880072f50c68
Jun  6 12:06:43 ipa18 kernel: NFS: nfs4_reclaim_open_state: Lock reclaim fa=
iled!
Jun  6 12:06:43 ipa18 kernel: NFS: nfs4_reclaim_open_state: Lock reclaim fa=
iled!
Jun  6 12:06:43 ipa18 kernel: AGLO: nfs4_reclaim_open_state clearing
NOGRACE from state=3Dffff880072f50c00
Jun  6 12:06:43 ipa18 kernel: AGLO: nfs4_do_reclaim end=3D0
Jun  6 12:06:43 ipa18 kernel: AGLO: nfs4_end_drain_session

This is a snippet from when we ignore that _NOGRACE is set and proceed
to set the _RECLAIM_REBOOT
Jun  6 12:21:58 ipa18 kernel: nfs4_state_mark_reclaim_nograce ffff88007363b=
bc0
Jun  6 12:21:58 ipa18 kernel: AGLO: nfs4_state_manager reclaim_nograce
cleared NOGRACE from client state ffff88002febe528
Jun  6 12:21:58 ipa18 kernel: AGLO: nfs4_do_reclaim start
Jun  6 12:21:58 ipa18 kernel: NFS call  test_stateid ffff880077ac4cf0
Jun  6 12:21:58 ipa18 kernel: nfs4_state_mark_reclaim_nograce ffff88007363b=
bc0
Jun  6 12:21:58 ipa18 kernel: AGLO: nfs4_do_reclaim status end=3D-11
Jun  6 12:21:58 ipa18 kernel: AGLO: nfs4_begin_drain_session
Jun  6 12:21:58 ipa18 kernel: AGLO: nfs4_state_mark_reclaim_reboot NOT
clearing _RECLAIM_REBOOT from state=3Dffff88007363bbc0
    *** Removing the check and instead proceeding to set the
_RECLAIM_REBOOT in the cl_state->flags so that
Jun  6 12:21:58 ipa18 kernel: AGLO: nfs4_begin_drain_session
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_begin_drain_session
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_reclaim_lease client
state=3Dffff88002febe528 flag has NOGRACE
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_state_manager reclaim reboot
op cl_state=3Dffff88002febe528
   *** Unlike the previous case the state manager now goes into the
reclaim reboot state.
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_do_reclaim start
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_reclaim_open_state clearing
NOGRACE from state=3Dffff88007363bbc0
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_do_reclaim end=3D0
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_state_manager reclaim_nograce
cleared NOGRACE from client state ffff88002febe528
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_do_reclaim start
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_do_reclaim end=3D0
Jun  6 12:21:59 ipa18 kernel: AGLO: nfs4_end_drain_session