From owner-freebsd-current@FreeBSD.ORG Thu Aug 25 15:18:22 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 BAF251065672 for ; Thu, 25 Aug 2011 15:18:22 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.mail.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id 789508FC0C for ; Thu, 25 Aug 2011 15:18:22 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Ap4EAJ5mVk6DaFvO/2dsb2JhbABCDoQ+pC2BQAEBBAEjBFIFFhgCAg0ZAlkGiAWpH5FVgSyED4ERBJMZkENU X-IronPort-AV: E=Sophos;i="4.68,281,1312171200"; d="scan'208";a="132220468" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-annu-pri.mail.uoguelph.ca with ESMTP; 25 Aug 2011 11:18:21 -0400 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 6CA16B3F3E; Thu, 25 Aug 2011 11:18:21 -0400 (EDT) Date: Thu, 25 Aug 2011 11:18:21 -0400 (EDT) From: Rick Macklem To: Benjamin Kaduk Message-ID: <1252487773.337126.1314285501432.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [172.17.91.201] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - FF3.0 (Win)/6.0.10_GA_2692) Cc: current@FreeBSD.org 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: Thu, 25 Aug 2011 15:18:22 -0000 Benjamin Kaduk wrote: > > 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. > Here's my updated patch (it will wrap to 1 the first time and then exceed 255 if 1<->255 are all in use). --- kern/vfs_init.c.sav 2011-06-11 18:58:33.000000000 -0400 +++ kern/vfs_init.c 2011-08-25 11:09:14.000000000 -0400 @@ -39,6 +39,7 @@ __FBSDID("$FreeBSD: head/sys/kern/vfs_in #include #include +#include #include #include #include @@ -138,6 +139,9 @@ vfs_register(struct vfsconf *vfc) struct sysctl_oid *oidp; struct vfsops *vfsops; static int once; + struct vfsconf *tvfc; + uint32_t hashval; + int secondpass; if (!once) { vattr_null(&va_null); @@ -152,7 +156,31 @@ 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 + * all of 1<->255 are assigned, it is limited to 8bits since that is + * what ZFS uses from vfc_typenum and is also the preferred range + * for vfs_getnewfsid(). + */ + hashval = fnv_32_str(vfc->vfc_name, FNV1_32_INIT); + hashval &= 0xff; + secondpass = 0; + do { + /* Look for and fix any collision. */ + TAILQ_FOREACH(tvfc, &vfsconf, vfc_list) { + if (hashval == tvfc->vfc_typenum) { + if (hashval == 255 && secondpass == 0) { + hashval = 1; + secondpass = 1; + } else + hashval++; + 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); /* > 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. > If file systems use VFS_SET(), I don't think this can happen, since the same vfc_name would imply "same module name" and the 2nd one wouldn't load. (Been there, w.r.t. nfs.) > > + 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? Well, I still don't know if anything has a use for vfs_sysctl(), so I'm not volunteering to take it out. (If others feel it should come out for 9.0, maybe... But I would still consider that a separate patch.) rick