lowcomms init fixes

Hello,

This patch resolves 2 problems. First, if we tear everything down, the connection cache and everything, and then go to create it again because we've registered a new lockspace, we will get a garbage connection for the first connection, because we never destroy the idr, which will still be holding references to memory. So this patch init's the connections_idr everytime lowcomms starts so we can make sure we don't have this problem. The second problem is that when we go to destroy all of the connections and remove the dlm_conn kmem cache, it will complain because there are still things allocated. This is because we generally do this

for (i = 0; i < max_nodeid; i++)

where max_nodeid is set to whatever the highest nodeid is requested, which starts@0, so we will always leave 1 thing allocated. This patch makes us set max_nodid appropriately, and fixes an inconsistent for loop in the code. Thank you,

Josef

index 2b32f3c..66986be 100644
@@ -196,7 +196,7 @@ static struct connection *__nodeid2con(i
 	}
 
 	if (nodeid > max_nodeid)
-		max_nodeid = nodeid;
+		max_nodeid = nodeid + 1;
 
 	return con;
 }

@@ -380,7 +380,7 @@ static void sctp_init_failed(void)
 	struct connection *con;
 
 	down(&connections_lock);
-	for (i=1; i<=max_nodeid; i++) {
+	for (i=1; i<max_nodeid; i++) {
 		con = __nodeid2con(i, 0);
 		if (!con)
 			continue;

@@ -1424,6 +1424,7 @@ int dlm_lowcomms_start(void)
 	int error = -EINVAL;
 	struct connection *con;
 
+	idr_init(&connections_idr);
 	init_local();
 	if (!dlm_local_count) {
 		log_print("no local IP address has been set");

@@ -1437,6 +1438,9 @@ int dlm_lowcomms_start(void)
 	if (!con_cache)
 		goto out;
 
+	/* set a default max_nodeid, it will be reset if we need more */
+	max_nodeid = 0;
+
 	/* Set some sysctl minima */
 	if (sysctl_rmem_max < NEEDED_RMEM)
 		sysctl_rmem_max = NEEDED_RMEM;

Ok I uncovered some other ugliness, when you go to run dlm_lowcomms_close when shutting down, you will get stuck in an infinite loop, because we had previously run dlm_lowcomms_stop when clvmd stopped, so we have freed all of our connections and we pull a garbage connection off of the idr, because we don't init it until we go to start the lowcomms again. So this is the same patch as before, except instead of init'ing the idr everytime we start lowcomms, we init it when we stop lowcomms so we don't have to worry about this kind of problem. Thank you,

Josef

index 2b32f3c..6fe250c 100644
@@ -196,7 +196,7 @@ static struct connection *__nodeid2con(i
 	}
 
 	if (nodeid > max_nodeid)
-		max_nodeid = nodeid;
+		max_nodeid = nodeid + 1;
 
 	return con;
 }

@@ -380,7 +380,7 @@ static void sctp_init_failed(void)
 	struct connection *con;
 
 	down(&connections_lock);
-	for (i=1; i<=max_nodeid; i++) {
+	for (i=1; i<max_nodeid; i++) {
 		con = __nodeid2con(i, 0);
 		if (!con)
 			continue;

@@ -1417,6 +1417,7 @@ void dlm_lowcomms_stop(void)
 	}
 	up(&connections_lock);
 	kmem_cache_destroy(con_cache);
+	idr_init(&connections_idr);
 }
 
 int dlm_lowcomms_start(void)

@@ -1437,6 +1438,9 @@ int dlm_lowcomms_start(void)
 	if (!con_cache)
 		goto out;
 
+	/* set a default max_nodeid, it will be reset if we need more */
+	max_nodeid = 0;
+
 	/* Set some sysctl minima */
 	if (sysctl_rmem_max < NEEDED_RMEM)
 		sysctl_rmem_max = NEEDED_RMEM;

Thanks for finding & fixing those bugs Josef.
I've changed the patch a little, making the comparison always <= max_nodeid, I prefer max_modeid to be what it says it as and not 1 more than the max node ID.

I'm a little concerned about the need for idr_init() in lowcomms_close(). Surely if the idr was properly cleaned out it would not be necessary - though somehow it seems to be, Do you know what's actually happening here ?

Anyway, here's the modified patch. it seems to work for me, but I'd be grateful if you could run it through your tests too, cos I'm supposed to be working on something else@the moment!

Steve, can you wait till Josef has tested this patch before adding it to the git tree? thanks.

--
Patrick

Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street,
Windsor, Berkshire, SL4 ITE, UK.
Registered in England and Wales under Company Registration No. 3798903 -------------- next part --------------
A non-text attachment was scrubbed...
Name: lowcomms-fix.patch
Type: text/x-patch
Size: 2509 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20070423/dea123b8/attachment.bin>

K I tested it and it works great, thanks much Patrick.

Josef