Date: Thu, 12 Jul 2007 23:33:40 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Kostik Belousov <kostikbel@gmail.com> Cc: bugs@freebsd.org, fs@freebsd.org Subject: Re: msdosfs not MPSAFE Message-ID: <20070712225324.F9515@besplex.bde.org> In-Reply-To: <20070712084115.GA2200@deviant.kiev.zoral.com.ua> References: <20070710233455.O2101@besplex.bde.org> <20070712084115.GA2200@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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 _______________________________________________ freebsd-bugs@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "freebsd-bugs-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070712225324.F9515>