From owner-freebsd-hackers@FreeBSD.ORG Fri Apr 27 06:49:25 2007 Return-Path: X-Original-To: freebsd-hackers@freebsd.org Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8D3CF16A406; Fri, 27 Apr 2007 06:49:25 +0000 (UTC) (envelope-from hselasky@c2i.net) Received: from swip.net (mailfe04.swip.net [212.247.154.97]) by mx1.freebsd.org (Postfix) with ESMTP id 7124C13C44B; Fri, 27 Apr 2007 06:49:24 +0000 (UTC) (envelope-from hselasky@c2i.net) X-Cloudmark-Score: 0.000000 [] Received: from [193.71.38.142] (account mc467741@c2i.net HELO [10.42.11.147]) by mailfe04.swip.net (CommuniGate Pro SMTP 5.1.7) with ESMTPA id 474056930; Fri, 27 Apr 2007 07:49:14 +0200 From: Hans Petter Selasky To: "Attilio Rao" Date: Fri, 27 Apr 2007 07:48:49 +0200 User-Agent: KMail/1.9.5 References: <200704262136.33196.hselasky@c2i.net> <46311708.5030002@elischer.org> <3bbf2fe10704261450n50b66392saa7dc2ea7f091d31@mail.gmail.com> In-Reply-To: <3bbf2fe10704261450n50b66392saa7dc2ea7f091d31@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200704270748.49404.hselasky@c2i.net> Cc: freebsd-hackers@freebsd.org, Julian Elischer Subject: Re: msleep() on recursivly locked mutexes X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Apr 2007 06:49:25 -0000 On Thursday 26 April 2007 23:50, Attilio Rao wrote: > 2007/4/26, Julian Elischer : > > The reason that mutexes ever recurse in the first place is usually > > because one piece of code calls itself (or a related piece of code) in a > > blind manner.. in other words, it doesn't know it is doing so. The whole > > concept of recursing mutexes is a bit gross. The concept of blindly > > unwinding them is I think just asking for trouble. > > > > Further the idea that holding a mutex "except for when we sleep" is a > > generally bright idea is also a bit odd to me.. If you hold a mutex and > > release it during sleep you probably should invalidate all assumptions > > you made during the period before you slept as whatever you were That is not always correct. If you run your code in a separate thread/taskqueue, then you simply wait for this thread/taskqueue to complete somewhere else. This is basically when I need to exit mutexes. > > protecting has possibly been raped while you slept. I have seen too many > > instances where people just called msleep and dropped the mutex they > > held, picked it up again on wakeup, and then blithely continued on > > without checking what happened while they were asleep. > > Basically, the idea you cannot hold "blocking" locks (mutexes and > rwlocks) while sleeping, cames from the difference there is behind > turnstiles and sleepqueues. > > Turnstiles are thought to serve syncronous events, for short period of > time (or rather short) while sleepqueues are thought to serve > asyncronous events, so that the path to be protected can be > definitively bigger. If you fit in the situation you have to call > first a blocking lock and later a sleeping lock, probabilly you are > just using a wrong locking strategy and you should really revisit it. The suggestion is just for convenience. Usually you don't have a recursed mutex to sleep on. It is just to catch some rare cases where you will end up with a doubly locked mutex, which is not part of the ordinary code path. I don't have such cases in the kernel of the new USB stack, but there are some cases in the USB device drivers, which is due to some mutex locking moves. Those I can fix. My idea was that by allowing recursive mutexes to sleep, you will end up with less panics in the end for the unwary code developer. You just protect your code with mutexes and if they recurse calling synchronous USB functions, you don't have to care. > > As you mention, it is not always possible to drop the blocking lock > before to sleep since you can break your critical path and free the > way for races of various genre. Even unlocking Giant, that is > auto-magically done by sleeping primitives, can lead to very difficult > to discover races (I can remind one in tty code, old of some months, > that can be a good proof-of-concept for that). > --HPS