I ran into a curious issue when a lock is being canceled. The
cancellation results in a lock request to the vfs layer instead of an
unlock request. This is particularly insidious when the process that
owns the lock is exiting. In that case, sometimes the erroneous lock is
applied AFTER the process has entered zombie state, preventing the lock
from ever being released. Eventually other processes block on the lock
causing a slow degredation of the system. In the 2.6.16 kernel this was
investigated on, the problem is compounded by the fact that the cl_sem
is held while blocking on the vfs lock, which results in most processes
accessing the nfs file system in question hanging.
It seems like the simplest solution is to force all situations where
nfs4_do_unlck() is being used to result in an unlock, so with that in
mind, I made the following change:
Signed-off-by: Frank Filz <[email protected]>
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 648e0ac..6751a8c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3152,6 +3152,11 @@ static struct rpc_task *nfs4_do_unlck(struct file_lock *fl,
{
struct nfs4_unlockdata *data;
+ /* Ensure this is an unlock - when canceling a lock, the
+ * canceled lock is passed in, and it won't be an unlock.
+ */
+ fl->fl_type = F_UNLCK;
+
data = nfs4_alloc_unlockdata(fl, ctx, lsp, seqid);
if (data == NULL) {
nfs_free_seqid(seqid);
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Mon, 2007-06-11 at 10:01 -0700, Frank Filz wrote:
> I ran into a curious issue when a lock is being canceled. The
> cancellation results in a lock request to the vfs layer instead of an
> unlock request. This is particularly insidious when the process that
Any idea why the cancellation turns into a lock request? I really don't
want to apply a patch that just appears to paper over the problem.
Cheers
Trond
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Wed, 2007-06-13 at 12:35 -0400, Trond Myklebust wrote:
> On Mon, 2007-06-11 at 10:01 -0700, Frank Filz wrote:
> > I ran into a curious issue when a lock is being canceled. The
> > cancellation results in a lock request to the vfs layer instead of an
> > unlock request. This is particularly insidious when the process that
>
> Any idea why the cancellation turns into a lock request? I really don't
> want to apply a patch that just appears to paper over the problem.
Ok, the path is as follows:
first _nfs4_do_setlk():
static int _nfs4_do_setlk(struct nfs4_state *state, int cmd, struct file_lock *fl, int reclaim)
...
ret = nfs4_wait_for_completion_rpc_task(task);
if (ret == 0) {
...
} else
data->cancelled = 1;
then nfs4_lock_release():
static void nfs4_lock_release(void *calldata)
...
if (data->cancelled != 0) {
struct rpc_task *task;
task = nfs4_do_unlck(&data->fl, data->ctx, data->lsp,
data->arg.lock_seqid);
The problem is the same file_lock that was passed in to _nfs4_do_setlk()
gets passed to nfs4_do_unlck() from nfs4_lock_release(). So the type is
still F_RDLCK or FWRLCK, not F_UNLCK. At some point, when cancelling the
lock, the type needs to be changed to F_UNLCK. It seemed easiest to do
that in nfs4_do_unlck(), but it could be done in nfs4_lock_release().
The concern I had with doing it there was if something still needed the
original file_lock, though it turns out the original file_lock still
needs to be modified by nfs4_do_unlck() because nfs4_do_unlck() uses the
original file_lock to pass to the vfs layer, and a copy of the original
file_lock for the RPC request.
Frank
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Wed, 2007-06-13 at 13:17 -0700, Frank Filz wrote:
> On Wed, 2007-06-13 at 12:35 -0400, Trond Myklebust wrote:
> > On Mon, 2007-06-11 at 10:01 -0700, Frank Filz wrote:
> > > I ran into a curious issue when a lock is being canceled. The
> > > cancellation results in a lock request to the vfs layer instead of an
> > > unlock request. This is particularly insidious when the process that
> >
> > Any idea why the cancellation turns into a lock request? I really don't
> > want to apply a patch that just appears to paper over the problem.
>
> Ok, the path is as follows:
>
> first _nfs4_do_setlk():
>
> static int _nfs4_do_setlk(struct nfs4_state *state, int cmd, struct file_lock *fl, int reclaim)
> ...
> ret = nfs4_wait_for_completion_rpc_task(task);
> if (ret == 0) {
> ...
> } else
> data->cancelled = 1;
>
> then nfs4_lock_release():
>
> static void nfs4_lock_release(void *calldata)
> ...
> if (data->cancelled != 0) {
> struct rpc_task *task;
> task = nfs4_do_unlck(&data->fl, data->ctx, data->lsp,
> data->arg.lock_seqid);
>
> The problem is the same file_lock that was passed in to _nfs4_do_setlk()
> gets passed to nfs4_do_unlck() from nfs4_lock_release(). So the type is
> still F_RDLCK or FWRLCK, not F_UNLCK. At some point, when cancelling the
> lock, the type needs to be changed to F_UNLCK. It seemed easiest to do
> that in nfs4_do_unlck(), but it could be done in nfs4_lock_release().
> The concern I had with doing it there was if something still needed the
> original file_lock, though it turns out the original file_lock still
> needs to be modified by nfs4_do_unlck() because nfs4_do_unlck() uses the
> original file_lock to pass to the vfs layer, and a copy of the original
> file_lock for the RPC request.
>
> Frank
I see. It would be great if you could summarise the above (or just
including the above text in full would be fine too) in the changelog
entry for the patch. (Sorry sometimes the one-liner patches are the
hardest to justify :-))
Cheers
Trond
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Wed, 2007-06-13 at 16:37 -0400, Trond Myklebust wrote:
> On Wed, 2007-06-13 at 13:17 -0700, Frank Filz wrote:
> > On Wed, 2007-06-13 at 12:35 -0400, Trond Myklebust wrote:
> > > On Mon, 2007-06-11 at 10:01 -0700, Frank Filz wrote:
> > > > I ran into a curious issue when a lock is being canceled. The
> > > > cancellation results in a lock request to the vfs layer instead of an
> > > > unlock request. This is particularly insidious when the process that
> > >
> > > Any idea why the cancellation turns into a lock request? I really don't
> > > want to apply a patch that just appears to paper over the problem.
> >
> > Ok, the path is as follows:
> >
> > first _nfs4_do_setlk():
> >
> > static int _nfs4_do_setlk(struct nfs4_state *state, int cmd, struct file_lock *fl, int reclaim)
> > ...
> > ret = nfs4_wait_for_completion_rpc_task(task);
> > if (ret == 0) {
> > ...
> > } else
> > data->cancelled = 1;
> >
> > then nfs4_lock_release():
> >
> > static void nfs4_lock_release(void *calldata)
> > ...
> > if (data->cancelled != 0) {
> > struct rpc_task *task;
> > task = nfs4_do_unlck(&data->fl, data->ctx, data->lsp,
> > data->arg.lock_seqid);
> >
> > The problem is the same file_lock that was passed in to _nfs4_do_setlk()
> > gets passed to nfs4_do_unlck() from nfs4_lock_release(). So the type is
> > still F_RDLCK or FWRLCK, not F_UNLCK. At some point, when cancelling the
> > lock, the type needs to be changed to F_UNLCK. It seemed easiest to do
> > that in nfs4_do_unlck(), but it could be done in nfs4_lock_release().
> > The concern I had with doing it there was if something still needed the
> > original file_lock, though it turns out the original file_lock still
> > needs to be modified by nfs4_do_unlck() because nfs4_do_unlck() uses the
> > original file_lock to pass to the vfs layer, and a copy of the original
> > file_lock for the RPC request.
> >
> > Frank
>
> I see. It would be great if you could summarise the above (or just
> including the above text in full would be fine too) in the changelog
> entry for the patch. (Sorry sometimes the one-liner patches are the
> hardest to justify :-))
Sure thing.
Frank
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs