[Ocfs2-commits] khackel commits r2049 - trunk/fs/ocfs2/dlm

svn-commits at oss.oracle.com svn-commits at oss.oracle.com
Thu Mar 24 19:25:06 CST 2005


Author: khackel
Signed-off-by: mfasheh
Signed-off-by: zab
Date: 2005-03-24 19:25:05 -0600 (Thu, 24 Mar 2005)
New Revision: 2049

Modified:
   trunk/fs/ocfs2/dlm/dlmast.c
   trunk/fs/ocfs2/dlm/dlmcommon.h
   trunk/fs/ocfs2/dlm/dlmconvert.c
   trunk/fs/ocfs2/dlm/dlmdomain.c
   trunk/fs/ocfs2/dlm/dlmlock.c
   trunk/fs/ocfs2/dlm/dlmmaster.c
   trunk/fs/ocfs2/dlm/dlmrecovery.c
   trunk/fs/ocfs2/dlm/dlmthread.c
   trunk/fs/ocfs2/dlm/dlmunlock.c
Log:
* removes some useless #warning statements
* fixes actual bugs in some #warnings
* change all list_del calls to list_del_init (so that checking
  against list_empty is always ok)
* fix some simple compile warnings
* get better handling of in-progress lock resources, using the
  DLM_FORWARD error status to have the caller retry
* remove unused list_heads on recovery structure

Signed-off-by: mfasheh
Signed-off-by: zab



Modified: trunk/fs/ocfs2/dlm/dlmast.c
===================================================================
--- trunk/fs/ocfs2/dlm/dlmast.c	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmast.c	2005-03-25 01:25:05 UTC (rev 2049)
@@ -300,7 +300,7 @@
 do_ast:
 	ret = DLM_NORMAL;
 	if (past->type == DLM_AST) {
-		list_del(&lock->list);
+		list_del_init(&lock->list);
 		list_add_tail(&lock->list, &res->granted);
 		dlmprintk("ast: adding to granted list... type=%d, "
 			  "convert_type=%d\n", lock->ml.type, lock->ml.convert_type);

Modified: trunk/fs/ocfs2/dlm/dlmcommon.h
===================================================================
--- trunk/fs/ocfs2/dlm/dlmcommon.h	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmcommon.h	2005-03-25 01:25:05 UTC (rev 2049)
@@ -354,9 +354,6 @@
 	int state;
 	u8 node_num;
 	struct list_head list;
-	struct list_head granted;
-	struct list_head converting;
-	struct list_head blocked;
 } dlm_reco_node_data;
 
 enum {

Modified: trunk/fs/ocfs2/dlm/dlmconvert.c
===================================================================
--- trunk/fs/ocfs2/dlm/dlmconvert.c	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmconvert.c	2005-03-25 01:25:05 UTC (rev 2049)
@@ -62,6 +62,8 @@
 					   dlm_lock *lock, int flags, int type);
 
 /* 
+ * this is only called directly by dlmlock(), and only when the 
+ * local node is the owner of the lockres
  * locking:
  *   caller needs:  none
  *   taken:         takes and drops res->spinlock
@@ -74,11 +76,17 @@
 	int call_ast = 0, kick_thread = 0;
 	dlm_status status;
 
-#warning i think i need some IN_PROGRESS work here
 	spin_lock(&res->spinlock);
+	/* we are not in a network handler, this is fine */
+	__dlm_wait_on_lockres(res);
+	res->state |= DLM_LOCK_RES_IN_PROGRESS;
+
 	status = __dlmconvert_master(dlm, res, lock, flags, type, 
 				     &call_ast, &kick_thread);
+
+	res->state &= ~DLM_LOCK_RES_IN_PROGRESS;
 	spin_unlock(&res->spinlock);
+	wake_up(&res->wq);
 
 	if (call_ast)
 		dlm_queue_ast(dlm, lock);
@@ -208,7 +216,7 @@
 		  res->lockname.name);
 
 	lock->ml.convert_type = type;
-	list_del(&lock->list);
+	list_del_init(&lock->list);
 	list_add_tail(&lock->list, &res->converting);
 
 unlock_exit:	
@@ -244,7 +252,7 @@
 	res->state |= DLM_LOCK_RES_IN_PROGRESS;
 
 	/* move lock to local convert queue */
-	list_del(&lock->list);
+	list_del_init(&lock->list);
 	list_add_tail(&lock->list, &res->converting);
 	if (lock->ml.convert_type != LKM_IVMODE) {
 		dlmprintk0("error! converting a remote lock that is already "
@@ -280,7 +288,7 @@
 
 	/* if it failed, move it back to granted queue */
 	if (status != DLM_NORMAL) {
-		list_del(&lock->list);
+		list_del_init(&lock->list);
 		list_add_tail(&lock->list, &res->granted);
 		lock->ml.convert_type = LKM_IVMODE;
 		lock->lksb->flags &= ~(DLM_LKSB_GET_LVB|DLM_LKSB_PUT_LVB);
@@ -347,6 +355,10 @@
 			dlmprintk("node %u returned DLM_MIGRATING "
 				  "from convert message!\n",
 				  res->owner);
+		} else if (ret == DLM_FORWARD) {
+			dlmprintk("node %u returned DLM_FORWARD "
+				  "from convert message!\n",
+				  res->owner);
 		}
 	} else {
 		dlmprintk("error occurred in net_send_message: %d\n", 
@@ -448,10 +460,16 @@
 		lksb->flags |= DLM_LKSB_GET_LVB;
 	}
 
-#warning i think we need some handling of IN_PROGRESS here!
 	spin_lock(&res->spinlock);
-	status = __dlmconvert_master(dlm, res, lock, flags, cnv->requested_type,
-				     &call_ast, &kick_thread);
+	if (res->state & DLM_LOCK_RES_IN_PROGRESS) {
+		status = DLM_FORWARD;
+	} else {
+		res->state |= DLM_LOCK_RES_IN_PROGRESS;
+		status = __dlmconvert_master(dlm, res, lock, flags, 
+					     cnv->requested_type,
+					     &call_ast, &kick_thread);
+		res->state &= ~DLM_LOCK_RES_IN_PROGRESS;
+	}
 	spin_unlock(&res->spinlock);
 
 

Modified: trunk/fs/ocfs2/dlm/dlmdomain.c
===================================================================
--- trunk/fs/ocfs2/dlm/dlmdomain.c	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmdomain.c	2005-03-25 01:25:05 UTC (rev 2049)
@@ -303,9 +303,8 @@
 
 static void dlm_migrate_all_locks(dlm_ctxt *dlm)
 {
-	int i, ret;
+	int i;
 	dlm_lock_resource *res;
-	struct list_head *iter;
 
 	dlmprintk("Migrating locks from domain %s\n", dlm->name);
 	spin_lock(&dlm->spinlock);

Modified: trunk/fs/ocfs2/dlm/dlmlock.c
===================================================================
--- trunk/fs/ocfs2/dlm/dlmlock.c	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmlock.c	2005-03-25 01:25:05 UTC (rev 2049)
@@ -179,13 +179,12 @@
 	res->state &= ~DLM_LOCK_RES_IN_PROGRESS;
 	if (status != DLM_NORMAL) {
 		/* remove from local queue if it failed */
-		list_del(&lock->list);
+		list_del_init(&lock->list);
 		lock->lksb->flags &= ~DLM_LKSB_GET_LVB;
 	}
 	spin_unlock(&res->spinlock);
 
 	dlm_lockres_calc_usage(dlm, res);
-bail:
 
 	wake_up(&res->wq);
 	return status;
@@ -426,13 +425,14 @@
 			status = dlmconvert_master(dlm, res, lock, flags, mode);
 		else 
 			status = dlmconvert_remote(dlm, res, lock, flags, mode);
-		if (status == DLM_RECOVERING || status == DLM_MIGRATING) {
+		if (status == DLM_RECOVERING || status == DLM_MIGRATING ||
+		    status == DLM_FORWARD) {
 			/* for now, see how this works without sleeping
 			 * and just retry right away.  I suspect the reco
 			 * or migration will complete fast enough that
 			 * no waiting will be necessary */
-			dlmprintk0("retrying convert with migration or "
-				   "recovery in progress\n");
+			dlmprintk0("retrying convert with migration/"
+				   "recovery/in-progress\n");
 			up_read(&dlm->recovery_sem);
 			goto retry_convert;
 		}

Modified: trunk/fs/ocfs2/dlm/dlmmaster.c
===================================================================
--- trunk/fs/ocfs2/dlm/dlmmaster.c	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmmaster.c	2005-03-25 01:25:05 UTC (rev 2049)
@@ -359,7 +359,7 @@
 
 	/* remove from list if not already */
 	if (!list_empty(&mle->list))
-		list_del(&mle->list);
+		list_del_init(&mle->list);
 
 	/* detach the mle from the domain node up/down events */
 	__dlm_mle_detach_hb_events(dlm, mle);
@@ -1535,7 +1535,7 @@
 			if (lock->ml.node != dlm->node_num) {
 				dlmprintk("freeing lock for node %u\n",
 					  lock->ml.node);
-				list_del(&lock->list);
+				list_del_init(&lock->list);
 				dlm_lockres_put(dlm, lock->lockres);
 				DLM_ASSERT(lock->lksb);
 				kfree(lock->lksb);
@@ -1740,7 +1740,7 @@
 		wake_up(&tmp->wq);
 		/* remove it from the list so that only one
 		 * mle will be found */
-		list_del(&tmp->list);
+		list_del_init(&tmp->list);
 		INIT_LIST_HEAD(&tmp->list);
 		spin_unlock(&tmp->spinlock);
 	}

Modified: trunk/fs/ocfs2/dlm/dlmrecovery.c
===================================================================
--- trunk/fs/ocfs2/dlm/dlmrecovery.c	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmrecovery.c	2005-03-25 01:25:05 UTC (rev 2049)
@@ -516,9 +516,6 @@
 		memset(ndata, 0, sizeof(dlm_reco_node_data));
 		ndata->node_num = num;
 		ndata->state = DLM_RECO_NODE_DATA_INIT;
-		INIT_LIST_HEAD(&ndata->granted);
-		INIT_LIST_HEAD(&ndata->converting);
-		INIT_LIST_HEAD(&ndata->blocked);
 		spin_lock(&dlm_reco_state_lock);
 		list_add_tail(&ndata->list, &dlm->reco.node_data);
 		spin_unlock(&dlm_reco_state_lock);
@@ -538,9 +535,9 @@
 	list_splice_init(&dlm->reco.node_data, &tmplist);
 	spin_unlock(&dlm_reco_state_lock);
 
-#warning this probably needs to be smarter
 	list_for_each_safe(iter, iter2, &tmplist) {
 		ndata = list_entry (iter, dlm_reco_node_data, list);
+		list_del_init(&ndata->list);
 		kfree(ndata);
 	}
 }
@@ -769,12 +766,12 @@
 			dlmprintk("found lockres owned by dead node while "
 				  "doing recovery for node %u. sending it.\n",
 				  dead_node);
-			list_del(&res->recovering);
+			list_del_init(&res->recovering);
 			list_add_tail(&res->recovering, list);
 		} else if (res->owner == DLM_LOCK_RES_OWNER_UNKNOWN) {
 			dlmprintk("found UNKNOWN owner while doing recovery "
 				  "for node %u. sending it.\n", dead_node);
-			list_del(&res->recovering);
+			list_del_init(&res->recovering);
 			list_add_tail(&res->recovering, list);
 		}
 	}
@@ -828,12 +825,18 @@
 	ret = net_send_message(DLM_MIG_LOCKRES_MSG, dlm->key, mres, 
 			       sz, send_to, &status);
 	if (ret < 0) {
-		dlmprintk("net_send_message returned %d\n", ret);
+		dlmerror("net_send_message returned %d\n", ret);
 	} else {
 		/* might get an -ENOMEM back here */
 		ret = status;
-		if (ret < 0) 
-			dlmprintk("reco data got status=%d\n", ret);
+		if (ret < 0) {
+			dlmerror("migrate lockres got status=%d\n", ret);
+			if (ret == -EFAULT) {
+				dlmerror("node %u told me to kill myself!\n",
+					 send_to);
+				BUG();
+			}
+		}
 	}
 
 	/* zero and reinit the message buffer */
@@ -1009,12 +1012,26 @@
 	if (res) {
 	 	/* this will get a ref on res */
 		/* mark it as recovering/migrating and hash it */
-#warning add checks of existing flags here
 		spin_lock(&res->spinlock);
-		if (mres->flags & DLM_MRES_RECOVERY)
+		if (mres->flags & DLM_MRES_RECOVERY) {
 			res->state |= DLM_LOCK_RES_RECOVERING;
-		else
+		} else {
+			if (res->state & DLM_LOCK_RES_MIGRATING) {
+				/* this is at least the second 
+				 * lockres message */
+				dlmprintk("lock %.*s is already migrating\n",
+					  mres->lockname_len,
+					  mres->lockname);
+			} else if (res->state & DLM_LOCK_RES_RECOVERING) {
+				/* caller should BUG */
+				dlmerror("node is attempting to migrate lock "
+					 "%.*s, but marked as recovering!\n",
+					 mres->lockname_len, mres->lockname);
+				ret = -EFAULT;
+				goto leave;
+			}
 			res->state |= DLM_LOCK_RES_MIGRATING;
+		}
 		spin_unlock(&res->spinlock);
 	} else {
 		/* need to allocate, just like if it was 
@@ -1049,7 +1066,6 @@
 			  "unknown owner.. will need to requery: "
 			  "%.*s\n", mres->lockname_len, mres->lockname);
 	} else {
-#warning is this the right time to do this?
 		spin_lock(&res->spinlock);
 		dlm_change_lockres_owner(dlm, res, dlm->node_num);
 		spin_unlock(&res->spinlock);
@@ -1084,7 +1100,6 @@
 {
 	dlm_ctxt *dlm = data;
 	dlm_migratable_lockres *mres;
-	dlm_status status = DLM_NORMAL;
 	int ret = 0;
 	dlm_lock_resource *res;
 	u8 real_master;
@@ -1115,7 +1130,7 @@
 				   "this node will take it.\n",
 				   res->lockname.len, res->lockname.name);
 		} else {
-			dlmprintk("master need to respond to sender "
+			dlmprintk("master needs to respond to sender "
 				  "that node %u still owns %.*s\n",
 				  real_master, res->lockname.len, 
 				  res->lockname.name);
@@ -1222,7 +1237,6 @@
 			  nodenum, *real_master);
 		ret = 0;
 	}
-leave:
 	return ret;
 }
 
@@ -1350,7 +1364,7 @@
 			 * to match the master here */
 				
 			/* move the lock to its proper place */
-			list_del(&lock->list);
+			list_del_init(&lock->list);
 			list_add_tail(&lock->list, queue);
 			spin_unlock(&res->spinlock);
 			
@@ -1428,7 +1442,7 @@
 {
 	res->state |= DLM_LOCK_RES_RECOVERING;
 	if (!list_empty(&res->recovering))
-		list_del(&res->recovering);
+		list_del_init(&res->recovering);
 	list_add_tail(&res->recovering, &dlm->reco.resources);
 }
 
@@ -1503,21 +1517,21 @@
 				list_for_each_safe(iter2, tmpiter, &res->granted) {
 					lock = list_entry (iter2, dlm_lock, list);
 					if (lock->ml.node == dead_node) {
-						list_del(&lock->list);
+						list_del_init(&lock->list);
 						kfree(lock);
 					}
 				}
 				list_for_each_safe(iter2, tmpiter, &res->converting) {
 					lock = list_entry (iter2, dlm_lock, list);
 					if (lock->ml.node == dead_node) {
-						list_del(&lock->list);
+						list_del_init(&lock->list);
 						kfree(lock);
 					}
 				}
 				list_for_each_safe(iter2, tmpiter, &res->blocked) {
 					lock = list_entry (iter2, dlm_lock, list);
 					if (lock->ml.node == dead_node) {
-						list_del(&lock->list);
+						list_del_init(&lock->list);
 						kfree(lock);
 					}
 				}
@@ -1743,7 +1757,6 @@
 	dlmprintk("node %u wants to recover node %u\n",
 		  br->node_idx, br->dead_node);
 	spin_lock(&dlm->spinlock);
-#warning need to do more here
 	if (dlm->reco.new_master != NM_INVALID_SLOT_NUM) {
 		dlmprintk("new_master already set to %u! "
 			  "that node had better be dead!!!\n",

Modified: trunk/fs/ocfs2/dlm/dlmthread.c
===================================================================
--- trunk/fs/ocfs2/dlm/dlmthread.c	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmthread.c	2005-03-25 01:25:05 UTC (rev 2049)
@@ -291,7 +291,7 @@
 
 		target->ml.type = target->ml.convert_type;
 		target->ml.convert_type = LKM_IVMODE;
-		list_del(&target->list);
+		list_del_init(&target->list);
 		list_add_tail(&target->list, &res->granted);
 
 		DLM_ASSERT(target->lksb);
@@ -348,7 +348,7 @@
 			  target->ml.type, target->ml.node);
 
 		// target->ml.type is already correct
-		list_del(&target->list);
+		list_del_init(&target->list);
 		list_add_tail(&target->list, &res->granted);
 
 		DLM_ASSERT(target->lksb);

Modified: trunk/fs/ocfs2/dlm/dlmunlock.c
===================================================================
--- trunk/fs/ocfs2/dlm/dlmunlock.c	2005-03-24 23:46:07 UTC (rev 2048)
+++ trunk/fs/ocfs2/dlm/dlmunlock.c	2005-03-25 01:25:05 UTC (rev 2049)
@@ -106,18 +106,18 @@
 
 	spin_lock(&res->spinlock);
 	if (res->state & DLM_LOCK_RES_IN_PROGRESS) {
-		if (!master_node) {
-		/* TODO: should we return -EAGAIN or something here? */
-			dlmprintk0("lockres in progress! eek!\n");
+		if (master_node) {
+			dlmprintk0("lockres in progress!\n");
+			spin_unlock(&res->spinlock);
+			return DLM_FORWARD;
 		}
-#warning THIS CAN SLEEP!!!
+		/* ok for this to sleep if not in a network handler */
 		__dlm_wait_on_lockres(res);
 		res->state |= DLM_LOCK_RES_IN_PROGRESS;
 	}
 	spin_lock(&lock->spinlock);
 
 	if (res->state & DLM_LOCK_RES_RECOVERING) {
-		/* !!!!! */
 		status = DLM_RECOVERING;
 		goto leave;
 	}
@@ -158,7 +158,7 @@
 	}
 
 	if (actions & DLM_UNLOCK_REMOVE_LOCK)
-		list_del(&lock->list);
+		list_del_init(&lock->list);
 	if (actions & DLM_UNLOCK_REGRANT_LOCK)
 		list_add_tail(&lock->list, &res->granted);
 
@@ -251,7 +251,10 @@
 		// successfully sent and received
 		if (status == DLM_CANCELGRANT)
 			ret = DLM_NORMAL;
-		else
+		else if (status == DLM_FORWARD) {
+			dlmprintk0("master was in-progress.  retry\n");
+			ret = DLM_FORWARD;
+		} else
 			ret = status;
 		lksb->status = status;
 	} else {
@@ -356,11 +359,13 @@
 		lksb->flags |= DLM_LKSB_PUT_LVB;
 		memcpy(&lksb->lvb[0], &unlock->lvb[0], DLM_LVB_LEN);
 	}
-#warning BUG! THIS CAN SLEEP!!!  
-	/* so either we should respond with EAGAIN in dlmunlock_master 
-	 * and skip the __dlm_wait_on_lockres, or this message type 
-	 * should be dispatched */
+
+	/* if this is in-progress, propagate the DLM_FORWARD
+	 * all the way back out */
 	status = dlmunlock_master(dlm, res, lock, lksb, flags, &ignore);
+	if (status == DLM_FORWARD)
+		dlmprintk0("lockres is in progress\n");
+
 	if (flags & LKM_PUT_LVB)
 		lksb->flags &= ~DLM_LKSB_PUT_LVB;
 
@@ -476,6 +481,7 @@
 	DLM_ASSERT(lock);
 	DLM_ASSERT(res);
 retry:
+	/* need to retry up here because owner may have changed */
 	dlmprintk("lock=%p res=%p\n", lock, res);
 
 	if (res->owner == dlm->node_num) {
@@ -491,9 +497,10 @@
 	}
 
 	if (status == DLM_RECOVERING ||
-	    status == DLM_MIGRATING) {
-		dlmprintk0("retrying unlock due to pending recovery "
-			   "or migration\n");
+	    status == DLM_MIGRATING ||
+	    status == DLM_FORWARD) {
+		dlmprintk0("retrying unlock due to pending recovery/"
+			   "migration/in-progress\n");
 		goto retry;
 	}
 	if (call_ast) {



More information about the Ocfs2-commits mailing list