[SSI] openssi/kernel/ipc sem.c,1.34,1.35

Roger Tsang <[email protected]> Fri, 02 Apr 2010 20:42:36 +0000
Newsgroups gmane.linux.cluster.ssic.cvs
Message-ID <[email protected]>
Update of /cvsroot/ssic-linux/openssi/kernel/ipc
In directory sfp-cvsdas-3.v30.ch3.sourceforge.com:/tmp/cvs-serv30086/kernel/ipc

Modified Files:
      Tag: OPENSSI-FC
	sem.c 
Log Message:
IPC Semaphores:
- sem_checkid() failure message level downgraded to KERN_DEBUG in __ssi_semexit() since the failure is not a bug [1941808].
- semundo_nodedown_thread() changes:
  - Skip remaining sem_array->undo list when a sem_semundo struct with pid is found since no other sem_semundo structures with same pid exists.
  - No longer hold sem_ids.sem mutex while traversing array of sem_array structures since we are only interested in existing semaphores at the time of node down event. There is no race with grow_ary().
  - Create semundo_nd_thread_lock mutex to prevent multiple semundo_nodedown_thread's from racing over pid_checked marker in sem_semundo structure.
  - Only call update_queue() when a sem_semundo structure is being removed.


Index: sem.c
===================================================================
RCS file: /cvsroot/ssic-linux/openssi/kernel/ipc/sem.c,v
retrieving revision 1.34
retrieving revision 1.35
diff -u -d -r1.34 -r1.35
--- sem.c	29 Mar 2010 06:20:55 -0000	1.34
+++ sem.c	2 Apr 2010 20:42:31 -0000	1.35
@@ -1936,12 +1936,14 @@
 	int nsems, i;
 
 	if (sem_checkid(sma, semid)) {
-		/* OpenSSI 1941808 - semundo structures confused.
-		 * sem_checkid() failure is not a bug because in OpenSSI
+		/* [ ssic-linux-Bugs-1941808 ] semundo structures confused.
+		 * sem_checkid() failure here is not a bug because in OpenSSI
 		 * tsk->sysvsem.undo_list (struct sem_undo) is separated from
-		 * sma->undo (struct sem_semundo).
+		 * sma->undo (struct sem_semundo). The old sem_array struct
+		 * was already freed by the current process. Another
+		 * process has created a new sem_array with the same ID.
 		 */
-		printk(KERN_INFO "%s: stale undo sem %d for pid %d\n",
+		printk(KERN_DEBUG "%s: stale undo sem %d for pid %d\n",
 					__FUNCTION__, semid, pid);
 		goto out_unlock;
 	}
@@ -2274,19 +2276,30 @@
  */
 void semundo_nodedown_thread(clusternode_t node)
 {
-	/* struct sem_queue *q; */
 	struct sem_semundo *un, **unp;
 	struct sem_array *sma;
-	int nsems, i, j;
+	int nsems, i, j, max_id;
 	pid_t pid = 0;
 	char pid_dead = 0;
 	char reset = 0; /* reset pid_checked marker */
+static DECLARE_MUTEX(semundo_nd_thread_lock);
 
 	extern int process_is_alive(pid_t);
 
+	down(&semundo_nd_thread_lock);
+
+	/* No need to hold sem_ids.sem mutex while traversing array of
+	 * sem_array structures since we are only interested in
+	 * existing semaphores at the time of node down event.
+	 */
+	/* SMP safe because sem_ids.entries->size does not shrink */
 	down(&sem_ids.sem);
+	max_id = sem_ids.entries->size;
+	up(&sem_ids.sem);
+
 again:
-	for (i=0; i < sem_ids.entries->size; i++) {
+	for (i=0; i < max_id; i++) {
+retry:
 		sma = sem_lock(i);
 		if (sma == NULL)
 			continue;
@@ -2294,30 +2307,36 @@
 			sem_unlock(sma);
 			continue;
 		}
-		for (unp = &sma->undo; (un = *unp);) {
+		for (unp = &sma->undo; (un = *unp); unp = &un->id_next) {
 			if (reset) {
 				un->pid_checked = 0;
-				unp = &un->id_next;
 				continue;
 			}
-			/* Check pid against all sem_semundo structures since
+
+			/* Check pid against all sem_array structures since
 			 * process_is_alive() is expensive.
 			 */
-			if (pid && un->pid != pid) {
-				unp = &un->id_next;
+			if (pid && un->pid != pid)
 				continue;
-			}
 			if (!un->pid_checked) {
 				un->pid_checked = 1;
 				if (!pid) {
 					pid = un->pid;
-					goto check_pid;
+					/* Drop lock. We might sleep. */
+					sem_unlock(sma);
+
+					BUG_ON(!pid);
+					if (!process_is_alive(pid))
+						pid_dead = 1;
+					goto retry;
 				}
 			}
-			if (!pid_dead) {
-				unp = &un->id_next;
-				continue;
-			}
+
+			/* There can only be one sem_semundo struct
+			 * with un->pid per struct sem_array->undo list.
+			 */
+			if (!pid_dead)
+				break;
 
 			/* Dupe of __ssi_semexit(); processing dead pid */
 			nsems = sma->sem_nsems;
@@ -2339,28 +2358,25 @@
 
 			*unp = un->id_next;
 			kfree(un);
+
+			update_queue(sma);
+			break;
 		}
-		update_queue(sma);
 		sem_unlock(sma);
 	}
 	if (pid) {
+		/* Traverse sem_semundo structures until all ->pid_checked */
 		pid = 0;
 		pid_dead = 0;
 		goto again;
 	}
 	if (!reset) {
-		/* Done nodedown. Reset un->pid_checked marker */
+		/* Done processing sem_semundo structures.
+		 * Reset un->pid_checked marker.
+		 */
 		reset = 1;
 		goto again;
 	}
-	up (&sem_ids.sem);
-	return;
-
-check_pid:
-	/* Drop lock. process_is_alive() might sleep. */
-	sem_unlock(sma);
-	if (!process_is_alive(pid))
-		pid_dead = 1;
-	goto again;
+	up(&semundo_nd_thread_lock);
 }
 #endif /* CONFIG_SSI */


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev