linux-kernel - Re: [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <506C190E.5050803@vmware.com>
Date:	Wed, 03 Oct 2012 12:53:02 +0200
From:	Thomas Hellstrom <thellstrom@...are.com>
To:	Daniel Vetter <daniel@...ll.ch>
CC:	Maarten Lankhorst <maarten.lankhorst@...onical.com>,
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	linaro-mm-sig@...ts.linaro.org, sumit.semwal@...aro.org,
	linux-media@...r.kernel.org
Subject: Re: [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER

On 10/03/2012 10:53 AM, Daniel Vetter wrote:
> On Wed, Oct 3, 2012 at 10:37 AM, Thomas Hellstrom <thellstrom@...are.com> 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/