* [PATCH 1/1] NFSv4.1 fix infinite loop on IO BAD_STATEID error
@ 2017-03-30 14:00 Olga Kornievskaia
2017-03-30 17:26 ` Anna Schumaker
0 siblings, 1 reply; 5+ messages in thread
From: Olga Kornievskaia @ 2017-03-30 14:00 UTC (permalink / raw)
To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs
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.
Fixes: 02bfab0414d7 ("NFSv4.1: Don't recheck delegations that have already been checked")
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
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();
return;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] NFSv4.1 fix infinite loop on IO BAD_STATEID error
2017-03-30 14:00 [PATCH 1/1] NFSv4.1 fix infinite loop on IO BAD_STATEID error Olga Kornievskaia
@ 2017-03-30 17:26 ` Anna Schumaker
2017-03-30 17:37 ` Olga Kornievskaia
0 siblings, 1 reply; 5+ messages in thread
From: Anna Schumaker @ 2017-03-30 17:26 UTC (permalink / raw)
To: Olga Kornievskaia, Trond.Myklebust, anna.schumaker; +Cc: linux-nfs
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.
>
> Fixes: 02bfab0414d7 ("NFSv4.1: Don't recheck delegations that have already been checked")
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
> 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?
Thanks,
Anna
> return;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] NFSv4.1 fix infinite loop on IO BAD_STATEID error
2017-03-30 17:26 ` Anna Schumaker
@ 2017-03-30 17:37 ` Olga Kornievskaia
2017-03-30 17:41 ` Anna Schumaker
0 siblings, 1 reply; 5+ messages in thread
From: Olga Kornievskaia @ 2017-03-30 17:37 UTC (permalink / raw)
To: Anna Schumaker; +Cc: Olga Kornievskaia, Trond Myklebust, linux-nfs
On Thu, Mar 30, 2017 at 1:26 PM, Anna Schumaker
<Anna.Schumaker@netapp.com> 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@netapp.com>
>> ---
>> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] NFSv4.1 fix infinite loop on IO BAD_STATEID error
2017-03-30 17:37 ` Olga Kornievskaia
@ 2017-03-30 17:41 ` Anna Schumaker
2017-03-30 17:49 ` Olga Kornievskaia
0 siblings, 1 reply; 5+ messages in thread
From: Anna Schumaker @ 2017-03-30 17:41 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: Olga Kornievskaia, Trond Myklebust, linux-nfs
On 03/30/2017 01:37 PM, Olga Kornievskaia wrote:
> On Thu, Mar 30, 2017 at 1:26 PM, Anna Schumaker
> <Anna.Schumaker@netapp.com> 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@netapp.com>
>>> ---
>>> 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?
I like the sound of that even better, thanks! :)
>
>>
>> Thanks,
>> Anna
>>
>>> return;
>>> }
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/1] NFSv4.1 fix infinite loop on IO BAD_STATEID error
2017-03-30 17:41 ` Anna Schumaker
@ 2017-03-30 17:49 ` Olga Kornievskaia
0 siblings, 0 replies; 5+ messages in thread
From: Olga Kornievskaia @ 2017-03-30 17:49 UTC (permalink / raw)
To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs
Commit 63d63cbf5e03 "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.
Fixes: 63d63cbf5e03 ("NFSv4.1: Don't recheck delegations that have already been checked")
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
fs/nfs/nfs4proc.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index dfa46e4..bd1e4f6 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2453,17 +2453,14 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
}
nfs4_stateid_copy(&stateid, &delegation->stateid);
- if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
+ if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) ||
+ !test_and_clear_bit(NFS_DELEGATION_TEST_EXPIRED,
+ &delegation->flags)) {
rcu_read_unlock();
nfs_finish_clear_delegation_stateid(state, &stateid);
return;
}
- if (!test_and_clear_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags)) {
- rcu_read_unlock();
- return;
- }
-
cred = get_rpccred(delegation->cred);
rcu_read_unlock();
status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-30 17:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-30 14:00 [PATCH 1/1] NFSv4.1 fix infinite loop on IO BAD_STATEID error Olga Kornievskaia
2017-03-30 17:26 ` Anna Schumaker
2017-03-30 17:37 ` Olga Kornievskaia
2017-03-30 17:41 ` Anna Schumaker
2017-03-30 17:49 ` Olga Kornievskaia
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox