Re: [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/02/2012 10:03 AM, Daniel Vetter wrote:
On Tue, Oct 02, 2012 at 08:46:32AM +0200, Thomas Hellstrom wrote:
On 10/01/2012 11:47 AM, Maarten Lankhorst wrote:
I was doing a evil hack where I 'released' lru_lock to lockdep before doing the annotation
for a blocking acquire, and left trylock annotations as they were. This made lockdep do the
right thing.
I've never looked into how lockdep works. Is this something that can
be done permanently or just for testing
purposes? Although not related to this, is it possible to do
something similar to the trylock reversal in the
fault() code where mmap_sem() and reserve() change order using a
reserve trylock?
lockdep just requires a bunch of annotations, is a compile-time configure
option CONFIG_PROVE_LOCKING and if disabled, has zero overhead. And it's
rather awesome in detected deadlocks and handling crazy locking schemes
correctly:
- correctly handles trylocks
- correctly handles nested locking (i.e. grabbing a global lock, then
   grabbing subordinate locks in an unordered sequence since the global
   lock ensures that no deadlocks can happen).
- any kinds of inversions with special contexts like hardirq, softirq
- same for page-reclaim, i.e. it will yell if you could (potentially)
   deadlock because your shrinker grabs a lock that you hold while calling
   kmalloc.
- there are special annotates for various subsystems, e.g. to check for
   del_timer_sync vs. locks held by that timer. Or the console_lock
   annotations I've just recently submitted.
- all that with a really flexible set of annotation primitives that afaics
   should work for almost any insane locking scheme. The fact that Maarten
   could come up with proper reservation annotations without any changes to
   lockdep testifies this (he only had to fix a tiny thing to make it a bit
   more strict in a corner case).

In short I think it's made of awesome. The only downside is that it lacks
documentation, you have to read the code to understand it :(

The reason I've suggested to Maarten to abolish the trylock_reservation
within the lru_lock is that in that way lockdep only ever sees the
trylock, and hence is less strict about complainig about deadlocks. But
semantically it's an unconditional reserve. Maarten had some horrible
hacks that leaked the lockdep annotations out of the new reservation code,
which allowed ttm to be properly annotated.  But those also reduced the
usefulness for any other users of the reservation code, and so Maarten
looked into whether he could remove that trylock dance in ttm.

Imo having excellent lockdep support for cross-device reservations is a
requirment, and ending up with less strict annotations for either ttm
based drivers or other drivers is not good. And imo the ugly layering that
Maarten had in his first proof-of-concept also indicates that something is
amiss in the design.


So if I understand you correctly, the reservation changes in TTM are motivated by the
fact that otherwise, in the generic reservation code, lockdep can only be
annotated for a trylock and not a waiting lock, when it *is* in fact a waiting lock.

I'm completely unfamiliar with setting up lockdep annotations, but the only place a deadlock might occur is if the trylock fails and we do a wait_for_unreserve(). Isn't it possible to annotate the call to wait_for_unreserve() just like an interruptible waiting lock
(that is always interrupted, but at least any deadlock will be catched?).

/Thomas


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux