[SSI] openssi/kernel/cluster/ssi/ipc ipcshm_svr.c, 1.16, 1.17 namesvr_func.c, 1.14, 1.15

Roger Tsang <[email protected]>
Newsgroups gmane.linux.cluster.ssic.cvs
Message-ID <[email protected]>
Update of /cvsroot/ssic-linux/openssi/kernel/cluster/ssi/ipc
In directory fdv4jf1.ch3.sourceforge.com:/tmp/cvs-serv32639/cluster/ssi/ipc

Modified Files:
      Tag: OPENSSI-FC
	ipcshm_svr.c namesvr_func.c 
Log Message:
IPC:
- Fix shm_ids_svr table race; mutex not held, wrong mutex held, or releasing unheld mutex. (#ifdef IPC_SHM_RACE_FIX)
- Downgrade NSC_IPC_WRLOCK() where appropriate; reduce contention. (#ifdef NSC_IPC_RWLOCK_DOWNGRADE)

 cluster/ssi/cfs/cfs_ipcshm.c   |   66 +++++++++++++++++++++++++++++------
 cluster/ssi/ipc/ipcshm_svr.c   |   76 ++++++++++++++++++++++++++++++++++-------
 cluster/ssi/ipc/namesvr_func.c |   68 ++++++++++++++++++++++++++++++++++++
 include/linux/config.h         |    1 
 ipc/shm.c                      |    3 +
 5 files changed, 188 insertions(+), 26 deletions(-)


Index: ipcshm_svr.c
===================================================================
RCS file: /cvsroot/ssic-linux/openssi/kernel/cluster/ssi/ipc/ipcshm_svr.c,v
retrieving revision 1.16
retrieving revision 1.17
diff -u -d -r1.16 -r1.17
--- ipcshm_svr.c	10 Oct 2008 08:10:32 -0000	1.16
+++ ipcshm_svr.c	24 Feb 2009 01:51:47 -0000	1.17
@@ -177,6 +177,7 @@
 	else {
 		shp->shm_perm.mode |= SHM_LOCK_DEST;
 		ipc_drop_locks(shmid, (struct kern_ipc_perm *)shp, &shm_ids, 0);
+		/* shm_ids.sem held */
 	}
 		
         return 0;
@@ -209,6 +210,9 @@
 do_ssi_shm_noclients(int id, clusternode_t svrnode, int dest)
 {
 	struct shmid_kernel_svr *svp;
+#ifdef IPC_SHM_RACE_FIX
+	nsc_nodelist_t *nl;
+#endif
 	clusternode_t node;
 	int ret;
 	nsc_nlcookie_t cookie;
@@ -236,14 +240,25 @@
 
 try_again:
 	cnt = nm_svr_num;
+#ifdef IPC_SHM_RACE_FIX
+	ipc_get_locks(0, &shm_ids_svr, 1);
+#endif
 	svp = (struct shmid_kernel_svr *)shm_svr_get(id);
-	if (!svp) {
+	if (!svp)
 		return -EIDRM;
-	}
 
+#ifdef IPC_SHM_RACE_FIX
+	nl = NSC_NODELIST_COPY(svp->shm_nodelist);
+	ipc_drop_locks(0, NULL, &shm_ids_svr, 1);
+#endif
 	cookie = CLUSTERNODE_INVAL;
+#ifdef IPC_SHM_RACE_FIX
+	while ((node = NSC_NODELIST_GET_NEXT(&cookie, nl))
+							!= CLUSTERNODE_INVAL) {
+#else
 	while ((node = NSC_NODELIST_GET_NEXT(&cookie, svp->shm_nodelist))
 							!= CLUSTERNODE_INVAL) {
+#endif
 		if (node == this_node)
 			continue;
 
@@ -288,14 +303,24 @@
 	nsc_nodelist_t *nl;
 	nsc_nlcookie_t cookie;
 
+#ifdef IPC_SHM_RACE_FIX
+	ipc_get_locks(0, &shm_ids_svr, 1);
+#endif
 	svr = (struct shmid_kernel_svr *)shm_svr_get(shp->id);
-	if (!svr)
+	if (!svr) {
+#ifdef IPC_SHM_RACE_FIX
+		ipc_drop_locks(0, NULL, &shm_ids_svr, 1);
+#endif
 		return;
+	}
 
 	/* Tell namesvr that shp is being destroyed */
 
 	/* Tell clients that shp is being destroyed */
 	nl = NSC_NODELIST_COPY(svr->shm_nodelist);
+#ifdef IPC_SHM_RACE_FIX
+	ipc_drop_locks(0, NULL, &shm_ids_svr, 1);
+#endif
 	cookie = CLUSTERNODE_INVAL;
 	while ((node = NSC_NODELIST_GET_NEXT(&cookie, nl))
 							!= CLUSTERNODE_INVAL) {
@@ -321,9 +346,15 @@
 	struct shmid_kernel_svr *shp;
 
 	*rval = cfs_shm_notify(cfs_shm_sb, clinode, 0);
+#ifdef IPC_SHM_RACE_FIX
+	ipc_get_locks(0, &shm_ids_svr, 1);
+#endif
 	shp = (struct shmid_kernel_svr *)shm_svr_get(id);
 	*key = shp->shm_perm.key;
 	*size = shp->shm_segsz;
+#ifdef IPC_SHM_RACE_FIX
+	ipc_drop_locks(0, NULL, &shm_ids_svr, 1);
+#endif
 	return 0;
 }
 
@@ -337,20 +368,35 @@
 {
 	struct shmid_kernel_svr *shmp;
 
+#ifdef IPC_SHM_RACE_FIX
+	ipc_get_locks(0, &shm_ids_svr, 1);
+#else
 	ipc_get_locks(0, &shm_ids, 1);
+#endif
 	shmp = (struct shmid_kernel_svr *)shm_svr_get(id);
 	if (shmp == NULL) {
+#ifdef IPC_SHM_RACE_FIX
+		ipc_drop_locks(0, NULL, &shm_ids_svr, 1);
+#else
 		ipc_drop_locks(0, NULL, &shm_ids, 1);
+#endif
 		*rval = -EINVAL;
 		return 0;
 	}
 	NSC_NODELIST_SET1(shmp->shm_nodelist, clinode);
 	*size = shmp->shm_segsz;
-	*rval = 0;
+#ifdef IPC_SHM_RACE_FIX
+	ipc_drop_locks(0, NULL, &shm_ids_svr, 1);
+#else
 	ipc_drop_locks(0, NULL, &shm_ids, 1);
+#endif
+	*rval = 0;
 	return 0;
 }
 
+/* Caller must assure shm_ids.sem is held.
+ * Returns with shm_ids.sem unheld.
+ */
 int
 ripc_shm_rmid(
 	clusternode_t node,
@@ -363,7 +409,7 @@
 
 	shp = (struct shmid_kernel *)ipc_get_locks(id, &shm_ids, 0);
 	if (!shp) {
-		/* Drop the locks aqcuired above */
+		/* Drop the locks acquired by caller */
 		ipc_drop_locks(0, NULL, &shm_ids, 1);
 		*rval = -EIDRM;
 		return 0;
@@ -380,7 +426,7 @@
 	shm_rmid(id);
 
 	shp->shm_perm.mode &= ~SHM_LOCK_DEST; 
-	/* Drop the locks aqcuired above */
+	/* Drop the locks acquired above */
 	ipc_drop_locks(id, (struct kern_ipc_perm *)shp, &shm_ids, 1);
 
 	fput(shp->shm_file);
@@ -395,12 +441,17 @@
 {
 	struct shmid_kernel_svr *svp;
 
+#ifdef IPC_SHM_RACE_FIX
+	ipc_get_locks(0, &shm_ids_svr, 1);
+#endif
 	svp = (struct shmid_kernel_svr *)shm_svr_get(id);
-	if (!svp)
-		return;
-
-	if (NSC_NODELIST_TEST1(svp->shm_nodelist, node))
-		NSC_NODELIST_CLR1(svp->shm_nodelist, node);
+	if (svp) {
+		if (NSC_NODELIST_TEST1(svp->shm_nodelist, node))
+			NSC_NODELIST_CLR1(svp->shm_nodelist, node);
+	}
+#ifdef IPC_SHM_RACE_FIX
+	ipc_drop_locks(0, NULL, &shm_ids_svr, 1);
+#endif
 }
 
 int
@@ -461,6 +512,7 @@
 	return 0;
 }
 
+/* Caller holds shm_ids.sem */
 int
 ssi_shm_cleanup(clusternode_t svrnode, int id, clusternode_t clinode)
 {
@@ -480,7 +532,7 @@
 			ipc_drop_locks(id, (struct kern_ipc_perm *)shp,
 					&shm_ids, 0);
 		}
-		else
+		else /* caller is remote */
 			ipc_get_locks(0, &shm_ids, 1);
 
 		ret = do_ssi_shm_noclients(id, svrnode, 2);

Index: namesvr_func.c
===================================================================
RCS file: /cvsroot/ssic-linux/openssi/kernel/cluster/ssi/ipc/namesvr_func.c,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -d -r1.14 -r1.15
--- namesvr_func.c	19 Feb 2009 08:01:02 -0000	1.14
+++ namesvr_func.c	24 Feb 2009 01:51:47 -0000	1.15
@@ -72,6 +72,10 @@
 extern struct ipc_ids msg_ids;
 extern struct ipc_ids sem_ids;
 extern struct ipc_ids shm_ids;
+#ifdef IPC_SHM_RACE_FIX
+extern void ipc_drop_locks(int, struct kern_ipc_perm *, struct ipc_ids *, int);
+extern struct kern_ipc_perm * ipc_get_locks(int, struct ipc_ids *, int);
+#endif
 extern struct shmid_kernel_svr *shm_svr_get(int);
 extern int ipc_rebuildid(struct ipc_ids *, int, struct kern_ipc_perm *);
 extern int ipc_get_inuse(struct ipc_ids *);
@@ -490,11 +494,18 @@
 	ipc_obj_db_t	*odbp;
 	ipc_obj_t	*objp;
 	int		error = 0;
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+	int		wrlock = 0;
+#endif
 	extern ATOMIC_INT_T local_nameserver_version;
 
 
 	odbp = &nsc_name_odb[service];
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+	NSC_IPC_RDLOCK(odbp);
+#else
 	NSC_IPC_WRLOCK(odbp);
+#endif
 #ifdef TEST_IPC
 	printk("%s\n","Message nameserver locked in getid");
 #endif
@@ -531,7 +542,12 @@
 		goto out;
 	}
 
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+	NSC_IPC_RDUNLOCK(odbp);
+	wrlock = 1;
+#else
 	NSC_IPC_WRUNLOCK(odbp);
+#endif
 
 	error = objsvr_new(odbp, &objp, key, *glid, in_flag, *server, view,
 			   READ_ATOMIC_INT(&local_nameserver_version));
@@ -567,6 +583,11 @@
 #endif
 	}
 out:
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+	if (!wrlock)
+		NSC_IPC_RDUNLOCK(odbp);
+	else
+#endif
 	NSC_IPC_WRUNLOCK(odbp);
 #ifdef TEST_IPC
 	printk("%s\n","name server spin lock dropped in out/getid");
@@ -584,7 +605,11 @@
 	int		idx=0, count;
 
 	odbp = &nsc_name_odb[service];
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+	NSC_IPC_RDLOCK(odbp);
+#else
 	NSC_IPC_WRLOCK(odbp);
+#endif
 #ifdef TEST_IPC
 	printk("%s\n","Message nameserver locked in getid");
 #endif
@@ -610,8 +635,13 @@
 		buf += sizeof(int);
 	}
 done:
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+	NSC_IPC_RDUNLOCK(odbp);
+	*size = idx;
+#else
 	*size = idx;
 	NSC_IPC_WRUNLOCK(odbp);
+#endif
 #ifdef TEST_IPC
 	printk("%s\n","name server spin lock dropped in out/getid");
 #endif
@@ -656,12 +686,20 @@
 	int error = 0;
 
 	odbp = &nsc_name_odb[service];
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+	NSC_IPC_RDLOCK(odbp);
+#else
 	NSC_IPC_WRLOCK(odbp);
+#endif
 	/*
 	 * lookup if this id exist
 	 */
 	if ((objp = objsvr_find_id(odbp, glid)) == NULL) {
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+		NSC_IPC_RDUNLOCK(odbp);
+#else
 		NSC_IPC_WRUNLOCK(odbp);
+#endif
 		return -EINVAL;
 	}
 	*key = objp->io_perm->key;
@@ -673,7 +711,11 @@
 	*view = objp->local_view;
 	if (service == NAME_SERVICE_SHM)
 		*sz = objp->io_size;
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+	NSC_IPC_RDUNLOCK(odbp);
+#else
 	NSC_IPC_WRUNLOCK(odbp);
+#endif
 	return error;
 }
 
@@ -731,6 +773,9 @@
 	 * package id/key pairs into one buffer for the new name server.
 	 */
 	for (i = 0; i < NAME_SERVICE_MAX; i++) {
+#ifdef IPC_SHM_RACE_FIX
+		ipc_get_locks(0, ipcname_svc_dbs[i], 1);
+#endif
 		num_objects[i] = ipc_get_inuse(ipcname_svc_dbs[i]);
 	}
 	printk("passed the first scan in ipcname_pull_data\n");
@@ -760,8 +805,12 @@
 		int id, maxid;
 		*((int *)buf) = num_objects[i];
 		buf += sizeof(int);
-		if (num_objects[i] == 0)
+		if (num_objects[i] == 0) {
+#ifdef IPC_SHM_RACE_FIX
+			ipc_drop_locks(0, NULL, ipcname_svc_dbs[i], 1);
+#endif
 			continue;
+		}
 		cur_object = 0;
 		maxid = ipc_get_maxid(ipcname_svc_dbs[i]);
 		for (id = 0; id <= maxid; id++) {
@@ -803,6 +852,9 @@
 			if (++cur_object == num_objects[i])
 				break;
 		}
+#ifdef IPC_SHM_RACE_FIX
+		ipc_drop_locks(0, NULL, ipcname_svc_dbs[i], 1);
+#endif
 	}
 
 	return 0;
@@ -888,14 +940,28 @@
 
 	for (service = 0; service < NAME_SERVICE_MAX; service++) {
 		odbp = &nsc_name_odb[service];
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+again:
+#endif
 		NSC_IPC_RDLOCK(odbp);
 
+#ifndef NSC_IPC_RWLOCK_DOWNGRADE
 again:
+#endif
 		idx = 0;
 		while (idx<odbp->iodb_size) {
 			if ((op=odbp->iodb_active[idx])!= NULL) {
 				if (op->svr_node == node) {
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+					NSC_IPC_RDUNLOCK(odbp);
+					NSC_IPC_WRLOCK(odbp);
+					/* re-check */
+					if (op && op->svr_node == node)
+#endif
 					(void)nsc_ipcremove(odbp, op->io_id);
+#ifdef NSC_IPC_RWLOCK_DOWNGRADE
+					NSC_IPC_WRUNLOCK(odbp);
+#endif
 					goto again;
 				}
 			}


------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.