cluster: RHEL4 - dlm-kernel: can_be_granted workaround for user lockspaces

[David Teigland]

Gitweb:        http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=184...
Commit:        18411e7ca35ad8b970bb57192c89ef2b067c8432
Parent:        6397a742c02d186ac97d55dc9ea9d50dbbce2493
Author:        David Teigland 
AuthorDate:    Thu Oct 29 12:23:54 2009 -0500
Committer:     David Teigland 
CommitterDate: Thu Oct 29 12:23:54 2009 -0500

dlm-kernel: can_be_granted workaround for user lockspaces

bz 483685

There is a problem in the _can_be_granted() logic regarding the
waitqueue.  Normally, a lock request cannot be granted if there's
another lock already on the waitqueue.  This gives FIFO fairness.
One of the big problems in the RHEL4 dlm is that the waitqueue is
misused, and locks are kept there while waiting for on a remote
lock request.  This can mess up the normal lock granting logic.

Example:

Requests for lkb1 and lkb2 are both sent to another node we think
is master.  Both are kept on the rsb waitqueue while waiting for a
reply.  The remote node ends up not being the master and returns
EINVAL for lkb1.

Local processing of lkb1 after getting the EINVAL reply:
- lkb1 is removed from the waitqueue
- we look up the master again (resdir is local for this rsb)
- the master ends up being us
- rsb nodeid set to 0
- lkb1 is passed into dlm_lock_stage2 and dlm_lock_stage3
- lkb1 is passed into _can_be_granted
- _can_be_granted sees lkb2 on the waitqueue so says lkb1 cannot
  be granted
- so lkb1 is added to the waitqueue (in the proper sense,
  i.e. not because it's waiting for a remote reply but because
  of the master granting logic)

Local processing of lkb2 after getting its EINVAL reply:
- lkb2 removed from the waitqueue
- we see we are now the master, rsb nodid is 0
- lkb2 is passed into dlm_lock_stage2 and dlm_lock_stage3
- lkb2 is passed into _can_be_granted
- _can_be_granted sees lkb1 on the waitqueue so says lkb2 cannot be
  granted
- so lkb2 is added to the waitqueue (again in the proper sense)

Other lock requests then arrive for this rsb, and all continue to
be added to the waitqueue because it's not empty.

This patch makes _can_be_granted return TRUE for locks, like lkb1,
that are being requested/tested by the grant logic for the first
time since being requested, i.e. they are not already on the
waitqueue.

To avoid regressions in this particularly sensitive area of code,
the fix is only enabled for user lockspaces (e.g. clvmd and
rgmanager), and the fix can be disabled by

echo 0 > /proc/cluster/config/dlm/user_grant_now

in case it causes a regression in some user lockspace workload.

Signed-off-by: David Teigland 
---
 dlm-kernel/src/config.c       |    8 +++++++-
 dlm-kernel/src/config.h       |    1 +
 dlm-kernel/src/device.c       |   11 +++++++++++
 dlm-kernel/src/dlm_internal.h |    1 +
 dlm-kernel/src/locking.c      |    9 +++++++++
 5 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/dlm-kernel/src/config.c b/dlm-kernel/src/config.c
index 2858c4f..3ce052c 100644
--- a/dlm-kernel/src/config.c
+++ b/dlm-kernel/src/config.c
@@ -29,6 +29,7 @@
 #define DEFAULT_CONN_INCREMENT    32
 #define DEFAULT_DEADLOCKTIME      10
 #define DEFAULT_RECOVER_TIMER      5
+#define DEFAULT_USER_GRANT_NOW     1

struct config_info dlm_config = {
    .tcp_port = DEFAULT_TCP_PORT,
@@ -40,7 +41,8 @@ struct config_info dlm_config = {
    .dirtbl_size = DEFAULT_DIRTBL_SIZE,
    .conn_increment = DEFAULT_CONN_INCREMENT,
    .deadlocktime = DEFAULT_DEADLOCKTIME,
-	.recover_timer = DEFAULT_RECOVER_TIMER
+	.recover_timer = DEFAULT_RECOVER_TIMER,
+	.user_grant_now = DEFAULT_USER_GRANT_NOW
 };

@@ -87,6 +89,10 @@ static struct config_proc_info {
     {
    .name = "recover_timer",
    .value = &dlm_config.recover_timer,
+    },
+    {
+	.name = "user_grant_now",
+	.value = &dlm_config.user_grant_now,
     }
 };
 static struct proc_dir_entry *dlm_dir;
diff --git a/dlm-kernel/src/config.h b/dlm-kernel/src/config.h
index 0b37cf4..e0dab07 100644
--- a/dlm-kernel/src/config.h
+++ b/dlm-kernel/src/config.h
@@ -25,6 +25,7 @@ struct config_info {
    int conn_increment;
    int deadlocktime;
    int recover_timer;
+	int user_grant_now;
 };

extern struct config_info dlm_config;
diff --git a/dlm-kernel/src/device.c b/dlm-kernel/src/device.c
index d7afeaf..44e1379 100644
--- a/dlm-kernel/src/device.c
+++ b/dlm-kernel/src/device.c
@@ -37,6 +37,8 @@

#include "dlm_internal.h"
 #include "device.h"
+#include "config.h"
+#include "lockspace.h"

extern struct dlm_lkb *dlm_get_lkb(struct dlm_ls *, int);
 static struct file_operations _dlm_fops;
@@ -179,6 +181,7 @@ static void add_lockspace_to_list(struct user_ls *lsinfo)
    device for userland to access it */
 static int register_lockspace(char *name, struct user_ls **ls, int flags)
 {
+	struct dlm_ls *dlmls;
    struct user_ls *newls;
    int status;
    int namelen;
@@ -223,6 +226,14 @@ static int register_lockspace(char *name, struct user_ls **ls, int flags)
    if (flags & DLM_USER_LSFLG_DEFAULTLS)
    	set_bit(LS_FLAG_DEFAULT, &newls->ls_flags);

+	dlmls = find_lockspace_by_name(name, strlen(name));
+	if (!dlmls) {
+		log_print("skip GRANT_NOW flag, lockspace not found");
+	} else {
+		if (dlm_config.user_grant_now)
+			set_bit(LSFL_USER_GRANT_NOW, &dlmls->ls_flags);
+	}
+
    add_lockspace_to_list(newls);
    *ls = newls;
    return 0;
diff --git a/dlm-kernel/src/dlm_internal.h b/dlm-kernel/src/dlm_internal.h
index 3400ee7..ed54db3 100644
--- a/dlm-kernel/src/dlm_internal.h
+++ b/dlm-kernel/src/dlm_internal.h
@@ -232,6 +232,7 @@ struct dlm_recover {
 #define LSFL_ALL_NODES_VALID	(11)
 #define LSFL_REQUEST_WARN	(12)
 #define LSFL_RECOVERD_EXIT      (13)
+#define LSFL_USER_GRANT_NOW	(14)

#define LSST_NONE		(0)
 #define LSST_INIT		(1)
diff --git a/dlm-kernel/src/locking.c b/dlm-kernel/src/locking.c
index 51be67d..73897e9 100644
--- a/dlm-kernel/src/locking.c
+++ b/dlm-kernel/src/locking.c
@@ -229,6 +229,7 @@ static int conversion_deadlock_detect(struct dlm_rsb *rsb, struct dlm_lkb *lkb)

static int _can_be_granted(struct dlm_rsb *r, struct dlm_lkb *lkb, int now)
 {
+	struct dlm_ls *ls = r->res_ls;
    int8_t conv = (lkb->lkb_grmode != DLM_LOCK_IV);

/*
@@ -340,6 +341,14 @@ static int _can_be_granted(struct dlm_rsb *r, struct dlm_lkb *lkb, int now)
        first_in_list(lkb, &r->res_waitqueue))
    	return TRUE;

+	/* special case for
+	   https://bugzilla.redhat.com/show_bug.cgi?id=483685 */
+
+	if (test_bit(LSFL_USER_GRANT_NOW, &ls->ls_flags) &&
+	    now && !conv && list_empty(&r->res_convertqueue) &&
+	    list_empty(&r->res_grantqueue))
+		return TRUE;
+
  out:
    /*
     * The following, enabled by CONVDEADLK, departs from VMS.