--IS0zKkzwUGydFO0o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline


Folks,

While playing with verified exec I found that there was an issue with
fileassoc_file_lookup() in that it wanted to malloc memory (via
vfs_fhcompose_alloc()) to hold a working copy of the file handle which
was then free'd when the work was done.  This means that you cannot
call fileassoc_file_lookup() when the vmlock is held which was causing
me some grief.  The diff below attempts to fix this issue by
allocating a couple of pools of storage that can be used as temporary
working storage for the filehandles.  One pool contains a number of
the minimum sized fhandle chunks, the other a lesser number of the
maximum sized fhandle chunks.  The idea is to try and keep to a
minimum the amount of memory we preallocate, an element from the small
pool is tried first, if the fhandle is too large to fit then the small
element is returned to its pool and storage from the large pool is
used.  This seems to work well for me and resolves the problems I was
having.  Anyone see any problems with this approach?

-- 
Brett Lymn

--IS0zKkzwUGydFO0o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=fileassoc_patch

Index: kern_fileassoc.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_fileassoc.c,v
retrieving revision 1.10
diff -u -r1.10 kern_fileassoc.c
--- kern_fileassoc.c	8 Sep 2006 13:57:38 -0000	1.10
+++ kern_fileassoc.c	30 Sep 2006 10:12:29 -0000
@@ -46,11 +46,19 @@
 #include <sys/fileassoc.h>
 #include <sys/hash.h>
 #include <sys/fstypes.h>
+#include <sys/pool.h>
 
 static struct fileassoc_hash_entry *
 fileassoc_file_lookup(struct vnode *, fhandle_t *);
 static struct fileassoc_hash_entry *
 fileassoc_file_add(struct vnode *, fhandle_t *);
+static int
+fileassoc_getfh_pool(struct pool *, size_t, struct vnode *,
+		     fhandle_t **);
+static int
+fileassoc_getfh(struct vnode *, fhandle_t **, struct pool **);
+static void
+fileassoc_fh_free(fhandle_t *, struct pool *);
 
 /*
  * Hook entry.
@@ -81,6 +89,18 @@
 
 struct fileassoc_hook fileassoc_hooks[FILEASSOC_NHOOKS];
 int fileassoc_nhooks;
+struct pool fileassoc_small_pool; /* FHANDLE_SIZE_MIN fhandle struct pool */
+struct pool fileassoc_large_pool; /* FHANDLE_SIZE_MAX fhandle struct pool */
+
+/* Set the number of fhandle structs to put into the small pool */
+#ifndef FILEASSOC_SMALL_POOL
+#define FILEASSOC_SMALL_POOL 16
+#endif
+
+/* Set the number of fhandle structs to put into the large pool */
+#ifndef FILEASSOC_LARGE_POOL
+#define FILEASSOC_LARGE_POOL 8
+#endif
 
 /* Global list of hash tables, one per device. */
 LIST_HEAD(, fileassoc_table) fileassoc_tables;
@@ -101,6 +121,15 @@
 {
 	memset(fileassoc_hooks, 0, sizeof(fileassoc_hooks));
 	fileassoc_nhooks = 0;
+	pool_init(&fileassoc_small_pool, FHANDLE_SIZE_MIN, 0, 0, 0,
+		  "fileassoc_small_pool", NULL);
+	if (pool_prime(&fileassoc_small_pool, FILEASSOC_SMALL_POOL) != 0)
+		panic("fileassoc_init: small pool prime failed");
+
+	pool_init(&fileassoc_large_pool, FHANDLE_SIZE_MAX, 0, 0, 0,
+		  "fileassoc_small_pool", NULL);
+	if (pool_prime(&fileassoc_large_pool, FILEASSOC_LARGE_POOL) != 0)
+		panic("fileassoc_init: large pool prime failed");
 }
 
 /*
@@ -144,6 +173,76 @@
 }
 
 /*
+ * Try to get a fhandle_t from the given pool.
+ */
+static int
+fileassoc_getfh_pool(struct pool *pool, size_t fhs, struct vnode *vp,
+		fhandle_t **fh)
+{
+	int error, s;
+	size_t fh_size;
+
+	/*
+	 * Try to get a fhandle from the pool - if the get succeeds
+	 * and the call to vfs_composefh() is ok with the size then we
+	 * return the fhandle.  Otherwise we return the fhandle to
+	 * the pool (if the get succeeded) and error out.
+	 */
+	fh_size = fhs;
+	s = splhigh();
+	*fh = pool_get(pool, PR_NOWAIT);
+	splx(s);
+	if (fh != NULL) {
+		error = vfs_composefh(vp, *fh, &fh_size);
+		if (error == 0)
+			return 0;
+
+		s = splhigh();
+		pool_put(pool, *fh);
+		splx(s);
+		return error;
+	}
+
+	return ENOMEM;
+}
+
+/*
+ * Get a fhandle_t structure from the pools, fetch from the small
+ * pool first, if that fails for any reason try the large pool,
+ * hopefully that will work.
+ */
+static int
+fileassoc_getfh(struct vnode *vp, fhandle_t **fh, struct pool **pool)
+{
+	int error;
+
+	*pool = &fileassoc_small_pool;
+	error = fileassoc_getfh_pool(*pool, FHANDLE_SIZE_MIN, vp, fh);
+	if (error != 0) {
+		/*
+		 * Try again with the large pool.
+		 */
+		*pool = &fileassoc_large_pool;
+		error = fileassoc_getfh_pool(*pool, FHANDLE_SIZE_MAX, vp, fh);
+	}
+
+	return error;
+}
+
+/*
+ * Release the given file handle back into the pool.
+ */
+static void
+fileassoc_fh_free(fhandle_t *fh, struct pool *pool)
+{
+	int s;
+
+	s = splhigh();
+	pool_put(pool, fh);
+	splx(s);
+}
+
+/*
  * Get the hash table for the specified device.
  */
 static struct fileassoc_table *
@@ -172,10 +271,12 @@
 	struct fileassoc_hash_entry *e;
 	size_t indx;
 	fhandle_t *th;
+	struct pool *pool;
 	int error;
 
+	pool = NULL;
 	if (hint == NULL) {
-		error = vfs_composefh_alloc(vp, &th);
+		error = fileassoc_getfh(vp, &th, &pool);
 		if (error)
 			return (NULL);
 	} else
@@ -184,7 +285,7 @@
 	tbl = fileassoc_table_lookup(vp->v_mount);
 	if (tbl == NULL) {
 		if (hint == NULL)
-			vfs_composefh_free(th);
+			fileassoc_fh_free(th, pool);
 
 		return (NULL);
 	}
@@ -202,7 +303,7 @@
 	}
 
 	if (hint == NULL)
-		vfs_composefh_free(th);
+		fileassoc_fh_free(th, pool);
 
 	return (NULL);
 }

--IS0zKkzwUGydFO0o--