Commit 27bf6305 authored by Tariq Saeed's avatar Tariq Saeed Committed by Linus Torvalds

ocfs2: fix deadlock when two nodes are converting same lock from PR to EX and...

ocfs2: fix deadlock when two nodes are converting same lock from PR to EX and idletimeout closes conn

Orabug: 18639535

Two node cluster and both nodes hold a lock at PR level and both want to
convert to EX at the same time.  Master node 1 has sent BAST and then
closes the connection due to idletime out.  Node 0 receives BAST, sends
unlock req with cancel flag but gets error -ENOTCONN.  The problem is
this error is ignored in dlm_send_remote_unlock_request() on the
**incorrect** assumption that the master is dead.  See NOTE in comment
why it returns DLM_NORMAL.  Upon getting DLM_NORMAL, node 0 proceeds to
sends convert (without cancel flg) which fails with -ENOTCONN.  waits 5
sec and resends.

This time gets DLM_IVLOCKID from the master since lock not found in
grant, it had been moved to converting queue in response to conv PR->EX
req.  No way out.

Node 1 (master)				Node 0
==============				======

  lock mode PR				PR

  convert PR -> EX
  mv grant -> convert and que BAST
  ...
                     <-------- convert PR -> EX
  convert que looks like this: ((node 1, PR -> EX) (node 0, PR -> EX))
  ...
                        BAST (want PR -> NL)
                     ------------------>
  ...
  idle timout, conn closed
                                ...
                                In response to BAST,
                                sends unlock with cancel convert flag
                                gets -ENOTCONN. Ignores and
                                sends remote convert request
                                gets -ENOTCONN, waits 5 Sec, retries
  ...
  reconnects
                   <----------------- convert req goes through on next try
  does not find lock on grant que
                   status DLM_IVLOCKID
                   ------------------>
  ...

No way out.  Fix is to keep retrying unlock with cancel flag until it
succeeds or the master dies.
Signed-off-by: default avatarTariq Saeed <tariq.x.saeed@oracle.com>
Reviewed-by: default avatarMark Fasheh <mfasheh@suse.de>
Cc: Joel Becker <jlbec@evilplan.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 5fb1beb0
...@@ -191,7 +191,9 @@ static enum dlm_status dlmunlock_common(struct dlm_ctxt *dlm, ...@@ -191,7 +191,9 @@ static enum dlm_status dlmunlock_common(struct dlm_ctxt *dlm,
DLM_UNLOCK_CLEAR_CONVERT_TYPE); DLM_UNLOCK_CLEAR_CONVERT_TYPE);
} else if (status == DLM_RECOVERING || } else if (status == DLM_RECOVERING ||
status == DLM_MIGRATING || status == DLM_MIGRATING ||
status == DLM_FORWARD) { status == DLM_FORWARD ||
status == DLM_NOLOCKMGR
) {
/* must clear the actions because this unlock /* must clear the actions because this unlock
* is about to be retried. cannot free or do * is about to be retried. cannot free or do
* any list manipulation. */ * any list manipulation. */
...@@ -200,7 +202,8 @@ static enum dlm_status dlmunlock_common(struct dlm_ctxt *dlm, ...@@ -200,7 +202,8 @@ static enum dlm_status dlmunlock_common(struct dlm_ctxt *dlm,
res->lockname.name, res->lockname.name,
status==DLM_RECOVERING?"recovering": status==DLM_RECOVERING?"recovering":
(status==DLM_MIGRATING?"migrating": (status==DLM_MIGRATING?"migrating":
"forward")); (status == DLM_FORWARD ? "forward" :
"nolockmanager")));
actions = 0; actions = 0;
} }
if (flags & LKM_CANCEL) if (flags & LKM_CANCEL)
...@@ -364,7 +367,10 @@ static enum dlm_status dlm_send_remote_unlock_request(struct dlm_ctxt *dlm, ...@@ -364,7 +367,10 @@ static enum dlm_status dlm_send_remote_unlock_request(struct dlm_ctxt *dlm,
* updated state to the recovery master. this thread * updated state to the recovery master. this thread
* just needs to finish out the operation and call * just needs to finish out the operation and call
* the unlockast. */ * the unlockast. */
ret = DLM_NORMAL; if (dlm_is_node_dead(dlm, owner))
ret = DLM_NORMAL;
else
ret = DLM_NOLOCKMGR;
} else { } else {
/* something bad. this will BUG in ocfs2 */ /* something bad. this will BUG in ocfs2 */
ret = dlm_err_to_dlm_status(tmpret); ret = dlm_err_to_dlm_status(tmpret);
...@@ -638,7 +644,9 @@ enum dlm_status dlmunlock(struct dlm_ctxt *dlm, struct dlm_lockstatus *lksb, ...@@ -638,7 +644,9 @@ enum dlm_status dlmunlock(struct dlm_ctxt *dlm, struct dlm_lockstatus *lksb,
if (status == DLM_RECOVERING || if (status == DLM_RECOVERING ||
status == DLM_MIGRATING || status == DLM_MIGRATING ||
status == DLM_FORWARD) { status == DLM_FORWARD ||
status == DLM_NOLOCKMGR) {
/* We want to go away for a tiny bit to allow recovery /* We want to go away for a tiny bit to allow recovery
* / migration to complete on this resource. I don't * / migration to complete on this resource. I don't
* know of any wait queue we could sleep on as this * know of any wait queue we could sleep on as this
...@@ -650,7 +658,7 @@ enum dlm_status dlmunlock(struct dlm_ctxt *dlm, struct dlm_lockstatus *lksb, ...@@ -650,7 +658,7 @@ enum dlm_status dlmunlock(struct dlm_ctxt *dlm, struct dlm_lockstatus *lksb,
msleep(50); msleep(50);
mlog(0, "retrying unlock due to pending recovery/" mlog(0, "retrying unlock due to pending recovery/"
"migration/in-progress\n"); "migration/in-progress/reconnect\n");
goto retry; goto retry;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment