From owner-freebsd-fs@FreeBSD.ORG Fri Sep 30 20:19:00 2011 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 23705106566B; Fri, 30 Sep 2011 20:19:00 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id B10358FC0C; Fri, 30 Sep 2011 20:18:59 +0000 (UTC) Received: from alf.home (alf.kiev.zoral.com.ua [10.1.1.177]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id p8UKIqoZ037219 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 30 Sep 2011 23:18:52 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from alf.home (kostik@localhost [127.0.0.1]) by alf.home (8.14.5/8.14.5) with ESMTP id p8UKIqfR058649; Fri, 30 Sep 2011 23:18:52 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by alf.home (8.14.5/8.14.5/Submit) id p8UKIpHj058648; Fri, 30 Sep 2011 23:18:51 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: alf.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 30 Sep 2011 23:18:51 +0300 From: Kostik Belousov To: Kirk McKusick Message-ID: <20110930201851.GB1511@deviant.kiev.zoral.com.ua> References: <201109301820.p8UIKSGj039445@chez.mckusick.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="m4l0o/auqPKO2x83" Content-Disposition: inline In-Reply-To: <201109301820.p8UIKSGj039445@chez.mckusick.com> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.3 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: Attilio Rao , Garrett Cooper , Xin LI , freebsd-fs@freebsd.org Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Sep 2011 20:19:00 -0000 --m4l0o/auqPKO2x83 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 30, 2011 at 11:20:28AM -0700, Kirk McKusick wrote: > > Date: Fri, 30 Sep 2011 15:31:56 +0200 > > Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? > > From: Attilio Rao > > To: Kirk McKusick > > Cc: Konstantin Belousov , > > Garrett Cooper , > > freebsd-fs@freebsd.org, Xin LI > >=20 > > 2011/9/30 Kirk McKusick : > >=20 > > > Here is my proposed fix. It does the unroll originally found in the > > > non-FORCE case before sleeping waiting for the vfs_busy to clear. > > > Is it acceptable to hold the mount mutex while calling VOP_UNLOCK? > > > If not, then it needs to be released before the unlock, reacquired > > > afterwards, and the check to see if the sleep is needed redone. > >=20 > > I thought about this approach when sending the e-mail, but there is a > > problem: you need to handle the MNTK_UNMOUNT flag checking and > > subsequent setting after coveredvnode is held, otherwise at the first > > looping you will just return EBUSY. > >=20 > > You can avoid the unlock by passing PVFS | PDROP. > >=20 > > Attilio >=20 > Problem noted. I have updated the patch to clear the MNTK_UNMOUNT > (and other flags set above it) after it returns from the sleep. > Which means I cannot use the PDROP flag now, but it is good to > know about it for future reference. >=20 > Still not clear to me if it is acceptable to hold the mount mutex > while calling VOP_UNLOCK. Should I drop the mount mutex around the > VOP_UNLOCK(coveredvp)? Other than that possible problem, this patch > appears to solve the EBUSY problem and avoid possible deadlocks. I do not understand which deadlock is talked about there. It seems thay Attilio concern was with acquiring covered vnode lock after mounted fs is busied, but this is prohibited. See r166167 for more detailed description of the order. >=20 > Kirk McKusick >=20 > Index: sys/kern/vfs_mount.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_mount.c (revision 225884) > +++ sys/kern/vfs_mount.c (working copy) > @@ -1187,6 +1187,7 @@ > =20 > mtx_assert(&Giant, MA_OWNED); > =20 > +top: > if ((coveredvp =3D mp->mnt_vnodecovered) !=3D NULL) { > mnt_gen_r =3D mp->mnt_gen; > VI_LOCK(coveredvp); > @@ -1227,21 +1228,19 @@ > mp->mnt_kern_flag |=3D MNTK_UNMOUNTF; > error =3D 0; > if (mp->mnt_lockref) { > - if ((flags & MNT_FORCE) =3D=3D 0) { > - mp->mnt_kern_flag &=3D ~(MNTK_UNMOUNT | MNTK_NOINSMNTQ | > - MNTK_UNMOUNTF); > - if (mp->mnt_kern_flag & MNTK_MWAIT) { > - mp->mnt_kern_flag &=3D ~MNTK_MWAIT; > - wakeup(mp); > - } > - MNT_IUNLOCK(mp); > - if (coveredvp) > - VOP_UNLOCK(coveredvp, 0); > - return (EBUSY); > + if (mp->mnt_kern_flag & MNTK_MWAIT) { > + mp->mnt_kern_flag &=3D ~MNTK_MWAIT; > + wakeup(mp); > } > + if (coveredvp) > + VOP_UNLOCK(coveredvp, 0); > mp->mnt_kern_flag |=3D MNTK_DRAINING; > error =3D msleep(&mp->mnt_lockref, MNT_MTX(mp), PVFS, > "mount drain", 0); > + mp->mnt_kern_flag &=3D ~(MNTK_UNMOUNT | MNTK_NOINSMNTQ | > + MNTK_UNMOUNTF); > + MNT_IUNLOCK(mp); > + goto top; > } > MNT_IUNLOCK(mp); > KASSERT(mp->mnt_lockref =3D=3D 0, --m4l0o/auqPKO2x83 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iEYEARECAAYFAk6GJCsACgkQC3+MBN1Mb4gjDQCghKC8OgO5SmPn3QAfwjbgBmiC 0yoAoM6YZsEQgGWARcYMPLFOWvCot3yj =UE5c -----END PGP SIGNATURE----- --m4l0o/auqPKO2x83--