On Thu, Mar 30, 2017 at 1:26 PM, Anna Schumaker <Anna.Schumaker@xxxxxxxxxx> wrote: > Hi Olga, > > On 03/30/2017 10:00 AM, Olga Kornievskaia wrote: >> Commit 02bfab0414d7 "NFSv4.1: Don't recheck delegations that >> have already been checked" introduced a regression where when a >> client received BAD_STATEID error it would not send any TEST_STATEID >> and instead go into an infinite loop of resending the IO that caused >> the BAD_STATEID. > > Can you double check the bad commit? I found it as 63d63cbf5e03 and not 02bfab0414d7. Oops. my bad. > >> >> Fixes: 02bfab0414d7 ("NFSv4.1: Don't recheck delegations that have already been checked") >> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx> >> --- >> fs/nfs/nfs4proc.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >> index dfa46e4..fb6d981 100644 >> --- a/fs/nfs/nfs4proc.c >> +++ b/fs/nfs/nfs4proc.c >> @@ -2460,6 +2460,7 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state) >> } >> >> if (!test_and_clear_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags)) { >> + nfs_finish_clear_delegation_stateid(state, &stateid); >> rcu_read_unlock(); > > The NFS_DELEGATION_REVOKED case does the rcu_read_unlock() before the call to nfs_finish_clear_delegation_stateid(). I'm not sure how much of a difference it makes, but can you swap the order of the calls here to match? Ok can do. While I'm at it, both if() do the same thing. Should they be squashed? > > Thanks, > Anna > >> return; >> } >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html