2002-04-04 17:45:56

by Dumas Patrice

[permalink] [raw]
Subject: 3 strange things in the lockd code ?

Hi,
I have read some lockd code, especially what is related with blocked locks, and
I have found 2 things I can't explain and a third which isn't really strange.

1) nlmsvc_grant_reply (in svclock.c) seems to be used nowhere, and there is
that in svcproc.c:
#define nlmsvc_proc_granted_res nlmsvc_proc_null

My understanding is that in nlmsvc_grant_blocked there is a callback to the client
with NLMPROC_GRANTED_MSG, the client get the callback using nlmsvc_proc_granted_msg
which in turn sends NLMPROC_GRANTED_RES (and also which triggers
nlmsvc_grant_callback which only rescuedule block). The server receive and responds
to the callback, but it is nlmsvc_proc_null.

I may be totally wrong, but it seems to me that nlmsvc_proc_granted_res should
instead call nlmsvc_grant_reply to delete the block. Did I missed something ?

2) If there are 2 clients on different hosts with the same cookie, the
nlmsvc_grant_callback may find an invalid block.

3) This one may be a design choice. In nlmclnt_lock, nlmclnt_block is called if
the server responds NLM_LCK_BLOCKED to NLMPROC_LOCK even though the client
doesn't want a blocking lock. If the server is broken and responds
NLM_LCK_BLOCKED even if it should have responded something else, the client
will call nlmclnt_block. To avoid that, maybe it could be possible to exchange
if ((status = nlmclnt_call(req, NLMPROC_LOCK)) >= 0)
with
if (((status = nlmclnt_call(req, NLMPROC_LOCK)) >= 0) && req->a_args.block)
But it may also be a design choice, not to try to turn around broken servers.


I would like to see whether what I think happens (in 1)) really happens, but
I don't know how to trigger lockd to print debug information (nor where to read
it, but I think syslog or dmesg will be enough). How could I do that ?

Pat



_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2002-04-08 12:07:32

by Trond Myklebust

[permalink] [raw]
Subject: Re: 3 strange things in the lockd code ?

>>>>> " " == Dumas Patrice <[email protected]> writes:

> My understanding is that in nlmsvc_grant_blocked there is a
> callback to the client with NLMPROC_GRANTED_MSG, the client get
> the callback using nlmsvc_proc_granted_msg which in turn sends
> NLMPROC_GRANTED_RES (and also which triggers
> nlmsvc_grant_callback which only rescuedule block). The server
> receive and responds to the callback, but it is
> nlmsvc_proc_null.

> I may be totally wrong, but it seems to me that
> nlmsvc_proc_granted_res should instead call nlmsvc_grant_reply
> to delete the block. Did I missed something ?

NLM_GRANTED_RES is part of the 'asynchronous RPC' NLM model which was
designed to support locking on systems without monitoring
(i.e. without rpc.statd').
Linux does not currently support such a model, nor do we really plan
to do so. Non-monitored locks are pretty hairy things to deal with
(see http://www.opengroup.org/onlinepubs/9629799/chap9.htm#tagcjh_10_01_01_02).

> 2) If there are 2 clients on different hosts with the same
> cookie, the
> nlmsvc_grant_callback may find an invalid block.

I agree. We should be checking the host address as well.

> 3) This one may be a design choice. In nlmclnt_lock,
> nlmclnt_block is called if
> the server responds NLM_LCK_BLOCKED to NLMPROC_LOCK even though
> the client doesn't want a blocking lock. If the server is
> broken and responds NLM_LCK_BLOCKED even if it should have
> responded something else, the client will call
> nlmclnt_block. To avoid that, maybe it could be possible to
> exchange if ((status = nlmclnt_call(req, NLMPROC_LOCK)) >= 0)
> with if (((status = nlmclnt_call(req, NLMPROC_LOCK)) >= 0) &&
> req->a_args.block) But it may also be a design choice, not to
> try to turn around broken servers.

Right.

Cheers,
Trond

_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2002-04-08 12:58:35

by Dumas Patrice

[permalink] [raw]
Subject: Re: 3 strange things in the lockd code ?

Hi,

> NLM_GRANTED_RES is part of the 'asynchronous RPC' NLM model which was
> designed to support locking on systems without monitoring
> (i.e. without rpc.statd').
> Linux does not currently support such a model, nor do we really plan
> to do so. Non-monitored locks are pretty hairy things to deal with
> (see http://www.opengroup.org/onlinepubs/9629799/chap9.htm#tagcjh_10_01_01_02).

Are you sure about that ?
After reading the code and the page you send me, I tend to disagree. My
undersstanding is that non-monitored locks are only concerned with the
NLM_NM_LOCK - Non-monitored Lock callback.

I didn't saw something really saying non-monitored clients aren't allowed to do
NLM_LOCK or NLM_LOCK_MSG, but I assume they aren't. However I am pretty sure
that monitored clients can do NLM_LOCK_MSG and thus they can receive in turn
NLM_GRANTED_MSG, which should trigger them to send a NLM_GRANTED_RES. And
indeed the dmesg output I send in a mail with subject "lockd bug (?)" shows
linux nfs clients do send NLM_GRANTED_MSG.

> I agree. We should be checking the host address as well.

I will try to make a patch.

Cheers

Pat

_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2002-04-08 13:12:37

by Trond Myklebust

[permalink] [raw]
Subject: Re: 3 strange things in the lockd code ?

>>>>> " " == Dumas Patrice <[email protected]> writes:

> I didn't saw something really saying non-monitored clients
> aren't allowed to do NLM_LOCK or NLM_LOCK_MSG, but I assume
> they aren't. However I am pretty sure that monitored clients
> can do NLM_LOCK_MSG and thus they can receive in turn
> NLM_GRANTED_MSG, which should trigger them to send a
> NLM_GRANTED_RES. And indeed the dmesg output I send in a mail
> with subject "lockd bug (?)" shows linux nfs clients do send
> NLM_GRANTED_MSG.

I would certainly accept patches that fix the support for asynchronous
calls on the server side. I'd be more sceptical about actually using
them for the client though.

>> I agree. We should be checking the host address as well.

> I will try to make a patch.

Thanks...

Cheers,
Trond

_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs