[SSI] openssi/kernel/ipc sem.c,1.36,1.37

Roger Tsang <[email protected]> Sun, 18 Apr 2010 20:36:37 +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-serv9429/ipc

Modified Files:
      Tag: OPENSSI-FC
	sem.c 
Log Message:
- Avoid duplicate pid alive checks when there are multiple semundo_nodedown_thread()'s. Last thread to reset sem_semundo->pid_checked. This will prevent the process_is_alive() storm but depending on timing may leave behind some sem_semundo structs until the next node down event or until the shared semaphore is removed.
- Fix semundo_nodedown_thread() skipped remaining sem_semundo structs in sem_array after encountering sem_semundo with pid_checked and pid was not dead. Bug introduced in OPENSSI-FC-1-9-6-PRE30.
- Remove redundant code in semundo_nodedown_thread(). Call __ssi_semexit() instead.
- Move beginning of __ssi_semexit() into ssi_semexit() so that semundo_nodedown_thread() can use __ssi_semexit().


Index: sem.c
===================================================================
RCS file: /cvsroot/ssic-linux/openssi/kernel/ipc/sem.c,v
retrieving revision 1.36
retrieving revision 1.37
diff -u -d -r1.36 -r1.37
--- sem.c	3 Apr 2010 19:41:51 -0000	1.36
+++ sem.c	18 Apr 2010 20:36:35 -0000	1.37
@@ -1854,40 +1854,11 @@
 }
 
 /* Called with sem_array locked */
-static inline void __ssi_semexit(int semid, pid_t pid, struct sem_array *sma)
+/* Taken from ipc/sem.c:exit_sem() */
+static inline void __ssi_semexit(struct sem_semundo *un, struct sem_array *sma)
 {
-	struct sem_semundo *un, **unp;
 	int nsems, i;
 
-	if (sem_checkid(sma, semid)) {
-		sem_unlock(sma);
-		/* [ 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). 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_DEBUG "%s: stale undo sem %d for pid %d\n",
-					__FUNCTION__, semid, pid);
-		return;
-	}
-
-	/* remove u from the sma->undo list */
-	for (unp = &sma->undo; (un = *unp); unp = &un->id_next) {
-		/* u == un */
-		if (pid == un->pid) {
-			*unp = un->id_next;
-			break;
-		}
-	}
-	if (!un) {
-		sem_unlock(sma);
-		printk(KERN_WARNING "%s: missing undo sem %d for pid %d\n",
-					__FUNCTION__, semid, pid);
-		return;
-	}
-
 	/* perform adjustments registered in u */
 	nsems = sma->sem_nsems;
 	for (i = 0; i < nsems; i++) {
@@ -1917,15 +1888,12 @@
 	sma->sem_otime = get_seconds();
 	/* maybe some queued-up processes were waiting for this */
 	update_queue(sma);
-	sem_unlock(sma);
-
-	/* Unlike base code we must free our sem_semundo, no-one else will
-	   do it for us */
-	kfree (un);
 }
 
+/* Taken from ipc/sem.c:exit_sem() */
 void ssi_semexit(int semid, pid_t pid)
 {
+	struct sem_semundo *un, **unp;
 	struct sem_array *sma;
 
 	if(semid == -1)
@@ -1935,7 +1903,41 @@
 	if (sma == NULL)
 		return;
 
-	__ssi_semexit(semid, pid, sma);
+	if (sem_checkid(sma, semid)) {
+		sem_unlock(sma);
+		/* [ 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). 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_DEBUG "%s: stale undo sem %d for pid %d\n",
+					__FUNCTION__, semid, pid);
+		return;
+	}
+
+	/* remove u from the sma->undo list */
+	for (unp = &sma->undo; (un = *unp); unp = &un->id_next) {
+		/* u == un */
+		if (pid == un->pid) {
+			*unp = un->id_next;
+			break;
+		}
+	}
+	if (!un) {
+		sem_unlock(sma);
+		printk(KERN_WARNING "%s: missing undo sem %d for pid %d\n",
+					__FUNCTION__, semid, pid);
+		return;
+	}
+
+	__ssi_semexit(un, sma);
+	sem_unlock(sma);
+
+	/* Unlike base code we must free our sem_semundo, no-one else will
+	   do it for us */
+	kfree (un);
 }
 #endif /* CONFIG_SSI */
 
@@ -2210,15 +2212,16 @@
 {
 	struct sem_semundo *un, **unp;
 	struct sem_array *sma;
-	int nsems, i, j, max_id;
+	int i, max_id;
 	pid_t pid = 0;
 	char pid_dead = 0;
 	char reset = 0; /* reset pid_checked marker */
-static DECLARE_MUTEX(semundo_nd_thread_lock);
+static DECLARE_RWSEM(semundo_nd_thread_sem);
+static atomic_t semundo_nd_threads = ATOMIC_INIT(0);
 
 	extern int process_is_alive(pid_t);
 
-	down(&semundo_nd_thread_lock);
+	atomic_inc(&semundo_nd_threads);
 
 	/* No need to hold sem_ids.sem mutex while traversing array of
 	 * sem_array structures since we are only interested in
@@ -2229,6 +2232,9 @@
 	max_id = sem_ids.entries->size;
 	up(&sem_ids.sem);
 
+	/* Prevent sem_semundo->pid_checked reset race */
+	down_read(&semundo_nd_thread_sem);
+
 again:
 	for (i=0; i < max_id; i++) {
 retry:
@@ -2241,7 +2247,8 @@
 		}
 		for (unp = &sma->undo; (un = *unp); unp = &un->id_next) {
 			if (reset) {
-				un->pid_checked = 0;
+				if (un->pid_checked)
+					un->pid_checked = 0;
 				continue;
 			}
 
@@ -2267,34 +2274,24 @@
 			/* There can only be one sem_semundo struct
 			 * with un->pid per struct sem_array->undo list.
 			 */
-			if (!pid_dead)
+			if (!pid_dead) {
+				if (!pid)
+					continue;
 				break;
-
-			/* Dupe of __ssi_semexit(); processing dead pid */
-			nsems = sma->sem_nsems;
-			for (j = 0; j < nsems; j++) {
-				struct sem * sem = &sma->sem_base[i];
-				sem->semval += un->semadj[i];
-				if (sem->semval < 0) { /*shudn't happen */
-					printk(KERN_WARNING
-						"%s: semval %d < 0\n",
-						__FUNCTION__,
-						sem->semval);
-					sem->semval = 0;
-				}
-				if (sem->semval > SEMVMX)
-					sem->semval = SEMVMX;
-				sem->sempid = un->pid;
 			}
-			sma->sem_otime = get_seconds();
+
+			__ssi_semexit(un, sma);
 
 			*unp = un->id_next;
 			kfree(un);
-
-			update_queue(sma);
 			break;
 		}
 		sem_unlock(sma);
+		/* Stop resetting ->pid_checked if there is a pending thread */
+		if (reset && atomic_read(&semundo_nd_threads)) {
+			up_write(&semundo_nd_thread_sem);
+			return;
+		}
 	}
 	if (pid) {
 		/* Traverse sem_semundo structures until all ->pid_checked */
@@ -2302,13 +2299,23 @@
 		pid_dead = 0;
 		goto again;
 	}
+	/* Done processing sem_semundo structures.
+	 * Last thread to reset un->pid_checked marker.
+	 */
 	if (!reset) {
-		/* Done processing sem_semundo structures.
-		 * Reset un->pid_checked marker.
-		 */
+		if (!atomic_dec_and_test(&semundo_nd_threads)) {
+			up_read(&semundo_nd_thread_sem);
+			return;
+		}
+		up_read(&semundo_nd_thread_sem);
+		down_write(&semundo_nd_thread_sem);
+		if (atomic_read(&semundo_nd_threads)) {
+			up_write(&semundo_nd_thread_sem);
+			return;
+		}
 		reset = 1;
 		goto again;
 	}
-	up(&semundo_nd_thread_lock);
+	up_write(&semundo_nd_thread_sem);
 }
 #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