Re: fsid change of ZFS?

Benjamin Kaduk wrote:
> 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_at_ 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_at_.
> >
> > 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
> > _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,8 _at__at_ 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);
> > _at__at_ -152,7 +155,26 _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
> > + * 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.
> 
I did it that way so that it could overflow for the highly unlikely case
of more than 255 file system types.

I suppose I should scan for an unused value 1<->255 and only allow it to
go past 255 if all of them are used.

I'll post yet another patch soon, rick

> 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);
> >
> > 	/*
> >
> >
> >
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to
> "freebsd-current-unsubscribe_at_freebsd.org"