From owner-freebsd-fs@FreeBSD.ORG Sat Aug 4 07:57:41 2007 Return-Path: Delivered-To: fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3CC7616A420; Sat, 4 Aug 2007 07:57:41 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from relay02.kiev.sovam.com (relay02.kiev.sovam.com [62.64.120.197]) by mx1.freebsd.org (Postfix) with ESMTP id DF97313C494; Sat, 4 Aug 2007 07:57:40 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from [89.162.146.170] (helo=skuns.kiev.zoral.com.ua) by relay02.kiev.sovam.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.67) (envelope-from ) id 1IHEW0-000NC7-KT; Sat, 04 Aug 2007 10:57:39 +0300 Received: from deviant.kiev.zoral.com.ua (root@[10.1.1.148]) by skuns.kiev.zoral.com.ua (8.14.1/8.14.1) with ESMTP id l747vWBX082313 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 4 Aug 2007 10:57:32 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.1/8.14.1) with ESMTP id l747vVjS043269; Sat, 4 Aug 2007 10:57:31 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.1/8.14.1/Submit) id l747vUlv043268; Sat, 4 Aug 2007 10:57:30 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 4 Aug 2007 10:57:30 +0300 From: Kostik Belousov To: Bruce Evans Message-ID: <20070804075730.GP2738@deviant.kiev.zoral.com.ua> References: <20070710233455.O2101@besplex.bde.org> <20070712084115.GA2200@deviant.kiev.zoral.com.ua> <20070712225324.F9515@besplex.bde.org> <20070712142127.GD2200@deviant.kiev.zoral.com.ua> <20070716195556.P12807@besplex.bde.org> <20070721063434.GI2200@deviant.kiev.zoral.com.ua> <20070721233613.Q3366@besplex.bde.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="h9WqFG8zn/Mwlkpe" Content-Disposition: inline In-Reply-To: <20070721233613.Q3366@besplex.bde.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.91.1, clamav-milter version 0.91.1 on skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-1.4 required=5.0 tests=ALL_TRUSTED autolearn=failed version=3.2.1 X-Spam-Checker-Version: SpamAssassin 3.2.1 (2007-05-02) on skuns.kiev.zoral.com.ua X-Scanner-Signature: 7318d724e6e33de4232bc9d11f85b76e X-DrWeb-checked: yes X-SpamTest-Envelope-From: kostikbel@gmail.com X-SpamTest-Group-ID: 00000000 X-SpamTest-Header: Not Detected X-SpamTest-Info: Profiles 1336 [August 3 2007] X-SpamTest-Info: helo_type=3 X-SpamTest-Method: none X-SpamTest-Rate: 0 X-SpamTest-Status: Not detected X-SpamTest-Status-Extended: not_detected X-SpamTest-Version: SMTP-Filter Version 3.0.0 [0255], KAS30/Release Cc: bugs@freebsd.org, fs@freebsd.org Subject: Re: msdosfs not MPSAFE 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: Sat, 04 Aug 2007 07:57:41 -0000 --h9WqFG8zn/Mwlkpe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jul 21, 2007 at 11:52:04PM +1000, Bruce Evans wrote: > On Sat, 21 Jul 2007, Kostik Belousov wrote: >=20 > >On Mon, Jul 16, 2007 at 08:18:14PM +1000, Bruce Evans wrote: > >>sx xlocking works, but is not quite right: > >>% /* > >>% + * XXX msdosfs_lookup() is split up because unlocking before all the > >>returns > >>% + * in the original function would be too churning. > >>% + */ > >>% +int > >>% +msdosfs_lookup(ap) > >>% + struct vop_cachedlookup_args *ap; > >>% +{ > >>% + int error; > >>% + > >>% + sx_xlock(&mbnambuf_lock); > >>% + error =3D msdosfs_lookup_locked(ap); > >>% + sx_xunlock(&mbnambuf_lock); > >>% + return (error); > >>% +} > >>% + > >>% +/* > > > >Assume that a directory A is participating in lookup() from two threads: > >thread 1 lookup the A itself; > >thread 2 lookup some entry in the A. > >Then, thread 1 would have mbnambuf_lock locked, and may wait for A' > >vnode lock; > >thread 2 shall own vnode lock for A, then locking mbnambuf_lock. > > > >I do not see what may prevent this LOR scenario from realizing, or what > >make it harmless. >=20 > Nothing I can see either. The wrapper is too global. >=20 > Next try: move locking into the inner loop in msdosfs_lookup(). Unlocking > is not as ugly as I feared. The following has only been tested at compile > time: >=20 > % Index: msdosfs_lookup.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 > % RCS file: /home/ncvs/src/sys/fs/msdosfs/msdosfs_lookup.c,v > % retrieving revision 1.40 > % diff -u -2 -r1.40 msdosfs_lookup.c > % --- msdosfs_lookup.c 26 Dec 2003 17:24:37 -0000 1.40 > % +++ msdosfs_lookup.c 21 Jul 2007 13:27:37 -0000 > % @@ -54,4 +54,5 @@ > % #include > % #include > % +#include > % #include > % #include > % @@ -63,4 +64,6 @@ > % #include > %=20 > % +extern struct sx mbnambuf_lock; > % + > % /* > % * When we search a directory the blocks containing directory entries = are > % @@ -192,4 +195,5 @@ > % */ > % tdp =3D NULL; > % + sx_xlock(&mbnambuf_lock); > % mbnambuf_init(); > % /* > % @@ -206,4 +210,5 @@ > % if (error =3D=3D E2BIG) > % break; > % + sx_xunlock(&mbnambuf_lock); > % return (error); > % } > % @@ -211,4 +216,5 @@ > % if (error) { > % brelse(bp); > % + sx_xunlock(&mbnambuf_lock); > % return (error); > % } > % @@ -240,4 +246,5 @@ > % if (dep->deName[0] =3D=3D SLOT_EMPTY) { > % brelse(bp); > % + sx_xunlock(&mbnambuf_lock); > % goto notfound; > % } > % @@ -301,4 +308,5 @@ > % dp->de_fndcnt =3D wincnt - 1; > %=20 > % + sx_xunlock(&mbnambuf_lock); > % goto found; > % } > % @@ -310,4 +318,5 @@ > % brelse(bp); > % } /* for (frcn =3D 0; ; frcn++) */ > % + sx_xunlock(&mbnambuf_lock); > %=20 > % notfound: >=20 > After moving the locking into msdosfs_conv.c and adding assertions there, > this should be a good enough fix until the mbnambuf interface is changed. > This bug is in all versions since 5.2-RELEASE. Once again, sorry for late answer. The change seems to be good. --h9WqFG8zn/Mwlkpe Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.7 (FreeBSD) iD8DBQFGtDFpC3+MBN1Mb4gRAr1XAJ9hWtnFtf0kiW6EvRrLwiCrJhvKZACfVdGC 9Dp51cDM0HfDVEaDRq2rbk4= =V3NH -----END PGP SIGNATURE----- --h9WqFG8zn/Mwlkpe--