From owner-freebsd-bugs@FreeBSD.ORG Thu Jul 12 13:33:44 2007 Return-Path: X-Original-To: bugs@freebsd.org Delivered-To: freebsd-bugs@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id CAB1416A400; Thu, 12 Jul 2007 13:33:44 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail01.syd.optusnet.com.au (mail01.syd.optusnet.com.au [211.29.132.182]) by mx1.freebsd.org (Postfix) with ESMTP id 6A2F013C46C; Thu, 12 Jul 2007 13:33:44 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c220-239-235-248.carlnfd3.nsw.optusnet.com.au [220.239.235.248]) by mail01.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id l6CDXe1g007363 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 12 Jul 2007 23:33:42 +1000 Date: Thu, 12 Jul 2007 23:33:40 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Kostik Belousov In-Reply-To: <20070712084115.GA2200@deviant.kiev.zoral.com.ua> Message-ID: <20070712225324.F9515@besplex.bde.org> References: <20070710233455.O2101@besplex.bde.org> <20070712084115.GA2200@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: bugs@freebsd.org, fs@freebsd.org, Bruce Evans 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: Thu, 12 Jul 2007 13:33:44 -0000 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 syscalls) >> was removed. It allows multiple threads to race accessing the shared >> static buffer `nambuf' and related variables. This causes remarkably >> [Add Giant locking] >> Please fix this better. mbnambuf_init() could return a non-static buffer >> that doesn't require locking. Deallocation would be painful. Note that >> even the details for Giant locking can't be hidden in msdosfs_conv.c like >> the current interfaces intend, since mbnambuf_init() is used a lot to >> reinitialize an in-use buffer, and there is no interface to drop usage. > It seems that msdosfs_lookup() can sleep, thus Giant protection would be > lost. It can certainly block in bread(). I now think this is not really an SMP bug. The nambuf* data structure requires a long-term global lock, but it has never had one. The bug seems to be relatively new. nambuf* is for multi-byte characters, not for long names, and has only existed since 2003 (msdosfs_vnops.c 1.141, etc.). I thought that long names were built up in nambuf, but they are apparently built up in the directory entry. This should work for multi-byte characters too -- don't translate anything until all the low- level directory entries have been accumulated. 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. Bruce