ocfs2: Remove CANCELGRANT from the view of dlmglue.

o2dlm has the non-standard behavior of providing a cancel callback
(unlock_ast) even when the cancel has failed (the locking operation
succeeded without canceling).  This is called CANCELGRANT after the
status code sent to the callback.  fs/dlm does not provide this
callback, so dlmglue must be changed to live without it.
o2dlm_unlock_ast_wrapper() in stackglue now ignores CANCELGRANT calls.

Because dlmglue no longer sees CANCELGRANT, ocfs2_unlock_ast() no longer
needs to check for it.  ocfs2_locking_ast() must catch that a cancel was
tried and clear the cancel state.

Making these changes opens up a locking race.  dlmglue uses the the
OCFS2_LOCK_BUSY flag to ensure only one thread is calling the dlm at any
one time.  But dlmglue must unlock the lockres before calling into the
dlm.  In the small window of time between unlocking the lockres and
calling the dlm, the downconvert thread can try to cancel the lock.  The
downconvert thread is checking the OCFS2_LOCK_BUSY flag - it doesn't
know that ocfs2_dlm_lock() has not yet been called.

Because ocfs2_dlm_lock() has not yet been called, the cancel operation
will just be a no-op.  There's nothing to cancel.  With CANCELGRANT,
dlmglue uses the CANCELGRANT callback to clear up the cancel state.
When it comes around again, it will retry the cancel.  Eventually, the
first thread will have called into ocfs2_dlm_lock(), and either the
lock or the cancel will succeed.  The downconvert thread can then do its
downconvert.

Without CANCELGRANT, there is nothing to clean up the cancellation
state.  The downconvert thread does not know to retry its operations.
More importantly, the original lock may be blocking on the other node
that is trying to cancel us.  With neither able to make progress, the
ast is never called and the cancellation state is never cleaned up that
way.  dlmglue is deadlocked.

The OCFS2_LOCK_PENDING flag is introduced to remedy this window.  It is
set at the same time OCFS2_LOCK_BUSY is.  Thus, the downconvert thread
can check whether the lock is cancelable.  If not, it just loops around
to try again.  Once ocfs2_dlm_lock() is called, the thread then clears
OCFS2_LOCK_PENDING and wakes the downconvert thread.  Now, if the
downconvert thread finds the lock BUSY, it can safely try to cancel it.
Whether the cancel works or not, the state will be properly set and the
lock processing can continue.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
Signed-off-by: Mark Fasheh <mfasheh@suse.com>

1 files changed, 13 insertions, 27 deletions

diff --git a/fs/ocfs2/stackglue.c b/fs/ocfs2/stackglue.c
index 670fa945c212..abdb9f6f4cc9 100644
--- a/fs/ocfs2/stackglue.c
+++ b/fs/ocfs2/stackglue.c
@@ -104,8 +104,8 @@ static int flags_to_o2dlm(u32 flags)
  *
  * DLM_NORMAL:          0
  * DLM_NOTQUEUED:       -EAGAIN
- * DLM_CANCELGRANT:     -DLM_ECANCEL
- * DLM_CANCEL:          -DLM_EUNLOCK
+ * DLM_CANCELGRANT:     -EBUSY
+ * DLM_CANCEL:          -DLM_ECANCEL
  */
 /* Keep in sync with dlmapi.h */
 static int status_map[] = {
@@ -113,13 +113,13 @@ static int status_map[] = {
         [DLM_GRANTED]                   = -EINVAL,
         [DLM_DENIED]                    = -EACCES,
         [DLM_DENIED_NOLOCKS]            = -EACCES,
-        [DLM_WORKING]                   = -EBUSY,
+        [DLM_WORKING]                   = -EACCES,
         [DLM_BLOCKED]                   = -EINVAL,
         [DLM_BLOCKED_ORPHAN]            = -EINVAL,
         [DLM_DENIED_GRACE_PERIOD]       = -EACCES,
         [DLM_SYSERR]                    = -ENOMEM,      /* It is what it is */
         [DLM_NOSUPPORT]                 = -EPROTO,
-        [DLM_CANCELGRANT]               = -DLM_ECANCEL, /* Cancel after grant */
+        [DLM_CANCELGRANT]               = -EBUSY,       /* Cancel after grant */
         [DLM_IVLOCKID]                  = -EINVAL,
         [DLM_SYNC]                      = -EINVAL,
         [DLM_BADTYPE]                   = -EINVAL,
@@ -137,7 +137,7 @@ static int status_map[] = {
         [DLM_VALNOTVALID]               = -EINVAL,
         [DLM_REJECTED]                  = -EPERM,
         [DLM_ABORT]                     = -EINVAL,
-        [DLM_CANCEL]                    = -DLM_EUNLOCK, /* Successful cancel */
+        [DLM_CANCEL]                    = -DLM_ECANCEL, /* Successful cancel */
         [DLM_IVRESHANDLE]               = -EINVAL,
         [DLM_DEADLOCK]                  = -EDEADLK,
         [DLM_DENIED_NOASTS]             = -EINVAL,
@@ -152,6 +152,7 @@ static int status_map[] = {
         [DLM_MIGRATING]                 = -ERESTART,
         [DLM_MAXSTATS]                  = -EINVAL,
 };
+
 static int dlm_status_to_errno(enum dlm_status status)
 {
         BUG_ON(status > (sizeof(status_map) / sizeof(status_map[0])));
@@ -175,38 +176,23 @@ static void o2dlm_blocking_ast_wrapper(void *astarg, int level)
 
 static void o2dlm_unlock_ast_wrapper(void *astarg, enum dlm_status status)
 {
-        int error;
+        int error = dlm_status_to_errno(status);
 
         BUG_ON(lproto == NULL);
 
         /*
-         * XXX: CANCEL values are sketchy.
-         *
-         * Currently we have preserved the o2dlm paradigm.  You can get
-         * unlock_ast() whether the cancel succeded or not.
-         *
-         * First, we're going to pass DLM_EUNLOCK just like fs/dlm does for
-         * successful unlocks.  That is a clean behavior.
-         *
          * In o2dlm, you can get both the lock_ast() for the lock being
          * granted and the unlock_ast() for the CANCEL failing.  A
          * successful cancel sends DLM_NORMAL here.  If the
          * lock grant happened before the cancel arrived, you get
-         * DLM_CANCELGRANT.  For now, we'll use DLM_ECANCEL to signify
-         * CANCELGRANT - the CANCEL was supposed to happen but didn't.  We
-         * can then use DLM_EUNLOCK to signify a successful CANCEL -
-         * effectively, the CANCEL caused the lock to roll back.
+         * DLM_CANCELGRANT.
          *
-         * In the future, we will likely move the o2dlm to send only one
-         * ast - either unlock_ast() for a successful CANCEL or lock_ast()
-         * when the grant succeeds.  At that point, we'll send DLM_ECANCEL
-         * for all cancel results (CANCELGRANT will no longer exist).
+         * There's no need for the double-ast.  If we see DLM_CANCELGRANT,
+         * we just ignore it.  We expect the lock_ast() to handle the
+         * granted lock.
          */
-        error = dlm_status_to_errno(status);
-
-        /* Successful unlock is DLM_EUNLOCK */
-        if (!error)
-                error = -DLM_EUNLOCK;
+        if (status == DLM_CANCELGRANT)
+                return;
 
         lproto->lp_unlock_ast(astarg, error);
 }