Re: fsid change of ZFS?

Benjamin Kaduk wrote:
> 
> If we're confident that we won't ever fully fill the hash table, I
> would
> think that this should wrap around back to zero (or one?) instead of
> overflowing.
> 
Here's my updated patch (it will wrap to 1 the first time and then
exceed 255 if 1<->255 are all in use).
--- kern/vfs_init.c.sav	2011-06-11 18:58:33.000000000 -0400
+++ kern/vfs_init.c	2011-08-25 11:09:14.000000000 -0400
_at__at_ -39,6 +39,7 _at__at_ __FBSDID("$FreeBSD: head/sys/kern/vfs_in
 
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/fnv_hash.h>
 #include <sys/kernel.h>
 #include <sys/linker.h>
 #include <sys/mount.h>
_at__at_ -138,6 +139,9 _at__at_ vfs_register(struct vfsconf *vfc)
 	struct sysctl_oid *oidp;
 	struct vfsops *vfsops;
 	static int once;
+	struct vfsconf *tvfc;
+	uint32_t hashval;
+	int secondpass;
 
 	if (!once) {
 		vattr_null(&va_null);
_at__at_ -152,7 +156,31 _at__at_ vfs_register(struct vfsconf *vfc)
 	if (vfs_byname(vfc->vfc_name) != NULL)
 		return EEXIST;
 
-	vfc->vfc_typenum = maxvfsconf++;
+	/*
+	 * Calculate a hash on vfc_name to use for vfc_typenum. Unless
+	 * all of 1<->255 are assigned, it is limited to 8bits since that is
+	 * what ZFS uses from vfc_typenum and is also the preferred range
+	 * for vfs_getnewfsid().
+	 */
+	hashval = fnv_32_str(vfc->vfc_name, FNV1_32_INIT);
+	hashval &= 0xff;
+	secondpass = 0;
+	do {
+		/* Look for and fix any collision. */
+		TAILQ_FOREACH(tvfc, &vfsconf, vfc_list) {
+			if (hashval == tvfc->vfc_typenum) {
+				if (hashval == 255 && secondpass == 0) {
+					hashval = 1;
+					secondpass = 1;
+				} else
+					hashval++;
+				break;
+			}
+		}
+	} while (tvfc != NULL);
+	vfc->vfc_typenum = hashval;
+	if (vfc->vfc_typenum >= maxvfsconf)
+		maxvfsconf = vfc->vfc_typenum + 1;
 	TAILQ_INSERT_TAIL(&vfsconf, vfc, vfc_list);
 
 	/*

> Do we need to care about something attempting to add the same vfc_name
> twice? This code will happily add a second entry at the next available
> index.
> 
If file systems use VFS_SET(), I don't think this can happen, since the
same vfc_name would imply "same module name" and the 2nd one wouldn't load.
(Been there, w.r.t. nfs.)

> > + break;
> > + }
> > + }
> > + } while (tvfc != NULL);
> > + vfc->vfc_typenum = hashval;
> > + if (vfc->vfc_typenum >= maxvfsconf)
> > + maxvfsconf = vfc->vfc_typenum + 1;
> 
> I guess we're holding off on killing maxvfsconf until after 9.0 is
> out?

Well, I still don't know if anything has a use for vfs_sysctl(), so
I'm not volunteering to take it out. (If others feel it should come
out for 9.0, maybe... But I would still consider that a separate patch.)

rick