On Wed, 24 Aug 2011, Rick Macklem wrote:

"afs"

The current OpenAFS codebase uses the all-caps "AFS". Judging by the omitted text, perhaps this should change. (We also don't use VFS_SET to set it, which I filed a bug about.)


and here is my current rendition of the patch. (I took Gleb's suggestion
and switched to fnv_32_str(). I'll leave it that way unless there is a
collision after adding any names people post to the above list.)

It sounds like people have agreed that this is a reasonable solution.
If hrs@ can confirm that testing shows it fixes the original problem
(the ZFS file handles don't change when it's loaded at different times),
I'll pass it along to re@.

Thanks for the helpful suggestions, rick

--- kern/vfs_init.c.sav 2011-06-11 18:58:33.000000000 -0400
+++ kern/vfs_init.c     2011-08-24 16:15:24.000000000 -0400
@@ -39,6 +39,7 @@ __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>
@@ -138,6 +139,8 @@ vfs_register(struct vfsconf *vfc)
        struct sysctl_oid *oidp;
        struct vfsops *vfsops;
        static int once;
+       struct vfsconf *tvfc;
+       uint32_t hashval;

        if (!once) {
                vattr_null(&va_null);
@@ -152,7 +155,26 @@ 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
+        * a collision occurs, it is limited to 8bits since that is
+        * what ZFS uses from vfc_typenum and that also limits how sparsely
+        * distributed vfc_typenum becomes.
+        */
+       hashval = fnv_32_str(vfc->vfc_name, FNV1_32_INIT);
+       hashval &= 0xff;
+       do {
+               /* Look for and fix any collision. */
+               TAILQ_FOREACH(tvfc, &vfsconf, vfc_list) {
+                       if (hashval == tvfc->vfc_typenum) {
+                               hashval++; /* Can exceed 8bits, if needed. */

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.

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.

+                               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?

-Ben

        TAILQ_INSERT_TAIL(&vfsconf, vfc, vfc_list);

        /*



_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[email protected]"

Reply via email to