Date: Thu, 19 May 2016 12:20:19 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bruce Evans <brde@optusnet.com.au> Cc: Konstantin Belousov <kostikbel@gmail.com>, fs@freebsd.org Subject: Re: fix for per-mount i/o counting in ffs Message-ID: <20160519120557.A2250@besplex.bde.org> In-Reply-To: <20160519094901.O1798@besplex.bde.org> References: <20160517072104.I2137@besplex.bde.org> <20160517084241.GY89104@kib.kiev.ua> <20160518061040.D5948@besplex.bde.org> <20160518070252.F6121@besplex.bde.org> <20160517220055.GF89104@kib.kiev.ua> <20160518084931.T6534@besplex.bde.org> <20160518110834.GJ89104@kib.kiev.ua> <20160519065714.H1393@besplex.bde.org> <20160519094901.O1798@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 19 May 2016, Bruce Evans wrote:
> On Thu, 19 May 2016, Bruce Evans wrote:
>
>> ...
>> I think the following works to prevent multiple mounts via all of the
>> known buggy paths: early in every fsmount():
Here is a lightly tested version:
X Index: ffs_vfsops.c
X ===================================================================
X --- ffs_vfsops.c (revision 300160)
X +++ ffs_vfsops.c (working copy)
X @@ -771,18 +771,24 @@
X error = g_vfs_open(devvp, &cp, "ffs", ronly ? 0 : 1);
X g_topology_unlock();
X PICKUP_GIANT();
X + /* XXX: v_bufobj is left set after errors. */
X + devvp->v_bufobj.bo_ops = &ffs_ops;
X VOP_UNLOCK(devvp, 0);
Since v_bufobj isn't cleaned after later errors, I didn't move the unlock
to keep it clean for this error alone.
X if (error)
X - goto out;
X - if (devvp->v_rdev->si_iosize_max != 0)
X - mp->mnt_iosize_max = devvp->v_rdev->si_iosize_max;
X + goto out1;
X + dev_lock();
X + if (dev->si_mountpt != NULL) {
X + dev_unlock();
X + error = EBUSY;
X + goto out1;
X + }
X + dev->si_mountpt = mp;
X + dev_unlock();
X + if (dev->si_iosize_max != 0)
X + mp->mnt_iosize_max = dev->si_iosize_max;
X if (mp->mnt_iosize_max > MAXPHYS)
X mp->mnt_iosize_max = MAXPHYS;
X
X - devvp->v_bufobj.bo_ops = &ffs_ops;
X - if (devvp->v_type == VCHR)
X - devvp->v_rdev->si_mountpt = mp;
X -
X fs = NULL;
X sblockloc = 0;
X /*
X @@ -1081,10 +1087,14 @@
X #endif /* !UFS_EXTATTR */
X return (0);
X out:
X + dev_lock();
X + if (dev->si_mountpt == NULL)
X + panic("lost si_mountpt in mount");
X + dev->si_mountpt = NULL;
X + dev_unlock();
I don't want the debugging panics or KASSERTs in the final version.
Explicit locking the stores of NULL is probably not needed. dev_rel()
will soon make these stores visible and other locking and ordering
makes it very unlikely that they become visible too early.
X +out1:
X if (bp)
X brelse(bp);
X - if (devvp->v_type == VCHR && devvp->v_rdev != NULL)
X - devvp->v_rdev->si_mountpt = NULL;
X if (cp != NULL) {
X DROP_GIANT();
X g_topology_lock();
X @@ -1287,8 +1297,11 @@
X g_vfs_close(ump->um_cp);
X g_topology_unlock();
X PICKUP_GIANT();
X - if (ump->um_devvp->v_type == VCHR && ump->um_devvp->v_rdev != NULL)
X - ump->um_devvp->v_rdev->si_mountpt = NULL;
X + dev_lock();
X + if (ump->um_dev->si_mountpt == NULL)
X + panic("lost si_mountpt in unmount");
X + ump->um_dev->si_mountpt = NULL;
X + dev_unlock();
X vrele(ump->um_devvp);
X dev_rel(ump->um_dev);
X mtx_destroy(UFS_MTX(ump));
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160519120557.A2250>