From owner-freebsd-bugs@FreeBSD.ORG Sat Jul 21 06:35:04 2007 Return-Path: Delivered-To: bugs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0BAB416A419; Sat, 21 Jul 2007 06:35:04 +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 97A7713C458; Sat, 21 Jul 2007 06:35:03 +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 1IC8YM-000JiN-Nn; Sat, 21 Jul 2007 09:35:01 +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 l6L6Yb9N081612 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 21 Jul 2007 09:34:37 +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 l6L6Yaeo050522; Sat, 21 Jul 2007 09:34:36 +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 l6L6YZee050521; Sat, 21 Jul 2007 09:34:35 +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, 21 Jul 2007 09:34:34 +0300 From: Kostik Belousov To: Bruce Evans Message-ID: <20070721063434.GI2200@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> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="12mfjY/2IcLDkfPj" Content-Disposition: inline In-Reply-To: <20070716195556.P12807@besplex.bde.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.90.3, clamav-milter version 0.90.3 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: e7d6a74e6a41bfc4865bf8c76b21c35c 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 1263 [July 20 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-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 21 Jul 2007 06:35:04 -0000 --12mfjY/2IcLDkfPj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 16, 2007 at 08:18:14PM +1000, Bruce Evans wrote: > On Thu, 12 Jul 2007, Kostik Belousov wrote: >=20 > >On Thu, Jul 12, 2007 at 11:33:40PM +1000, Bruce Evans wrote: > >> > >>On Thu, 12 Jul 2007, Kostik Belousov wrote: > >> > >>>On Wed, Jul 11, 2007 at 12:08:19AM +1000, Bruce Evans wrote: > >>>>msdsofs has been broken since Giant locking for file systems (or=20 > >>>>syscalls) > >>>>was removed. It allows multiple threads to race accessing the shared > >>>>static buffer `nambuf' and related variables. This causes remarkably > >> > >>>It seems that msdosfs_lookup() can sleep, thus Giant protection would = be > >>>lost. > >> > >>It can certainly block in bread(). > >Besides bread(), there is a (re)locking for ".." case, and deget() call, > >that itself calls malloc(M_WAITOK), vfs_hash_get(), getnewvnode() and > >readep(). The latter itself calls bread(). > > > >This is from the brief look. >=20 > I think msdosfs_lookup() doesn't need to own nambuf near the deget() > call. Not sure -- I was looking more at msdosfs_readdir(). >=20 > >>How does my adding Giant locking help? I checked that at least in > >>FreeBSD-~5.2-current, msdosfs_readdir() is already Giant-locked, so my > >>fix just increments the recursion count. What happens to recursively- > >>held Giant locks across sleeps? I think they should cause a KASSERT() > >>failure, but if they are handled by only dropping Giant once then my > >>fix might sort of work but sleeps would be broken generally. > >> > >Look at the kern/kern_sync.c:_sleep(). It does DROP_GIANT(), that (from > >the sys/mutex.h) calls mtx_unlock() until Giant is owned. >=20 > So it is very mysterious that Giant locking helped. Anyway, it doesn't > work, and cases where it doesn't help showed up in further testing. >=20 > sx xlocking works, but is not quite right: > % /* > % + * XXX msdosfs_lookup() is split up because unlocking before all the= =20 > 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. Did I missed something ? --12mfjY/2IcLDkfPj Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.7 (FreeBSD) iD8DBQFGoaj4C3+MBN1Mb4gRAlcwAKCbJh5IOqLlZkd05jW2t6ktgbIaQACgnJNr dtX3BEnDUbOzxeOKkxXrmW8= =ts/I -----END PGP SIGNATURE----- --12mfjY/2IcLDkfPj--