[SSI] openssi/kernel/cluster/ssi/util rmtfb.c,1.33,1.34

Update of /cvsroot/ssic-linux/openssi/kernel/cluster/ssi/util
In directory sfp-cvsdas-3.v30.ch3.sourceforge.com:/tmp/cvs-serv27177/cluster/ssi/util

Modified Files:
      Tag: OPENSSI-FC
	rmtfb.c 
Log Message:
- rmtfb_newcli: fix file struct leak if lost race with another rmtfb_newcli().
- rmtfb_newcli: search for existing rmtfb_cli structure early. avoid memory allocation. avoid possible RPC.
- rmtfb_newcli: add comment about this function.

(#ifdef RMTFB_HASH_LOCKLESS)
- rmtfb_getcli_id: use yield() instead of idelay().
- rmtfb_getcli_id_lckd: rename function to __rmtfb_getcli_id. it is lockless.
- rmtfb_newcli: use yield() instead of idelay().

(#ifdef RMTFB_REFCNT_FIX)
- rmtfb_newcli: skip rmtfb_clrcli(). fix -ERFB_TRYAGAIN errors due to reference count off by one after we lost the race with another thread in rmtfb_newcli(). rmtfb_clrcli() is called to undo rmtfb_setcli_reopen(). the undo caused the reference count for rmtfb_svr structure to be off by one.
- rmtfb_rclose: no need to NULL out private_data field in file structure. rmtfb_close() is called from __fput() and constructor zero's all fields in file structure.


Index: rmtfb.c
===================================================================
RCS file: /cvsroot/ssic-linux/openssi/kernel/cluster/ssi/util/rmtfb.c,v
retrieving revision 1.33
retrieving revision 1.34
diff -u -d -r1.33 -r1.34
--- rmtfb.c	17 Jan 2011 06:19:45 -0000	1.33
+++ rmtfb.c	17 Jan 2011 06:33:53 -0000	1.34
@@ -962,12 +962,12 @@
 #endif /* !RMTFB_REFCNT_FIX */
 }
 
+#ifdef RMTFB_HASH_LOCKLESS
 static struct rmtfb_cli *
-rmtfb_getcli_id_lckd(unsigned long id)
+__rmtfb_getcli_id(unsigned long id)
 {
 	struct rmtfb_cli *rfb;
 
-#ifdef RMTFB_HASH_LOCKLESS
 	rfb = (struct rmtfb_cli *) rmtfb_getcmn(
 					&rmtfb_clitbl[rfbhash(id)],
 					rmtfb_test_id,
@@ -982,7 +982,13 @@
 		return ERR_PTR(-ERFB_RELEASE);
 	}
 	return rfb;
+}
 #else /* RMTFB_HASH_LOCKLESS */
+static struct rmtfb_cli *
+rmtfb_getcli_id_lckd(unsigned long id)
+{
+	struct rmtfb_cli *rfb;
+
 	SSI_ASSERT_LOCKED_SHR_RW_LOCK(&rmtfb_clitbl_lock);
 
 	rfb = (struct rmtfb_cli *) rmtfb_getcmn(
@@ -1006,8 +1012,8 @@
 		SSI_ASSERT(!rfb || atomic_read(&rfb->common.rfb_file->f_count));
 #endif
 	return rfb;
-#endif /* !RMTFB_HASH_LOCKLESS */
 }
+#endif /* !RMTFB_HASH_LOCKLESS */
 
 /* does an implicit get_file(cli->common.rfb_file) */
 #ifdef SSI_SOCK_REOP_TYPE
@@ -1053,7 +1059,7 @@
 	loop = 0;
 #ifdef RMTFB_HASH_LOCKLESS
 	for (;;) {
-		rfb = rmtfb_getcli_id_lckd(id);
+		rfb = __rmtfb_getcli_id(id);
 		if (!rfb)
 			break;
 		if (PTR_ERR(rfb) != -ERFB_RELEASE)
@@ -1067,7 +1073,7 @@
 			printk(KERN_INFO "rmtfb_getcli_id: looping\n");
 			loop = 0;
 		}
-		idelay(HZ/50);
+		yield();
 	}
 #else /* RMTFB_HASH_LOCKLESS */
 repeat:
@@ -1387,6 +1393,19 @@
 #endif /* REOP_EXPORT_PATH_SVRNODE */
 
 #ifdef RMTFB_HASH_LOCKLESS
+/**
+ * rmtfb_newcli - returns pointer to new "or existing" rmtfb_cli structure
+ * with matching rfb_id.
+ *
+ * @file - file struct to be referenced by new rmtfb_cli structure.
+ * @id - value of rfb_id field in rmtfb_cli structure. 0: generate one.
+ * @svr - OpenSSI node number where the "real" inode resides.
+ * @noremote - boolean. 1: skip rmtfb_svr structure setup (only because there
+ * is one already setup for this OpenSSI node).
+ *
+ * NOTE! fput() is called on @file before returning to caller if there is an
+ * existing rmtfb_cli structure with the same rfb_id.
+ */
 struct rmtfb_cli *
 rmtfb_newcli(struct file *file, unsigned long id,
 	     clusternode_t svr, char noremote)
@@ -1395,9 +1414,18 @@
 	struct rmtfb_cli *rfb;
 	unsigned long hash;
 	int error;
-	char import = !!id;
+	char import;
 
-	SSI_ASSERT(svr && !inval(1,svr));
+	if (id) {
+		/* Fast check: see if another import didn't make one already */
+		rfb = __rmtfb_getcli_id(id);
+		if (rfb && !IS_ERR(rfb)) {
+			/* BUG: ssi_sockfs_make_cli() expects no race */
+			BUG_ON(noremote);
+			fput(file); /* rfb->common.rfb_file != file */
+			return rfb;
+		}
+	}
 
 	if (unlikely(file->private_data)) {
 		printk(KERN_ERR "%s: non-exportable filesystem (%s)\n",
@@ -1406,11 +1434,14 @@
 		goto out;
 	}
 
+	SSI_ASSERT(svr && !inval(1,svr));
+
 	rfb = rmtfb_cli_alloc(file, id, svr);
 	if (!rfb) {
 		error = -ENOMEM;
 		goto out;
 	}
+	import = !!id;
 	if (!id)
 		id = rfb->common.rfb_id;
 
@@ -1437,27 +1468,32 @@
 
 		/* Make sure another import didn't make one already */
 		for (;;) {
-			old = rmtfb_getcli_id_lckd(id);
+			old = __rmtfb_getcli_id(id);
 			if (!old || PTR_ERR(old) != -ERFB_RELEASE)
 				break;
 			up(&rmtfb_newcli_lock);
-
-			idelay(HZ/50);
+			/* Avoids a race where clrcli and setcli could
+			 * be received out of order at the server,
+			 * leaving it unaware that this node is its
+			 * client again.
+			 */
 			if (++loop > rmtfb_maxloops) {
 				printk(KERN_INFO "rmtfb_newcli: looping\n");
 				loop = 0;
 			}
+			yield();
 			down(&rmtfb_newcli_lock);
 		}
-		if (old) {
+		if (unlikely(old)) {
 			up(&rmtfb_newcli_lock);
-#ifdef RMTFB_REFCNT_FIX
-			/* Undo rmtfb_setcli_re/open() */
-			if (!noremote)
-				(void) rmtfb_clrcli(rfb);
-#endif
+			/* NB: No need to undo rmtfb_setcli_re/open() */
 			rmtfb_freecli(rfb);
 			rfb = old;
+			if (!IS_ERR(rfb)) {
+				/* BUG: ssi_sockfs_make_cli() expects no race */
+				BUG_ON(noremote);
+				fput(file); /* rfb->common.rfb_file != file */
+			}
 			goto out;
 		}
 	}
@@ -1672,7 +1708,6 @@
 #endif /* RFBDEBUG */
 
 #ifdef RMTFB_REFCNT_FIX
-	file->private_data = NULL;
 	rmtfb_putcli(rfb); /* Release file->private_data */
 #else
 	LOCK_EXCL_RW_LOCK(&rmtfb_clitbl_lock);


------------------------------------------------------------------------------
Protect Your Site and Customers from Malware Attacks
Learn about various malware tactics and how to avoid them. Understand 
malware threats, the impact they can have on your business, and how you 
can protect your company and customers by using code signing.
http://p.sf.net/sfu/oracle-sfdevnl