Johannes Stezenbach wrote: > On Mon, Mar 05, 2007 at 06:19:01PM +0100, Oliver Endriss wrote: > > > > From 'Linux Device Drivers' (replace 'down' by 'mutex_lock'): > > | ... > > | down decrements the value of the semaphore and waits as long as need > > | be. down_ interruptible does the same, but the operation is > > | interruptible. The interruptible version is almost always the one you > > | will want; it allows a user-space process that is waiting on a > > | semaphore to be interrupted by the user. You do not, as a general > > ^^^^^^^^^^^^^^^^^^^^^^^^ > > | rule, want to use noninterruptible operations unless there truly is no > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > | alternative. Noninterruptible operations are a good way to create > > ^^^^^^^^^^^ > > | unkillable processes (the dreaded D state seen in ps), and annoy > > | your users. Using down_interruptible requires some extra care, > > | however, if the operation is interrupted, the function returns a > > | nonzero value, and the caller does not hold the semaphore. Proper use > > | of down_interruptible requires always checking the return value and > > | responding accordingly. > > | ... > > I don't think this advice applies to mutexes (at least if the > mutexes are used in the usual way to protect some common data). > > For event semaphores, you block waiting for an event, and if > the event doesn't happen (maybe you wait for an irq), you need > a way out or you're screwed. So using down_interruptible() > is what you want. > > Mutexes, however, are supposed to be held only while manipulating > some shared data. So the code looks *always* like this: > > mutex_lock(&mtx); > do_something_with_shared_data(); > mutex_unlock(&mtx); > > Now if some process sleeps uninterruptibly in mutex_lock(), > some other process holds the mutex and sleeps in do_something(). > If it wedges up, you either have some locking bug > (someone forgot a mutex_unlock()), or it wedged up in > do_something(), and you got to fix *that*. Well, your example is simple. Locking is easy if there is only one mutex involved. But there are a lot of mutexes/spinlocks/event semaphores in various layers of the subsystem. I simply cannot tell whether a deadlock may happen under some rare conditions or not... > mutex_unlock_interruptible() would only help to paper over > locking bugs, and its use in dvb-core comes from a straight > conversion from down_interruptible(), which itself was used > because it was once useful during development. How that the > signal handling was found to be buggy I think it's much better > to use mutex_lock() instead of fixing the mutex_unlock_interruptible() > usage. > > BTW, some statistics: > > linux-2.6.20$ grep -r '\<mutex_lock\>' . | wc -l > 3080 > linux-2.6.20$ grep -r '\<mutex_lock_interruptible\>' . | wc -l > 236 Ok, to make it short: Please proceed and apply this patch, or convert all occurrences of mutex_lock_interruptible() to mutex_lock() if you prefer. dvb_core is not my working area. ;-) Oliver -- -------------------------------------------------------- VDR Remote Plugin 0.3.9 available at http://www.escape-edv.de/endriss/vdr/ -------------------------------------------------------- _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
