From owner-freebsd-fs@freebsd.org Thu May 19 10:41:36 2016 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 103FCB42EE2 for ; Thu, 19 May 2016 10:41:36 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mailman.ysv.freebsd.org (unknown [127.0.1.3]) by mx1.freebsd.org (Postfix) with ESMTP id ECACE1AB6 for ; Thu, 19 May 2016 10:41:35 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: by mailman.ysv.freebsd.org (Postfix) id EBFB7B42EE1; Thu, 19 May 2016 10:41:35 +0000 (UTC) Delivered-To: fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EBA37B42EE0 for ; Thu, 19 May 2016 10:41:35 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 9730B1AB3 for ; Thu, 19 May 2016 10:41:35 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id u4JAfTdn056064 (version=TLSv1 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Thu, 19 May 2016 13:41:29 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua u4JAfTdn056064 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id u4JAfS7R056063; Thu, 19 May 2016 13:41:28 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 19 May 2016 13:41:28 +0300 From: Konstantin Belousov To: Bruce Evans Cc: fs@freebsd.org Subject: Re: fix for per-mount i/o counting in ffs Message-ID: <20160519104128.GN89104@kib.kiev.ua> 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> <20160519120557.A2250@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160519120557.A2250@besplex.bde.org> User-Agent: Mutt/1.6.1 (2016-04-27) 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 autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 May 2016 10:41:36 -0000 On Thu, May 19, 2016 at 12:20:19PM +1000, Bruce Evans wrote: > 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: There is no need to protect the si_mountpt with any locking, the field itself serves as a lock good enough, also preventing the parallel mounts of the same devices. I changed the assignement to atomic_cmpset, which is enough there. It is somewhat pity that this would reliably disable multiple ro mounts of the same volume. There is no need to move assignment of NULL to dev->si_mountpt later in ffs_unmount(), the moment where the assignment is performed is safe for other thread to start another mount. I still want to keep devvp locked for long enough to cover the bufobj hacking, and I do not want to move bufobj.bo_ops change before g_vfs_open() succeed. I also wanted to remove GIANT dances, but this requires geom patch, which I will mail separately. diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c index 712fc21..21425f5 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -764,24 +764,29 @@ ffs_mountfs(devvp, mp, td) cred = td ? td->td_ucred : NOCRED; ronly = (mp->mnt_flag & MNT_RDONLY) != 0; + KASSERT(devvp->v_type == VCHR, ("reclaimed devvp")); dev = devvp->v_rdev; dev_ref(dev); + if (!atomic_cmpset_ptr(&dev->si_mountpt, 0, mp)) { + dev_rel(dev); + VOP_UNLOCK(devvp, 0); + return (EBUSY); + } DROP_GIANT(); g_topology_lock(); error = g_vfs_open(devvp, &cp, "ffs", ronly ? 0 : 1); g_topology_unlock(); PICKUP_GIANT(); - VOP_UNLOCK(devvp, 0); - if (error) + if (error != 0) { + VOP_UNLOCK(devvp, 0); goto out; - if (devvp->v_rdev->si_iosize_max != 0) - mp->mnt_iosize_max = devvp->v_rdev->si_iosize_max; + } + if (dev->si_iosize_max != 0) + mp->mnt_iosize_max = dev->si_iosize_max; if (mp->mnt_iosize_max > MAXPHYS) mp->mnt_iosize_max = MAXPHYS; - devvp->v_bufobj.bo_ops = &ffs_ops; - if (devvp->v_type == VCHR) - devvp->v_rdev->si_mountpt = mp; + VOP_UNLOCK(devvp, 0); fs = NULL; sblockloc = 0; @@ -1083,8 +1088,6 @@ ffs_mountfs(devvp, mp, td) out: if (bp) brelse(bp); - if (devvp->v_type == VCHR && devvp->v_rdev != NULL) - devvp->v_rdev->si_mountpt = NULL; if (cp != NULL) { DROP_GIANT(); g_topology_lock(); @@ -1102,6 +1105,7 @@ out: free(ump, M_UFSMNT); mp->mnt_data = NULL; } + dev->si_mountpt = NULL; dev_rel(dev); return (error); } @@ -1287,8 +1291,7 @@ ffs_unmount(mp, mntflags) g_vfs_close(ump->um_cp); g_topology_unlock(); PICKUP_GIANT(); - if (ump->um_devvp->v_type == VCHR && ump->um_devvp->v_rdev != NULL) - ump->um_devvp->v_rdev->si_mountpt = NULL; + ump->um_dev->si_mountpt = NULL; vrele(ump->um_devvp); dev_rel(ump->um_dev); mtx_destroy(UFS_MTX(ump));