Hi Ben,
On Mon, Sep 21, 2020 at 7:05 AM Benjamin Coddington <bcodding@xxxxxxxxxx> wrote:
>
> Since commit 0e0cb35b417f ("NFSv4: Handle NFS4ERR_OLD_STATEID in
> CLOSE/OPEN_DOWNGRADE") the following livelock may occur if a CLOSE races
> with the update of the nfs_state:
>
> Process 1 Process 2 Server
> ========= ========= ========
> OPEN file
> OPEN file
> Reply OPEN (1)
> Reply OPEN (2)
> Update state (1)
> CLOSE file (1)
> Reply OLD_STATEID (1)
> CLOSE file (2)
> Reply CLOSE (-1)
> Update state (2)
> wait for state change
> OPEN file
> wake
> CLOSE file
> OPEN file
> wake
> CLOSE file
> ...
> ...
>
> As long as the first process continues updating state, the second process
> will fail to exit the loop in nfs_set_open_stateid_locked(). This livelock
> has been observed in generic/168.
Once I apply this patch I have trouble with generic/478 doing lock reclaim:
[ 937.460505] run fstests generic/478 at 2020-09-22 09:59:14
[ 937.607990] NFS: __nfs4_reclaim_open_state: Lock reclaim failed!
And the test just hangs until I kill it.
Just thought you should know!
Anna
>
> Fix this by detecting the case in nfs_need_update_open_stateid() and
> then exit the loop if:
> - the state is NFS_OPEN_STATE, and
> - the stateid sequence is > 1, and
> - the stateid doesn't match the current open stateid
>
> Fixes: 0e0cb35b417f ("NFSv4: Handle NFS4ERR_OLD_STATEID in CLOSE/OPEN_DOWNGRADE")
> Cc: stable@xxxxxxxxxxxxxxx # v5.4+
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 45e0585e0667..9ced7a62c05e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -1570,10 +1570,14 @@ static bool nfs_need_update_open_stateid(struct nfs4_state *state,
> {
> if (test_bit(NFS_OPEN_STATE, &state->flags) == 0 ||
> !nfs4_stateid_match_other(stateid, &state->open_stateid)) {
> - if (stateid->seqid == cpu_to_be32(1))
> + if (stateid->seqid == cpu_to_be32(1)) {
> nfs_state_log_update_open_stateid(state);
> - else
> - set_bit(NFS_STATE_CHANGE_WAIT, &state->flags);
> + } else {
> + if (!nfs4_stateid_match_other(stateid, &state->open_stateid))
> + return false;
> + else
> + set_bit(NFS_STATE_CHANGE_WAIT, &state->flags);
> + }
> return true;
> }
>
> --
> 2.20.1
>
