Date: Wed, 24 Aug 2011 17:17:17 -0400 (EDT) From: Benjamin Kaduk <kaduk@MIT.EDU> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: current@FreeBSD.org Subject: Re: fsid change of ZFS? Message-ID: <alpine.GSO.1.10.1108241711320.7526@multics.mit.edu> In-Reply-To: <468764384.310026.1314219682612.JavaMail.root@erie.cs.uoguelph.ca> References: <468764384.310026.1314219682612.JavaMail.root@erie.cs.uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
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);
>
> /*
>
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.GSO.1.10.1108241711320.7526>