Re: fsid change of ZFS?

Rick Macklem <rmacklem_at_uoguelph.ca> wrote
  in <920337541.272757.1314192294772.JavaMail.root_at_erie.cs.uoguelph.ca>:

rm> Kostik Belousov wrote:
rm> > On Tue, Aug 23, 2011 at 11:23:03PM +0200, Pawel Jakub Dawidek wrote:
rm> > > On Tue, Aug 23, 2011 at 04:11:20PM -0400, Rick Macklem wrote:
rm> > > > Pawel Jakub Dawidek wrote:
rm> > > > > On Tue, Aug 23, 2011 at 10:09:41AM -0400, Rick Macklem wrote:
rm> > > > > > Ok, I'll admit I wasn't very fond of a fixed table that would
rm> > > > > > inevitably
rm> > > > > > get out of date someday, either.
rm> > > > > >
rm> > > > > > I didn't think hashing for the cases not in the table was
rm> > > > > > worth the
rm> > > > > > effort,
rm> > > > > > but doing a hash instead of a table seems reasonable.
rm> > > > > >
rm> > > > > > I see that ZFS only uses the low order 8 bits, so I'll try and
rm> > > > > > come
rm> > > > > > up
rm> > > > > > with an 8bit hash solution and will post a patch for
rm> > > > > > testing/review
rm> > > > > > soon.
rm> > > > > >
rm> > > > > > I don't think the vfs_sysctl() is that great a concern, given
rm> > > > > > that
rm> > > > > > it
rm> > > > > > appears to be deprecated already anyhow. (With an 8bit hash,
rm> > > > > > vfs_typenum
rm> > > > > > won't be that sparse.) I'll also make sure that whatever hash
rm> > > > > > I use
rm> > > > > > doesn't collide for the current list of file names (although I
rm> > > > > > will
rm> > > > > > include
rm> > > > > > code that handles a collision in the patch).
rm> > > > >
rm> > > > > Sounds great. Thanks!
rm> > > > >
rm> > > > Here's the patch. (Hiroki could you please test this, thanks,
rm> > > > rick.)
rm> > > > ps: If the white space gets trashed, the same patch is at:
rm> > > >    http://people.freebsd.org/~rmacklem/fsid.patch
rm> > >
rm> > > The patch is fine by me. Thanks, Rick!
rm> >
rm> > Sorry, I am late.
rm> >
rm> > It seems that the probability of the collisions for the hash is quite
rm> > high.
rm> > Due to the fixup procedure, the resulting typenum will depend on the
rm> > order
rm> > of the module initialization, isn't it ? IMO, it makes the patch goal
rm> > not
rm> > met.
rm> Well, the about 15 file system types currently in FreeBSD don't collide.
rm> (At least with the patch I posted. I'll test it again with "hash & 0xff;".)
rm>
rm> Also, since the collision results in the second one using the next higher
rm> value (usually, until you get something like 50+ file system types), it works
rm> for those cases unless the order that these 2 file systems call vfs_register()
rm> is reversed. (The likelyhood of this reversal is lower than the likelyhood
rm> of vfs_register() just being called in a different order, I think?)
rm>
rm> Since it now changes whenever a different kernel config or different module
rm> loading order is used and there aren't a lot of complaints, I think "usually
rm> works" is good enough. However, it doesn't matter to me, so I'll let others
rm> decide.
rm>
rm> Yet another option is to go back to what hrs_at_ originally suggested, which
rm> was change ZFS to not use vfc_typenum in its fsid. (I now see that 0 won't
rm> conflict with any assigned vfc_typenum values, since they start at 1 and
rm> 255 would also probably be safe, since its unlikely there ever will be
rm> 255 different file system types.) The problem with this is no future FS
rm> can use the same trick.

 My opinion is using a hash function which occurs no collision in the
 well-known names is sufficient for our purpose because the number of
 file systems on a running system is small anyway and not changed
 frequently in most cases.  I am not sure which function is best;
 fnv_32_str() seemed better against a large data set but it is not
 likely to have >30 file systems, for example.

-- Hiroki