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/03/2012 10:53 AM, Daniel Vetter wrote:
On Wed, Oct 3, 2012 at 10:37 AM, Thomas Hellstrom <thellstrom@xxxxxxxxxx> wrote:
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?).
Hm, I have to admit that idea hasn't crossed my mind, but it's indeed
a hole in our current reservation lockdep annotations - since we're
blocking for the unreserve, other threads could potential block
waiting on us to release a lock we're holding already, resulting in a
deadlock.

Since no other locking primitive that I know of has this
wait_for_unlocked interface, I don't know how we could map this in
lockdep. One idea is to grab the lock and release it again immediately
(only in the annotations, not the real lock ofc). But I need to check
the lockdep code to see whether that doesn't trip it up.

I imagine doing the same as mutex_lock_interruptible() does in the
interrupted path should work...
It simply calls the unlock lockdep annotation function if it breaks
out. So doing a lock/unlock cycle in wait_unreserve should do what we
want.

And to properly annotate the ttm reserve paths we could just add an
unconditional wait_unreserve call at the beginning like you suggested
(maybe with #ifdef CONFIG_PROVE_LOCKING in case ppl freak out about
the added atomic read in the uncontended case).
-Daniel

I think atomic_read()s are cheap, at least on intel as IIRC they don't require bus locking,
still I think we should keep it within CONFIG_PROVE_LOCKING

which btw reminds me there's an optimization that can be done in that one should really only call atomic_cmpxchg() if a preceding atomic_read() hints that it will succeed.

Now, does this mean TTM can keep the atomic reserve <-> lru list removal?

Thanks,
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