cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [Cluster-devel] cluster/group/dlm_controld deadlock.c
@ 2007-08-23 19:08 teigland
  0 siblings, 0 replies; 3+ messages in thread
From: teigland @ 2007-08-23 19:08 UTC (permalink / raw)
  To: cluster-devel.redhat.com

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2007-08-23 19:08:11

Modified files:
	group/dlm_controld: deadlock.c 

Log message:
	needed to be a little more thorough in taking a canceled transaction
	out of the dependency graph for cases where multiple locks were blocked
	between the same two transactions

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/dlm_controld/deadlock.c.diff?cvsroot=cluster&r1=1.6&r2=1.7

--- cluster/group/dlm_controld/deadlock.c	2007/08/17 21:17:53	1.6
+++ cluster/group/dlm_controld/deadlock.c	2007/08/23 19:08:11	1.7
@@ -73,6 +73,8 @@
 	struct dlm_rsb		*rsb;       /* lock is on resource */
 	struct trans		*trans;     /* lock owned by this transaction */
 	struct list_head	trans_list; /* tr->locks */
+	struct trans		*waitfor_trans; /* the trans that's holding the
+						   lock that's blocking us */
 };
 
 /* waitfor pointers alloc'ed 4 at at time */
@@ -87,7 +89,9 @@
 							 waitfor */
 	int			waitfor_alloc;
 	int			waitfor_count;        /* count of in-use
-							 waitfor slots */
+							 waitfor slots and
+						         num of trans's we're
+						         waiting on */
 	struct trans		**waitfor;	      /* waitfor_alloc trans
 							 pointers */
 };
@@ -765,8 +769,6 @@
 			goto out_it;
 		}
 
-		log_group(ls, "read_checkpoint: ckpt read %llu bytes",
-			  (unsigned long long)iov.readSize);
 		section_len = iov.readSize;
 
 		if (!section_len)
@@ -1598,6 +1600,7 @@
 
 	tr->waitfor[tr->waitfor_count++] = granted_lkb->trans;
 	granted_lkb->trans->others_waiting_on_us++;
+	waiting_lkb->waitfor_trans = granted_lkb->trans;
 }
 
 /* for each trans, for each waiting lock, go to rsb of the lock,
@@ -1742,14 +1745,32 @@
 		if (lkb->lock.status == DLM_LKSTS_GRANTED)
 			continue;
 		send_cancel_lock(ls, tr, lkb);
-		tr->waitfor_count--;
-	}
 
-	if (tr->waitfor_count)
-		log_group(ls, "canceled trans has non-zero waitfor_count %d",
-			  tr->waitfor_count);
+		/* When this canceled trans has multiple locks all blocked by
+		   locks held by one other trans, that other trans is only
+		   added to tr->waitfor once, and only one of these waiting
+		   locks will have waitfor_trans set.  So, the lkb with
+		   non-null waitfor_trans was the first one responsible
+		   for adding waitfor_trans to tr->waitfor.
+
+		   We could potentially forget about keeping track of lkb->
+		   waitfor_trans, forget about calling remove_waitfor()
+		   here and just set tr->waitfor_count = 0 after this loop.
+		   The loss would be that waitfor_trans->others_waiting_on_us
+		   would not get decremented. */
+
+		if (lkb->waitfor_trans)
+			remove_waitfor(tr, lkb->waitfor_trans);
+	}
+
+	/* this shouldn't happen, if it does something's not working right */
+	if (tr->waitfor_count) {
+		log_group(ls, "cancel_trans: %llx non-zero waitfor_count %d",
+			  (unsigned long long)tr->xid, tr->waitfor_count);
+	}
 
-	/* this should now remove the canceled trans */
+	/* this should now remove the canceled trans since it now has a zero
+	   waitfor_count */
 	removed = reduce_waitfor_graph(ls);
 
 	if (!removed)
@@ -1772,11 +1793,12 @@
 	log_group(ls, "locks:");
 	
 	list_for_each_entry(lkb, &tr->locks, trans_list) {
-		log_group(ls, "  %s: id %08x gr %s rq %s pid %u \"%s\"",
+		log_group(ls, "  %s: id %08x gr %s rq %s pid %u:%u \"%s\"",
 			  status_str(lkb->lock.status),
 			  lkb->lock.id,
 			  dlm_mode_str(lkb->lock.grmode),
 			  dlm_mode_str(lkb->lock.rqmode),
+			  lkb->home,
 			  lkb->lock.ownpid,
 			  lkb->rsb->name);
 	}



^ permalink raw reply	[flat|nested] 3+ messages in thread
* [Cluster-devel] cluster/group/dlm_controld deadlock.c
@ 2007-08-13 15:32 teigland
  0 siblings, 0 replies; 3+ messages in thread
From: teigland @ 2007-08-13 15:32 UTC (permalink / raw)
  To: cluster-devel.redhat.com

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2007-08-13 15:32:05

Modified files:
	group/dlm_controld: deadlock.c 

Log message:
	put back the ability to do pid-based deadlock detection on 5.1 kernels

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/dlm_controld/deadlock.c.diff?cvsroot=cluster&r1=1.4&r2=1.5

--- cluster/group/dlm_controld/deadlock.c	2007/08/10 20:23:07	1.4
+++ cluster/group/dlm_controld/deadlock.c	2007/08/13 15:32:05	1.5
@@ -294,14 +294,14 @@
 	return create_lkb();
 }
 
-static void add_lock(struct lockspace *ls, struct dlm_rsb *r, int from_nodeid,
-		     struct pack_lock *lock)
+static struct dlm_lkb *add_lock(struct lockspace *ls, struct dlm_rsb *r,
+				int from_nodeid, struct pack_lock *lock)
 {
 	struct dlm_lkb *lkb;
 
 	lkb = get_lkb(r, lock);
 	if (!lkb)
-		return;
+		return NULL;
 
 	switch (lock->copy) {
 	case LOCAL_COPY:
@@ -354,6 +354,7 @@
 
 	if (list_empty(&lkb->list))
 		add_lkb(r, lkb);
+	return lkb;
 }
 
 static void parse_r_name(char *line, char *name)
@@ -376,6 +377,77 @@
 
 #define LOCK_LINE_MAX 1024
 
+/* old/original way of dumping (only master state) in 5.1 kernel;
+   does deadlock detection based on pid instead of xid */
+
+static int read_debugfs_master(struct lockspace *ls)
+{
+	FILE *file;
+	char path[PATH_MAX];
+	char line[LOCK_LINE_MAX];
+	struct dlm_rsb *r;
+	struct dlm_lkb *lkb;
+	struct pack_lock lock;
+	char r_name[65];
+	unsigned long long xid;
+	unsigned int waiting;
+	int r_len;
+	int rv;
+
+	snprintf(path, PATH_MAX, "/sys/kernel/debug/dlm/%s_master", ls->name);
+
+	file = fopen(path, "r");
+	if (!file)
+		return -1;
+
+	/* skip the header on the first line */
+	fgets(line, LOCK_LINE_MAX, file);
+
+	while (fgets(line, LOCK_LINE_MAX, file)) {
+		memset(&lock, 0, sizeof(struct pack_lock));
+
+		rv = sscanf(line, "%x %d %x %u %llu %x %hhd %hhd %hhd %u %d",
+			    &lock.id,
+			    &lock.nodeid,
+			    &lock.remid,
+			    &lock.ownpid,
+			    &xid,
+			    &lock.exflags,
+			    &lock.status,
+			    &lock.grmode,
+			    &lock.rqmode,
+			    &waiting,
+			    &r_len);
+
+		if (rv != 11) {
+			log_error("invalid debugfs line %d: %s", rv, line);
+			goto out;
+		}
+
+		memset(r_name, 0, sizeof(r_name));
+		parse_r_name(line, r_name);
+
+		r = get_resource(ls, r_name, r_len);
+		if (!r)
+			break;
+
+		/* we want lock.xid to be zero before calling add_lock
+		   so it will treat this like the full master copy (not
+		   partial).  then set the xid manually at the end to
+		   ownpid (there will be no process copy to merge and
+		   get the xid from in 5.1) */
+
+		set_copy(&lock);
+		lkb = add_lock(ls, r, our_nodeid, &lock);
+		if (!lkb)
+			break;
+		lkb->lock.xid = lock.ownpid;
+	}
+ out:
+	fclose(file);
+	return 0;
+}
+
 static int read_debugfs_locks(struct lockspace *ls)
 {
 	FILE *file;
@@ -1026,10 +1098,8 @@
 
 	rv = read_debugfs_locks(ls);
 	if (rv < 0) {
-#if 0
 		/* compat for RHEL5.1 kernels */
 		rv = read_debugfs_master(ls);
-#endif
 		if (rv < 0) {
 			log_error("can't read dlm debugfs file: %s",
 				  strerror(errno));



^ permalink raw reply	[flat|nested] 3+ messages in thread
* [Cluster-devel] cluster/group/dlm_controld deadlock.c
@ 2007-08-07 19:20 teigland
  0 siblings, 0 replies; 3+ messages in thread
From: teigland @ 2007-08-07 19:20 UTC (permalink / raw)
  To: cluster-devel.redhat.com

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2007-08-07 19:20:55

Modified files:
	group/dlm_controld: deadlock.c 

Log message:
	don't add the same transaction to a waitfor list more than once

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/dlm_controld/deadlock.c.diff?cvsroot=cluster&r1=1.2&r2=1.3

--- cluster/group/dlm_controld/deadlock.c	2007/08/06 21:50:26	1.2
+++ cluster/group/dlm_controld/deadlock.c	2007/08/07 19:20:54	1.3
@@ -732,7 +732,8 @@
 		return;
 	}
 
-	log_group(ls, "write_checkpoint: open ckpt handle %llx", (long long)h);
+	log_group(ls, "write_checkpoint: open ckpt handle %llx",
+		  (unsigned long long)h);
 	ls->lock_ckpt_handle = (uint64_t) h;
 
 	list_for_each_entry(r, &ls->resources, list) {
@@ -791,7 +792,7 @@
 	}
 	if (error != CPG_OK) {
 		log_error("cpg_mcast_joined error %d handle %llx",
-			  (int)error, (long long)h);
+			  (int)error, (unsigned long long)h);
 		disable_deadlock();
 		return -1;
 	}
@@ -1232,7 +1233,7 @@
 
 	if (waiting_lkb->trans->xid == granted_lkb->trans->xid) {
 		log_debug("waiting and granted same trans %llx",
-			  (long long)waiting_lkb->trans->xid);
+			  (unsigned long long)waiting_lkb->trans->xid);
 		return 0;
 	}
 
@@ -1240,19 +1241,35 @@
 				waiting_lkb->lock.rqmode);
 }
 
-/* TODO: don't add new waitfor trans if we're already waiting for the same
-   trans for another lock */
+static int in_waitfor(struct trans *tr, struct trans *add_tr)
+{
+	int i;
+
+	for (i = 0; i < tr->waitfor_alloc; i++) {
+		if (!tr->waitfor[i])
+			continue;
+		if (tr->waitfor[i] == add_tr)
+			return 1;
+	}
+	return 0;
+}
 
-static void add_waitfor(struct dlm_lkb *waiting_lkb,
+static void add_waitfor(struct lockspace *ls, struct dlm_lkb *waiting_lkb,
 			struct dlm_lkb *granted_lkb)
 {
-	struct trans *tr;
+	struct trans *tr = waiting_lkb->trans;
 	int old_alloc, i;
 
 	if (locks_compat(waiting_lkb, granted_lkb))
 		return;
 
-	tr = waiting_lkb->trans;
+	/* don't add the same trans to the waitfor list multiple times */
+	if (tr->waitfor_count && in_waitfor(tr, granted_lkb->trans)) {
+		log_group(ls, "trans %llx already waiting for trans %llx",
+			  (unsigned long long)tr->xid,
+			  (unsigned long long)granted_lkb->trans->xid);
+		return;
+	}
 
 	if (tr->waitfor_count == tr->waitfor_alloc) {
 		old_alloc = tr->waitfor_alloc;
@@ -1289,7 +1306,7 @@
 				if (granted_lkb->lock.status==DLM_LKSTS_WAITING)
 					continue;
 				/* granted_lkb status is GRANTED or CONVERT */
-				add_waitfor(waiting_lkb, granted_lkb);
+				add_waitfor(ls, waiting_lkb, granted_lkb);
 			}
 		}
 	}



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-08-23 19:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-23 19:08 [Cluster-devel] cluster/group/dlm_controld deadlock.c teigland
  -- strict thread matches above, loose matches on Subject: below --
2007-08-13 15:32 teigland
2007-08-07 19:20 teigland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).