2010-03-16 10:18:41

by Mi Jinlong

[permalink] [raw]
Subject: Re: [PATCH] Fix bug nfslock request sending fail will be process as blocked



Trond Myklebust =E5=86=99=E9=81=93:
> On Fri, 2010-03-12 at 18:17 +0800, Mi Jinlong wrote:=20
>> If local reason cause nfslock request send fail(means status < 0,
>> resp->status not be reset), the request will be process as blocked=20
>> at first now.
>>
>> This patch initialize resp->status to nlm_lck_denied_nolocks, it=20
>> can make the following process correctly.
>>
>> Signed-off-by: Mi Jinlong <[email protected]>
>> ---
>> fs/lockd/clntproc.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c
>> index c81249f..a631582 100644
>> --- a/fs/lockd/clntproc.c
>> +++ b/fs/lockd/clntproc.c
>> @@ -535,7 +535,7 @@ again:
>> * Initialise resp->status to a valid non-zero value,
>> * since 0 =3D=3D nlm_lck_granted
>> */
>> - resp->status =3D nlm_lck_blocked;
>> + resp->status =3D nlm_lck_denied_nolocks;
>> for(;;) {
>> /* Reboot protection */
>> fl->fl_u.nfs_fl.state =3D host->h_state;
>=20
> We _want_ to process it as being blocked if the RPC call was
> interrupted. The above patch will cause the client to just abandon th=
e
> interrupted lock request without sending the CANCEL request...

No, the above patch don't just abandon the interrupted lock request,
when the RPC call was interrupted, client will send an UNLOCK request=
for status < 0.
...
583 if (status < 0)
584 goto out_unlock;
...

And, I think an UNLOCK request is more advisable than a CANCEL reques=
t.
If a LOCK request was succeed when CANCEL request coming, it's useles=
s;
the lock should be unlocked.

That's only my opinion.

thanks,
Mi Jinlong



2010-03-16 13:12:54

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] Fix bug nfslock request sending fail will be process as blocked

On Tue, 2010-03-16 at 18:20 +0800, Mi Jinlong wrote:=20
>=20
> Trond Myklebust =E5=86=99=E9=81=93:
> > On Fri, 2010-03-12 at 18:17 +0800, Mi Jinlong wrote:=20
> >> If local reason cause nfslock request send fail(means status < 0,
> >> resp->status not be reset), the request will be process as blocked=
=20
> >> at first now.
> >>
> >> This patch initialize resp->status to nlm_lck_denied_nolocks, it=20
> >> can make the following process correctly.
> >>
> >> Signed-off-by: Mi Jinlong <[email protected]>
> >> ---
> >> fs/lockd/clntproc.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c
> >> index c81249f..a631582 100644
> >> --- a/fs/lockd/clntproc.c
> >> +++ b/fs/lockd/clntproc.c
> >> @@ -535,7 +535,7 @@ again:
> >> * Initialise resp->status to a valid non-zero value,
> >> * since 0 =3D=3D nlm_lck_granted
> >> */
> >> - resp->status =3D nlm_lck_blocked;
> >> + resp->status =3D nlm_lck_denied_nolocks;
> >> for(;;) {
> >> /* Reboot protection */
> >> fl->fl_u.nfs_fl.state =3D host->h_state;
> >=20
> > We _want_ to process it as being blocked if the RPC call was
> > interrupted. The above patch will cause the client to just abandon =
the
> > interrupted lock request without sending the CANCEL request...
>=20
> No, the above patch don't just abandon the interrupted lock request=
,
> when the RPC call was interrupted, client will send an UNLOCK reque=
st for status < 0.
> ...
> 583 if (status < 0)
> 584 goto out_unlock;
> ...
>=20
> And, I think an UNLOCK request is more advisable than a CANCEL requ=
est.
> If a LOCK request was succeed when CANCEL request coming, it's usel=
ess;
> the lock should be unlocked.

An unlock does not guarantee that the server will cancel the blocked
lock request.

Trond