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: <506BED25.2060804@vmware.com>
Date:	Wed, 03 Oct 2012 09:45:41 +0200
From:	Thomas Hellstrom <thellstrom@...are.com>
To:	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, Daniel Vetter <daniel@...ll.ch>
Subject: Re: [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER

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-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/