2002-11-14 19:50:07

by Juan Gomez

[permalink] [raw]
Subject: Non-blocking lock requests during the grace period






I found out that the current Linux client of lockd blocks non-blocking lock
requests while the server is in the grace period.
I think this is incorrect behavior and I am wondering if the will exists
out there to correct this and return "resource not available"
to the process when a request is for a *non-blocking* lock while the server
is in the grace period.

It is extremelly odd to issue a *non-blocking* call and be blocked in the
kernel for about a minute when the server happens
to be in the grace period.

This consists of a minor change to nlmclnt_call() and I would send the
patch if someone will review and include it.

Regards, Juan



2002-11-15 02:27:19

by Trond Myklebust

[permalink] [raw]
Subject: Re: Non-blocking lock requests during the grace period

>>>>> " " == Juan Gomez <[email protected]> writes:

> I found out that the current Linux client of lockd blocks
> non-blocking lock requests while the server is in the grace
> period. I think this is incorrect behavior and I am wondering
> if the will exists out there to correct this and return
> "resource not available" to the process when a request is for a
> *non-blocking* lock while the server is in the grace period.

Would the following fix it?

Cheers,
Trond

--- linux-2.5.47/fs/lockd/clntproc.c.orig 2002-09-29 10:15:13.000000000 -0400
+++ linux-2.5.47/fs/lockd/clntproc.c 2002-11-14 21:32:26.000000000 -0500
@@ -256,10 +256,8 @@
msg.rpc_cred = NULL;

do {
- if (host->h_reclaiming && !argp->reclaim) {
- interruptible_sleep_on(&host->h_gracewait);
- continue;
- }
+ if (host->h_reclaiming && !argp->reclaim)
+ goto wait_on_grace;

/* If we have no RPC client yet, create one. */
if ((clnt = nlm_bind_host(host)) == NULL)
@@ -296,6 +294,9 @@
dprintk("lockd: server returns status %d\n", resp->status);
return 0; /* Okay, call complete */
}
+wait_on_grace:
+ if (!argp->block)
+ return -EAGAIN;

/* Back off a little and try again */
interruptible_sleep_on_timeout(&host->h_gracewait, 15*HZ);

2002-11-15 16:26:18

by Juan Gomez

[permalink] [raw]
Subject: Re: Non-blocking lock requests during the grace period






Trond,

I think this would fix it but the patch I have tested locally is a bit
different.

1).-Instead of

+wait_on_grace:
+ if (!argp->block)
+ return -EAGAIN;

I have

+wait_on_grace:
+ if ((proc == NLMPROC_LOCK) && !argp->block)
+ return -EAGAIN;

2.-I also have this part enclosed in the if(resp->status ==
NLM_LCK_DENIED_GRACE_PERIOD) as follows:

if(resp->status == NLM_LCK_DENIED_GRACE_PERIOD) {

blah blah...

wait_on_grace:
if ((proc == NLMPROC_LOCK) && !argp->block)
return -EAGAIN
} else {

....
}

This with the intention to be very specific as to when we want the return
-EAGAIN to be called.

Juan



|---------+---------------------------->
| | Trond Myklebust |
| | <trond.myklebust@|
| | fys.uio.no> |
| | |
| | 11/14/02 06:33 PM|
| | |
|---------+---------------------------->
>-------------------------------------------------------------------------------------------------------------------------|
| |
| To: Juan Gomez/Almaden/IBM@IBMUS |
| cc: [email protected], [email protected] |
| Subject: Re: Non-blocking lock requests during the grace period |
| |
| |
>-------------------------------------------------------------------------------------------------------------------------|



>>>>> " " == Juan Gomez <[email protected]> writes:

> I found out that the current Linux client of lockd blocks
> non-blocking lock requests while the server is in the grace
> period. I think this is incorrect behavior and I am wondering
> if the will exists out there to correct this and return
> "resource not available" to the process when a request is for a
> *non-blocking* lock while the server is in the grace period.

Would the following fix it?

Cheers,
Trond

--- linux-2.5.47/fs/lockd/clntproc.c.orig 2002-09-29
10:15:13.000000000 -0400
+++ linux-2.5.47/fs/lockd/clntproc.c 2002-11-14
21:32:26.000000000 -0500
@@ -256,10 +256,8 @@
msg.rpc_cred = NULL;

do {
- if (host->h_reclaiming && !argp->reclaim) {
-
interruptible_sleep_on(&host->h_gracewait);
- continue;
- }
+ if (host->h_reclaiming && !argp->reclaim)
+ goto wait_on_grace;

/* If we have no RPC client yet, create one. */
if ((clnt = nlm_bind_host(host)) == NULL)
@@ -296,6 +294,9 @@
dprintk("lockd: server returns status
%d\n", resp->status);
return 0; /* Okay, call
complete */
}
+wait_on_grace:
+ if (!argp->block)
+ return -EAGAIN;

/* Back off a little and try again */
interruptible_sleep_on_timeout(&host->h_gracewait,
15*HZ);



2002-11-15 17:32:12

by Trond Myklebust

[permalink] [raw]
Subject: Re: Non-blocking lock requests during the grace period


> 2.-I also have this part enclosed in the if(resp->status ==
> NLM_LCK_DENIED_GRACE_PERIOD) as follows:

> if(resp->status == NLM_LCK_DENIED_GRACE_PERIOD) {

> blah blah...

> wait_on_grace:
> if ((proc == NLMPROC_LOCK) &&
> !argp->block)
> return -EAGAIN
> } else {

> ....
> }

> This with the intention to be very specific as to when we want
> the return -EAGAIN to be called.

The above means that you will still block on a F_GETLK query...

In any case, why would we want to return -EAGAIN in one case where
argp->block isn't set, and not in another? If there are cases where we
want to block and where we are not currently setting argp->block (the
only one I can think of might be NLMPROC_UNLOCK), then we should fix
the caller.

Cheers,
Trond

2002-11-15 18:41:54

by Juan Gomez

[permalink] [raw]
Subject: Re: Non-blocking lock requests during the grace period





Guess you are right. I was just worried that this code does not become
active for any other case than the one we were trying to fix.
We should also return when called from test as it should be non-blocking.
Then we do have the problem with UNLOCKs so we
have two choices: either fix the caller or just check for either lock or
test and block. I rather the last one.


Juan




|---------+---------------------------->
| | Trond Myklebust |
| | <trond.myklebust@|
| | fys.uio.no> |
| | |
| | 11/15/02 09:35 AM|
| | Please respond to|
| | trond.myklebust |
| | |
|---------+---------------------------->
>-------------------------------------------------------------------------------------------------------------------------|
| |
| To: Juan Gomez/Almaden/IBM@IBMUS |
| cc: [email protected], [email protected] |
| Subject: Re: Non-blocking lock requests during the grace period |
| |
| |
>-------------------------------------------------------------------------------------------------------------------------|




> 2.-I also have this part enclosed in the if(resp->status ==
> NLM_LCK_DENIED_GRACE_PERIOD) as follows:

> if(resp->status == NLM_LCK_DENIED_GRACE_PERIOD) {

> blah blah...

> wait_on_grace:
> if ((proc == NLMPROC_LOCK) &&
> !argp->block)
> return -EAGAIN
> } else {

> ....
> }

> This with the intention to be very specific as to when we want
> the return -EAGAIN to be called.

The above means that you will still block on a F_GETLK query...

In any case, why would we want to return -EAGAIN in one case where
argp->block isn't set, and not in another? If there are cases where we
want to block and where we are not currently setting argp->block (the
only one I can think of might be NLMPROC_UNLOCK), then we should fix
the caller.

Cheers,
Trond



2002-11-15 19:04:27

by Juan Gomez

[permalink] [raw]
Subject: Re: Non-blocking lock requests during the grace period





On second thoughts, I think I wonder if we want to fail F_GETLK while in
the grace period? seems like we won't there it makes sense to hold until
the grace period clears otherwise the client app may think there is
something wrong (note that F_GETLK man page does not provide EAGAIN as
a possible error code).
The case for F_SETLK is different as the result is expected when the lock
is not available due to a previous holder (or, as I want to do it, when the
lock is not available ether by previous holder or grace period).


Juan



|---------+---------------------------->
| | Trond Myklebust |
| | <trond.myklebust@|
| | fys.uio.no> |
| | |
| | 11/15/02 09:35 AM|
| | Please respond to|
| | trond.myklebust |
| | |
|---------+---------------------------->
>-------------------------------------------------------------------------------------------------------------------------|
| |
| To: Juan Gomez/Almaden/IBM@IBMUS |
| cc: [email protected], [email protected] |
| Subject: Re: Non-blocking lock requests during the grace period |
| |
| |
>-------------------------------------------------------------------------------------------------------------------------|




> 2.-I also have this part enclosed in the if(resp->status ==
> NLM_LCK_DENIED_GRACE_PERIOD) as follows:

> if(resp->status == NLM_LCK_DENIED_GRACE_PERIOD) {

> blah blah...

> wait_on_grace:
> if ((proc == NLMPROC_LOCK) &&
> !argp->block)
> return -EAGAIN
> } else {

> ....
> }

> This with the intention to be very specific as to when we want
> the return -EAGAIN to be called.

The above means that you will still block on a F_GETLK query...

In any case, why would we want to return -EAGAIN in one case where
argp->block isn't set, and not in another? If there are cases where we
want to block and where we are not currently setting argp->block (the
only one I can think of might be NLMPROC_UNLOCK), then we should fix
the caller.

Cheers,
Trond



2002-11-15 19:54:30

by Trond Myklebust

[permalink] [raw]
Subject: Re: Non-blocking lock requests during the grace period

>>>>> " " == Juan Gomez <[email protected]> writes:

> (note that F_GETLK man page does not provide EAGAIN as a
> possible error code).

Hmm....

In the Single UNIX spec, there appears to be a rather unexpected
difference between expected behaviour for lockf and fcntl here:


http://www.db.opengroup.org/cgi-bin/dbcgi?CALLER=sud2/indx.tpl&TOKEN=FIYR&TPL=sd_fileframe&dir=xsh&file=lockf

[EACCES] or [EAGAIN]
The function argument is F_TLOCK or F_TEST and the section is already
locked by another process.

OTOH

http://www.db.opengroup.org/cgi-bin/dbcgi?CALLER=sud2/indx.tpl&TOKEN=FIYR&TPL=sd_fileframe&dir=xsh&file=fcntl

[EACCES] or [EAGAIN]
The cmd argument is F_SETLK, and one of the following conditions
is true:

* The type of lock (l_type) is shared ( F_RDLCK) or exclusive
( F_WRLCK), and the segment of a file to be locked is
already exclusive locked by another process.
* The type of lock is exclusive, and some portion of the file
segment to be locked is already shared locked or exclusive
locked by another process.

Cheers,
Trond

2002-11-19 00:58:20

by mike.kupfer

[permalink] [raw]
Subject: Re: [NFS] Re: Non-blocking lock requests during the grace period

>>>>> "Trond" == Trond Myklebust <[email protected]> writes:

>>>>> " " == Juan Gomez <[email protected]> writes:

>> (note that F_GETLK man page does not provide EAGAIN as a
>> possible error code).

F_GETLK indicates a conflict by changing the arg struct to show the
conflicting lock.

As for the original topic, I would hesitate before changing the client
locking code to return EAGAIN just because the server is in its grace
period. The "blocking" or "non-blocking" behavior is tied to what
happens when there is already a lock that conflicts with the requested
one. When the server is in the grace period, it's unknown as to
whether there is already a lock that conflicts with the requested
one.

Mike Kupfer [email protected]
Solaris File Sharing Speaking for myself, not for Sun.