Date: Wed, 20 Feb 2013 07:43:09 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Jamie Gritton <jamie@FreeBSD.org> Cc: fs@FreeBSD.org Subject: Re: mount/kldload race Message-ID: <20130220054309.GD2598@kib.kiev.ua> In-Reply-To: <51244A13.8030907@FreeBSD.org> References: <51244A13.8030907@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--7HhoQoqNsng1reXT
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
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=20
> same time, at least not those that depend on kernel modules. But it=20
> turns out that's going to be a pretty common situation with jails and=20
> nullfs. And I found that when attempting such a feat will cause most of=
=20
> these simultaneous mounts to fail with ENODEV.
>=20
> It turns out that the problem is a race in vfs_byname_kld(). First it'll=
=20
> see if the fstype is loaded, and if it isn't then it will load the=20
> module. But if the module is loaded by a different process between those=
=20
> two points, the resulting EEXIST from kern_kldload() will make=20
> vfs_byname_kld() error out.
>=20
> The fix is pretty simple: don't treat EEXIST as an error. By going on,=20
> and rechecking for the fstype, the filesystem can be mounted while still=
=20
> allowing any "real" error to be caught. I'm including a small patch that=
=20
> will accomplish this, and I'd appreciate a quick look by anyone who's=20
> familiar with this part of things before I commit it.
>=20
> - 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 @@
> =20
> /* 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.
> + }
> 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 (fileid !=3D 0)
> + (void)kern_kldunload(td, fileid, LINKER_UNLOAD_FORCE);
> *error =3D ENODEV;
> return (NULL);
> }
> _______________________________________________
> freebsd-fs@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org"
--7HhoQoqNsng1reXT
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)
iQIcBAEBAgAGBQJRJGJtAAoJEJDCuSvBvK1BcN0P/23vCOiUpRsEV5ODpT1qdD9E
DsdB8/WAIc7xL1OgSUL5M4kORDpH6i7gqnDLlYJRtPvc6fikvrEGR9MpC4fFMHJ3
58LaKdhWTvl2YRKyLxcpO4rC9yx86DVLbA57ya7EH+P+9Ij/Ehh0NM8yaVO3xCQE
0/aP0MwPHxfAbkm8ybB8MEVegLWzBEMGds8Yvybxm0fIRuCNZKprhVd+XpcM73Dp
LoTeRm0ho+ggzlJv4NfwPsCoYgMHLML0wLibqXbUsQgJVKvNZ06sYZVFvT6ywc+n
b1L2y/Qxibmxww/lUy61AgxXg+/h7vIme21IsWn905K3ka0IWhuOud3Mn2X51N4b
cSqXC0FWhDcXT77iPOp/aVlOKrPqtarqX3WqFdM6AiGkZ/pciis4YtXvn1q2midV
UbBB2rdORUuSZXs1yJNLuOn5UKLFCrZKSnxScZxSaEhE3U7OwCXRjfxYFbTvlovr
Xl5dQqpLirrZlhjg2gYcftl964BhmIIEj0a3QrWFfZZ4cfE9JOmQ0n5JpWfNopir
FNmLS6nsNwG8sJN0cRKloC1cB5yELHn7ZeFvGKD2ttQRCpFH1ou7Oc3cnV5Xv07m
UXBO5K3ztmyxnS0TSuNyuIN01YFN6cxH3+dU0ssMl05+UpYsf6SL7Kpz/DGK3TTT
Em0OAxk446w7bjtf56UX
=u3qn
-----END PGP SIGNATURE-----
--7HhoQoqNsng1reXT--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130220054309.GD2598>