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

Update of /cvsroot/ssic-linux/openssi/kernel/ipc
In directory sfp-cvsdas-3.v30.ch3.sourceforge.com:/tmp/cvs-serv17839/kernel/ipc

Modified Files:
      Tag: OPENSSI-FC
	sem.c 
Log Message:
IPC Semaphores:
- Fix sem_nodehint[] race. Implement sem_nodehint_lock read/write spinlock.
- Fix sys_semctl() error paths livelock due to stale values.
- Fix livelock on -EINVAL in sys_semtimedop().
- Consolidate redundant code. Implement ipc/sem.c:sem_find_svr_node().
- Fix semundo_nodedown_thread() partial hang due to might sleep in process_is_alive() while holding IPC sem spinlock.
  - Add pid_checked marker in struct sem_semundo for node down processing.
- Fix semundo_nodedown_thread() dereferencing invalid struct sem_array.
- semundo_nodedown_thread() to scan sem_ids.entries->size instead of the maximum possible (sem_ids.max_id).

 include/linux/sem.h |    7 
 ipc/sem.c           |  360 +++++++++++++++++++++++++-------------------
 2 files changed, 213 insertions(+), 154 deletions(-)


Index: sem.c
===================================================================
RCS file: /cvsroot/ssic-linux/openssi/kernel/ipc/sem.c,v
retrieving revision 1.33
retrieving revision 1.34
diff -u -d -r1.33 -r1.34
--- sem.c	5 Mar 2010 06:30:21 -0000	1.33
+++ sem.c	29 Mar 2010 06:20:55 -0000	1.34
@@ -144,7 +144,9 @@
 static void freeundos(int);
 static clusternode_t sem_get_svr_node(int);
 static void sem_set_svr_node(int, clusternode_t);
+static clusternode_t sem_find_svr_node(int);
 
+static __cacheline_aligned_in_smp DEFINE_RWLOCK(sem_nodehint_lock);
 clusternode_t *sem_nodehint;
 int sem_nodehint_sz = 0;
 #endif /* CONFIG_SSI */
@@ -259,19 +261,34 @@
 }
 
 #ifdef CONFIG_SSI
-static clusternode_t sem_get_svr_node(int semid)
+static int __sem_get_idx(int semid)
 {
-	clusternode_t svr_node = 0;
 	int lid;
 
-	if (sem_nodehint == NULL) {
+	if (unlikely(sem_nodehint == NULL)) {
 		SSI_ASSERT(sem_nodehint_sz == 0);
-		return 0;
+		return -1;
 	}
 
 	lid = semid % SEQ_MULTIPLIER;
-	if (lid < sem_nodehint_sz)
-		svr_node = sem_nodehint[lid];
+	if (lid >= sem_nodehint_sz)
+		return -1;
+
+	return lid;
+}
+
+static clusternode_t sem_get_svr_node(int semid)
+{
+	clusternode_t svr_node;
+	int lid;
+
+	lid = __sem_get_idx(semid);
+	if (lid == -1)
+		return 0;
+
+	read_lock(&sem_nodehint_lock);
+	svr_node = sem_nodehint[lid];
+	read_unlock(&sem_nodehint_lock);
 
 	return svr_node;
 }
@@ -280,14 +297,21 @@
 {
 	int lid;
 
-	if (sem_nodehint == NULL) {
-		SSI_ASSERT(sem_nodehint_sz == 0);
+	lid = __sem_get_idx(semid);
+	if (lid == -1)
+		return;
+
+	read_lock(&sem_nodehint_lock);
+	if (sem_nodehint[lid] == svr_node) {
+		read_unlock(&sem_nodehint_lock);
 		return;
 	}
+	read_unlock(&sem_nodehint_lock);
 
-	lid = semid % SEQ_MULTIPLIER;
-	if (lid < sem_nodehint_sz)
+	write_lock(&sem_nodehint_lock);
+	if (sem_nodehint[lid] != svr_node)
 		sem_nodehint[lid] = svr_node;
+	write_unlock(&sem_nodehint_lock);
 }
 
 long ssi_semget(key_t key, int nsems, int semflg, struct sem_array* sma)
@@ -338,7 +362,10 @@
 		if (server == this_node) {
 			/* Create the server structure */
 			if ((retval = newary(key, nsems, semflg, newid)) < 0) {
-				printk("Unable to add new id %lu to sem_ids, removing the object on the nameserver\n", newid);
+				printk(KERN_WARNING "%s: Unable to add new id "
+					"%lu to sem_ids, removing the object "
+					"on the nameserver\n",
+					__FUNCTION__, newid);
 				cli_ipcname_rmid(NAME_SERVICE_SEM, newid);
 				server = 0;
 			}
@@ -606,8 +633,7 @@
 static void freeary (struct sem_array *sma, int id)
 {
 #ifdef CONFIG_SSI
-	struct sem_semundo *un;
-	struct sem_semundo *u;
+	struct sem_semundo **un, *u;
 #else
 	struct sem_undo *un;
 #endif /* CONFIG_SSI */
@@ -615,11 +641,7 @@
 	int size;
 
 #ifdef CONFIG_SSI
-	for (un = sma->undo; un;) {
-		u = un;
-		un = u->id_next;
-		kfree(u);
-	}
+	for (un = &sma->undo; (u = *un); *un = u->id_next, kfree(u));
 #else
 	/* Invalidate the existing undo structures for this semaphore set.
 	 * (They will be freed without any further action in exit_sem()
@@ -1106,23 +1128,21 @@
 	int version;
 
 #ifdef CONFIG_SSI
+	clusternode_t svr_node;
 	int rval;
-	int flags, view, sz;
-	clusternode_t svr_node=0;
-	ssi_procstate_t pstate;
-	key_t key;
 	int tmpcmd = cmd;
 
 	remote_cmd = FALSE;
+
 	version = ipc_parse_version(&cmd);
 	if ((cmd == SEM_INFO) || (cmd == IPC_INFO)) {
 		svr_node = this_node;
-
 		if (semid == -1) {
 			remote_cmd=TRUE;
 			semid = 0;
 		}
-	}
+	} else
+		svr_node = 0;
 	cmd = tmpcmd;
 #endif /* CONFIG_SSI */
 
@@ -1130,48 +1150,36 @@
 		return -EINVAL;
 
 #ifdef CONFIG_SSI
-	if ((semid >= 0)&&(cmd >= 0)) {
+	if (svr_node != this_node &&
+	    semid >= 0 && cmd >= 0) {
 namesvr_go:
-		if (!svr_node)
-			svr_node = sem_get_svr_node(semid);
+		svr_node = sem_get_svr_node(semid);
 		if (!svr_node) {
-			if (cli_ipcname_findid(NAME_SERVICE_SEM, semid, &key,
-					&svr_node, &flags, &view, &sz)<0) {
-				sem_set_svr_node(semid, 0);
+			svr_node = sem_find_svr_node(semid);
+			if (!svr_node)
 				return -EINVAL;
-			} else if (svr_node == CLUSTERNODE_INVAL) {
-				sem_set_svr_node(semid, 0);
-				idelay(HZ/10);
-				goto namesvr_go;
-			} else if (svr_node) {
-				down(&sem_ids.sem);
-				sem_set_svr_node(semid, svr_node);
-				up(&sem_ids.sem);
-			}
+			sem_set_svr_node(semid, svr_node);
 		}
 
-		if (svr_node && (svr_node != this_node)) {
+		if (svr_node != this_node) {
+			ssi_procstate_t pstate;
 			int status;
+
 			ssi_procstate_get(&pstate);
 			status = cli_ripc_semctl(svr_node, &rval, semid,
-						 semnum, cmd, &pstate,
-						 &arg, 1);
-			version = ipc_parse_version(&cmd);
-			/* If we are able to invalidate server semaphore objects
-			   delete the local objects. */
+						 semnum, cmd, &pstate, &arg, 1);
 			if (status) {
-				cmd = tmpcmd;
 				sem_set_svr_node(semid, 0);
+				idelay(HZ/10);
 				goto namesvr_go;
-			} else if ((rval == 0) && (cmd == IPC_RMID)) {
+			}
+			if (rval == 0 && cmd == IPC_RMID) {
 				/* If we are able to invalidate server semaphore
 				   objects, delete the local objects. */
 				freeundos(semid);
 				sem_set_svr_node(semid, 0);
-				return rval;
 			}
-			else
-				return rval;
+			return rval;
 		}
 		cmd = tmpcmd;
 	}
@@ -1632,9 +1640,8 @@
 		 * for this process and this semaphore set.
 		 */
 		un = find_undo(semid);
-		if (IS_ERR(un)) {
+		if (IS_ERR(un))
 			error = PTR_ERR(un);
-		}
 	}
 
 	return error;
@@ -1661,12 +1668,7 @@
 {
 	int error = -EINVAL;
 #ifdef CONFIG_SSI
-	int rval, flags, view, sz;
-	clusternode_t svr_node=0;
-	ssi_procstate_t pstate;
-	ics_userbuf_t utsops;
-	ics_userbuf_t utimeout;
-	key_t key;
+	clusternode_t svr_node;
 #else
 	struct sem_array *sma;
 	struct sembuf fast_sops[SEMOPM_FAST];
@@ -1686,46 +1688,37 @@
 namesvr_op_go:
 	svr_node = sem_get_svr_node(semid);
 	if (!svr_node) {
-		if (cli_ipcname_findid(NAME_SERVICE_SEM, semid, &key,
-					&svr_node, &flags, &view, &sz)<0) {
-			sem_set_svr_node(semid, 0);
+		svr_node = sem_find_svr_node(semid);
+		if (!svr_node)
 			return -EINVAL;
-		} else if (svr_node == CLUSTERNODE_INVAL) {
-			sem_set_svr_node(semid, 0);
-			idelay(HZ/10);
-			goto namesvr_op_go;
-		} else if (svr_node) {
-			down(&sem_ids.sem);
-			sem_set_svr_node(semid, svr_node);
-			up(&sem_ids.sem);
-		}
+		sem_set_svr_node(semid, svr_node);
 	}
-	if (svr_node && (svr_node != this_node)) {
-		ssi_procstate_get(&pstate);
+	if (svr_node != this_node) {
+		ssi_procstate_t pstate;
+		ics_userbuf_t utsops;
+		ics_userbuf_t utimeout;
+		int status;
 
+		ssi_procstate_get(&pstate);
 		ics_userbuf_set(&utsops, tsops, nsops * sizeof(*tsops));
 		ics_userbuf_set(&utimeout, timeout, sizeof(*timeout));
-		cli_ripc_semop(svr_node, &rval, semid, nsops,
-			       &pstate, &utsops, &utimeout);
 
-		if(!rval)
-			undocheck(tsops, semid, nsops);
-		if (rval == -EINVAL) {
+		status = cli_ripc_semop(svr_node, &error, semid, nsops,
+			       &pstate, &utsops, &utimeout);
+		if (status) {
 			sem_set_svr_node(semid, 0);
+			idelay(HZ/10);
 			goto namesvr_op_go;
 		}
-		return rval;
+		if (!error)
+			undocheck(tsops, semid, nsops);
 	} else {
 		error = ssi_semop(semid, tsops, nsops, timeout);
-		if(!error) {
+		if(!error)
 			undocheck(tsops, semid, nsops);
-		} else if (error == -EINVAL) {
-			sem_set_svr_node(semid, 0);
-			goto namesvr_op_go;
-		}
-		return error;
 	}
-#else
+	return error;
+#else /* CONFIG_SSI */
 	if(nsops > SEMOPM_FAST) {
 		sops = kmalloc(sizeof(*sops)*nsops,GFP_KERNEL);
 		if(sops==NULL)
@@ -1873,7 +1866,7 @@
 	if(sops != fast_sops)
 		kfree(sops);
 	return error;
-#endif /* CONFIG_SSI */
+#endif /* !CONFIG_SSI */
 }
 
 asmlinkage long sys_semop (int semid, struct sembuf __user *tsops, unsigned nsops)
@@ -1909,15 +1902,48 @@
 }
 
 #ifdef CONFIG_SSI
+static clusternode_t sem_find_svr_node(int semid)
+{
+	clusternode_t svr_node;
+	key_t key;
+	int rval, flags, view, sz;
+
+	for (;;) {
+		svr_node = sem_get_svr_node(semid);
+		if (svr_node)
+			break;
+		rval = cli_ipcname_findid(NAME_SERVICE_SEM, semid, &key,
+						&svr_node, &flags, &view, &sz);
+		if (rval < 0) {
+			printk(KERN_WARNING "%s: unable to find sem %d "
+					"at server\n", __FUNCTION__, semid);
+			break;
+		}
+		if (svr_node == CLUSTERNODE_INVAL) {
+			sem_set_svr_node(semid, 0);
+			idelay(HZ/10);
+			continue;
+		}
+		SSI_ASSERT(svr_node);
+		break;
+	}
+	return svr_node;
+}
+
 static inline void __ssi_semexit(int semid, pid_t pid, struct sem_array *sma)
 {
-	int nsems, i;
 	struct sem_semundo *un, **unp;
+	int nsems, i;
 
-	if (sem_checkid (sma, semid)) {
-		printk ("semexit: stale undo sem %d for pid %d\n",
-			semid, pid);
-		goto next_entry;
+	if (sem_checkid(sma, semid)) {
+		/* OpenSSI 1941808 - semundo structures confused.
+		 * sem_checkid() failure is not a bug because in OpenSSI
+		 * tsk->sysvsem.undo_list (struct sem_undo) is separated from
+		 * sma->undo (struct sem_semundo).
+		 */
+		printk(KERN_INFO "%s: stale undo sem %d for pid %d\n",
+					__FUNCTION__, semid, pid);
+		goto out_unlock;
 	}
 
 	/* remove u from the sma->undo list */
@@ -1926,9 +1952,10 @@
 		if (pid == un->pid)
 			goto found;
 	}
-	printk (KERN_WARNING "semexit: missing undo sem %d for pid %d\n",
-		semid, pid);
-	goto next_entry;
+	printk(KERN_WARNING "%s: missing undo sem %d for pid %d\n",
+				__FUNCTION__, semid, pid);
+	goto out_unlock;
+
 found:
 	*unp = un->id_next;
 	/* perform adjustments registered in u */
@@ -1965,7 +1992,7 @@
 	   do it for us */
 	kfree (un);
 
-next_entry:
+out_unlock:
 	sem_unlock(sma);
 }
 #endif /* CONFIG_SSI */
@@ -1987,10 +2014,7 @@
 	struct sem_undo_list *undo_list;
 	struct sem_undo *u, **up;
 #ifdef CONFIG_SSI
-	ssi_procstate_t pstate;
-	key_t key;
-	clusternode_t svr_node=0;
-	int rval, flags, view, sz;
+	clusternode_t svr_node;
 #endif
 
 	undo_list = tsk->sysvsem.undo_list;
@@ -2005,41 +2029,30 @@
          * is the last task exiting for this undo list.
 	 */
 	for (up = &undo_list->proc_list; (u = *up); *up = u->proc_next, kfree(u)) {
-		struct sem_array *sma;
-		int semid = u->semid;
-		if(semid == -1)
+		if (u->semid == -1)
 			continue;
-namesvr_semexit_go:
-		svr_node = sem_get_svr_node(semid);
+
+		svr_node = sem_get_svr_node(u->semid);
 		if (!svr_node) {
-			if (cli_ipcname_findid(NAME_SERVICE_SEM, semid, &key,
-					&svr_node, &flags, &view, &sz)<0) {
-				sem_set_svr_node(semid, 0);
-			} else if (svr_node == CLUSTERNODE_INVAL) {
-				sem_set_svr_node(semid, 0);
-				idelay(HZ/10);
-				goto namesvr_semexit_go;
-			} else if (svr_node)
-				sem_set_svr_node(semid, svr_node);
-		}
-		if (svr_node && (svr_node != this_node)) {
-			sem_set_svr_node(semid, 0);
-			ssi_procstate_get(&pstate);
-			cli_ripc_semexit(svr_node, &rval, &pstate, semid, current->tgid);
-		} else {
-			sma = sem_lock(semid);
-			if (sma == NULL)
+			svr_node = sem_find_svr_node(u->semid);
+			if (!svr_node)
 				continue;
+		}
 
-			if (u->semid == -1) {
-				sem_unlock(sma);
-				continue;
-			}
+		sem_set_svr_node(u->semid, 0);
 
-			__ssi_semexit(u->semid, current->tgid, sma);
-		}
+		if (svr_node != this_node) {
+			ssi_procstate_t pstate;
+			int rval;
+
+			ssi_procstate_get(&pstate);
+			cli_ripc_semexit(svr_node, &rval, &pstate,
+						u->semid, current->tgid);
+			/* Ignoring -EREMOTE */
+		} else
+			ssi_semexit(u->semid, current->tgid);
 	}
-#else
+#else /* CONFIG_SSI */
 	/* There's no need to hold the semundo list lock, as current
          * is the last task exiting for this undo list.
 	 */
@@ -2103,7 +2116,7 @@
 next_entry:
 		sem_unlock(sma);
 	}
-#endif /* CONFIG_SSI */
+#endif /* !CONFIG_SSI */
 	kfree(undo_list);
 }
 
@@ -2262,15 +2275,18 @@
 void semundo_nodedown_thread(clusternode_t node)
 {
 	/* struct sem_queue *q; */
-	struct sem_semundo *un = NULL, **unp;
+	struct sem_semundo *un, **unp;
 	struct sem_array *sma;
 	int nsems, i, j;
+	pid_t pid = 0;
+	char pid_dead = 0;
+	char reset = 0; /* reset pid_checked marker */
 
 	extern int process_is_alive(pid_t);
 
 	down(&sem_ids.sem);
-	for (i=0; i<sem_ids.max_id; i++)
-	{
+again:
+	for (i=0; i < sem_ids.entries->size; i++) {
 		sma = sem_lock(i);
 		if (sma == NULL)
 			continue;
@@ -2278,29 +2294,73 @@
 			sem_unlock(sma);
 			continue;
 		}
-		for (unp = &sma->undo; (un = *unp); unp = &(un->id_next)) {
-			if (!process_is_alive(un->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;
-					}
-					sem->sempid = un->pid;
+		for (unp = &sma->undo; (un = *unp);) {
+			if (reset) {
+				un->pid_checked = 0;
+				unp = &un->id_next;
+				continue;
+			}
+			/* Check pid against all sem_semundo structures since
+			 * process_is_alive() is expensive.
+			 */
+			if (pid && un->pid != pid) {
+				unp = &un->id_next;
+				continue;
+			}
+			if (!un->pid_checked) {
+				un->pid_checked = 1;
+				if (!pid) {
+					pid = un->pid;
+					goto check_pid;
 				}
-				sma->sem_otime = get_seconds();
-				*unp = un->id_next;
-				kfree (un);
 			}
+			if (!pid_dead) {
+				unp = &un->id_next;
+				continue;
+			}
+
+			/* 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();
+
+			*unp = un->id_next;
+			kfree(un);
 		}
-		sem_unlock(sma);
 		update_queue(sma);
+		sem_unlock(sma);
+	}
+	if (pid) {
+		pid = 0;
+		pid_dead = 0;
+		goto again;
+	}
+	if (!reset) {
+		/* Done nodedown. 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;
 }
 #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