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
[-- Attachment #1 --]
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 those
> 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 still
> allowing any "real" error to be caught. I'm including a small patch that
> 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
> ===================================================================
> --- 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 = kern_kldload(td, fstype, &fileid);
> + if (*error == EEXIST) {
> + *error = 0;
> + fileid = 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);
>
> /* Look up again to see if the VFS was loaded. */
> vfsp = vfs_byname(fstype);
> if (vfsp == NULL) {
> - (void)kern_kldunload(td, fileid, LINKER_UNLOAD_FORCE);
> + if (fileid != 0)
> + (void)kern_kldunload(td, fileid, LINKER_UNLOAD_FORCE);
> *error = 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"
[-- Attachment #2 --]
-----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-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130220054309.GD2598>