From owner-freebsd-current@FreeBSD.ORG Wed Aug 24 21:01:23 2011 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 69A12106564A; Wed, 24 Aug 2011 21:01:23 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-jnhn.mail.uoguelph.ca (esa-jnhn.mail.uoguelph.ca [131.104.91.44]) by mx1.freebsd.org (Postfix) with ESMTP id F08DE8FC1E; Wed, 24 Aug 2011 21:01:22 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Ap4EAPJlVU6DaFvO/2dsb2JhbABChEykK4FAAQEFIwRSGxgCAg0ZAlkGLrAjkWCBLIQNgRAEkxmRFw X-IronPort-AV: E=Sophos;i="4.68,277,1312171200"; d="scan'208";a="135448656" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-jnhn-pri.mail.uoguelph.ca with ESMTP; 24 Aug 2011 17:01:22 -0400 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 980F2B3F77; Wed, 24 Aug 2011 17:01:22 -0400 (EDT) Date: Wed, 24 Aug 2011 17:01:22 -0400 (EDT) From: Rick Macklem To: Pawel Jakub Dawidek Message-ID: <468764384.310026.1314219682612.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20110824200813.GC1688@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [172.17.91.202] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - FF3.0 (Win)/6.0.10_GA_2692) Cc: Kostik Belousov , current@freebsd.org, kaduk@MIT.EDU Subject: Re: fsid change of ZFS? X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Aug 2011 21:01:23 -0000 Pawel Jakub Dawidek wrote: > On Wed, Aug 24, 2011 at 04:41:25PM +0300, Kostik Belousov wrote: > > On Wed, Aug 24, 2011 at 09:36:37AM -0400, Rick Macklem wrote: > > > Well, doesn't this result in the same issue as the fixed table? > > > In other words, the developer has to supply the "suggested byte" > > > for > > > fsid and make sure that it doesn't conflict with other "suggested > > > byte" > > > values or suffer the same consequence as forgetting to update the > > > fixed > > > table. (ie. It just puts the fixed value in a different place, > > > from what > > > I see, for in-tree modules. Also, with a fixed table, they are all > > > in > > > one place, so it's easy to choose a non-colliding value?) > > The reason for my proposal was Pawel note that a porter of the > > filesystem > > should be aware of some place in kern/ where to register, besides > > writing > > the module. > > Well, he has to be aware, but we should do all we can to minimize the > number of place he needs to update, as it is easy to forget some. > > I agree with Rick that what you proposed is similar to fixed table of > file system names and I'd prefer to avoid that. If we can have > name-based hash that produces no collision for in-tree file systems > and > know current 3rd party file systems plus collision detection for the > future then it is good enough, IMHO. And this is what Rick proposed > with > his patch. > Ok, here is the list of file system names I've been checking and there haven't been any collisions (either hash function). If you know of other well known file type names, please email and I'll add them to the list and check for collisions again. "cd9660" "ufs" "procfs" "devfs" "msdosfs" "nfs" "xfs" "reiserfs" "tmpfs" "hpfs" "ntfs" "ext2fs" "udf" "zfs" "afs" 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 #include +#include #include #include #include @@ -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. */ + 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); /*