From owner-freebsd-fs@FreeBSD.ORG Wed Feb 20 15:37:14 2013 Return-Path: Delivered-To: fs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 04CFEB41; Wed, 20 Feb 2013 15:37:14 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) by mx1.freebsd.org (Postfix) with ESMTP id 57222C29; Wed, 20 Feb 2013 15:37:13 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.6/8.14.6) with ESMTP id r1KFb5Uo069015; Wed, 20 Feb 2013 17:37:05 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.7.4 kib.kiev.ua r1KFb5Uo069015 Received: (from kostik@localhost) by tom.home (8.14.6/8.14.6/Submit) id r1KFb5ZH069014; Wed, 20 Feb 2013 17:37:05 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 20 Feb 2013 17:37:05 +0200 From: Konstantin Belousov To: Jamie Gritton Subject: Re: mount/kldload race Message-ID: <20130220153705.GE2598@kib.kiev.ua> References: <51244A13.8030907@FreeBSD.org> <20130220054309.GD2598@kib.kiev.ua> <5124E372.1000009@FreeBSD.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="UOi+gfmBpEZPw9cU" Content-Disposition: inline In-Reply-To: <5124E372.1000009@FreeBSD.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: fs@FreeBSD.org X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2013 15:37:14 -0000 --UOi+gfmBpEZPw9cU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 20, 2013 at 07:53:38AM -0700, Jamie Gritton wrote: > On 02/19/13 22:43, Konstantin Belousov wrote: > > On Tue, Feb 19, 2013 at 08:59:15PM -0700, Jamie Gritton wrote: > >> Perhaps most people don't try to mount a bunch of filesystems at the > >> same time, at least not those that depend on kernel modules. But it > >> turns out that's going to be a pretty common situation with jails and > >> nullfs. And I found that when attempting such a feat will cause most of > >> these simultaneous mounts to fail with ENODEV. > >> > >> It turns out that the problem is a race in vfs_byname_kld(). First it'= ll > >> see if the fstype is loaded, and if it isn't then it will load the > >> module. But if the module is loaded by a different process between tho= se > >> two points, the resulting EEXIST from kern_kldload() will make > >> vfs_byname_kld() error out. > >> > >> The fix is pretty simple: don't treat EEXIST as an error. By going on, > >> and rechecking for the fstype, the filesystem can be mounted while sti= ll > >> allowing any "real" error to be caught. I'm including a small patch th= at > >> will accomplish this, and I'd appreciate a quick look by anyone who's > >> familiar with this part of things before I commit it. > >> > >> - Jamie > > > >> Index: sys/kern/vfs_init.c > >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > >> --- sys/kern/vfs_init.c (revision 247000) > >> +++ sys/kern/vfs_init.c (working copy) > >> @@ -130,13 +130,18 @@ > >> > >> /* Try to load the respective module. */ > >> *error =3D kern_kldload(td, fstype,&fileid); > >> + if (*error =3D=3D EEXIST) { > >> + *error =3D 0; > >> + fileid =3D 0; > > Why do you clear fileid ? Is this to prevent an attempt to kldunload() > > the module which was not loaded by the current thread ? > > > > If yes, I would suggest to use the separate flag to track this, > > which is cleared on EEXIST error. IMHO it is cleaner and less puzzling. >=20 > Yes, that's why. As a side note, I clear *error ostensibly for the sake= =20 > of the callers, but it turns out none of the callers actually look at=20 > the returned error. >=20 > Here's a new patch with an added flag: I have no further comments, looks good. >=20 >=20 > Index: sys/kern/vfs_init.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/kern/vfs_init.c (revision 247000) > +++ sys/kern/vfs_init.c (working copy) > @@ -122,7 +122,7 @@ > vfs_byname_kld(const char *fstype, struct thread *td, int *error) > { > struct vfsconf *vfsp; > - int fileid; > + int fileid, loaded; >=20 > vfsp =3D vfs_byname(fstype); > if (vfsp !=3D NULL) > @@ -130,13 +130,17 @@ >=20 > /* Try to load the respective module. */ > *error =3D kern_kldload(td, fstype, &fileid); > + loaded =3D (*error =3D=3D 0); > + if (*error =3D=3D EEXIST) > + *error =3D 0; > if (*error) > return (NULL); >=20 > /* Look up again to see if the VFS was loaded. */ > vfsp =3D vfs_byname(fstype); > if (vfsp =3D=3D NULL) { > - (void)kern_kldunload(td, fileid, LINKER_UNLOAD_FORCE); > + if (loaded) > + (void)kern_kldunload(td, fileid, LINKER_UNLOAD_FORCE); > *error =3D ENODEV; > return (NULL); > } --UOi+gfmBpEZPw9cU Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iQIcBAEBAgAGBQJRJO2gAAoJEJDCuSvBvK1Bj44P/3XSMKbo4w/4W3Gf7MnCcn01 NOkmXv36TthOxkXUEOWldzKUQzYuvG4lbQ8m8w+nrlP5C5wVFY2a7jWAZBHLEs96 m/H35R5Dke3a4UEd1Q7lb47LhIhR65hmzyBYCP2ylwLKdAI4kuWxmoFPV62QFIqt csMomk2N7FtMB4F4ryMxHTA6ERzbjh5JE+kdR2KtDFXOdWeDa7no4NYnXjAcxACK +ikfCHrvYfgsq3goUg12CSFbQth9naKSbRtSm5qXF07akjnwNc+mtrni4mxN/+YM 8kgehvojPIFkDzOw+tTK7EnRlTovXroV0VcDm0r0rAZFo/3eJxvh5nIqbyBpM4oO tqTs1qGk1o4XA8lytAfP2UFUb9LOD2CIarjcre6Mj/tF5t0CThZLJEZvLRjxNZ0L Cp46C/vf6m348UsP06gu74WibtGwVBlJthr+IXwLUZnNvsKrDZLrq4EXl7n1gxJ9 VYkeIYStpKklwj/6h0GHBAZhF9sbDUsqUPQ6VFnKz6EOQIxsZTIQtytWImLOTArZ DHZRKta7AdtylDk8YHH83p90AMEduCBFWse/ZGv7+kcUbyehtfMm1QmTkdBwPZd4 WYKd/Gy5y35xgD+MWuTvly7aqss3dwHk581CQyfAjFYEkvcW8VZtLYDrgbrk9rfu /vE7ZFtHooVRyr6mlsV2 =PTS+ -----END PGP SIGNATURE----- --UOi+gfmBpEZPw9cU--