Re: [PATCH v4 07/12] drm/i915: GEM operations need to be done under the big lock

[Chris Wilson <chris@chris-wilson.co.uk>]

On Tue, Feb 02, 2016 at 02:46:19PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> VMA creation and GEM list management need the big lock.
> 
> v2:
> 
> Mutex unlock ended on the wrong path somehow. (0-day, Julia Lawall)
> 
> Not to mention drm_gem_object_unreference was there in existing
> code with no mutex held.
> 
> v3:
> 
> Some callers of i915_gem_object_create_stolen_for_preallocated
> already hold the lock so move the mutex into the other caller
> as well.
> 
> v4:
> 
> Changed to lockdep_assert_held. (Chris Wilson)

To appease Daniel, I was thinking could we smarten up

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index c57e424d914b..e10da10e0033 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -422,8 +422,19 @@ struct lock_class_key { };
 
 #define lockdep_depth(tsk)     (0)
 
+#ifdef CONFIG_POOR_MANS_LOCKDEP
+#define __lockdep_check(l) \
+       ({ __builtin_types_compatible_p(typeof(*l), struct mutex) ? \
+        mutex_is_locked((struct mutex *)l) : \
+        __builtin_types_compatible_p(typeof(*l), spinlock_t) ? \
+        spin_is_locked((spinlock_t *)l) : \
+        0; })
+#define lockdep_assert_held(l) do { WARN_ON(!__lockdep_check(l)); } while (0)
+#define lockdep_assert_held_once(l) do { WARN_ON_ONCE(!__lockdep_check(l)); } while (0)
+#else
 #define lockdep_assert_held(l)                 do { (void)(l); } while (0)
 #define lockdep_assert_held_once(l)            do { (void)(l); } while (0)
+#endif
 
 #define lockdep_recursing(tsk)                 (0)
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8c15b29d5adc..9f96341f8773 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1025,6 +1025,17 @@ config LOCKDEP
        select KALLSYMS
        select KALLSYMS_ALL
 
+config POOR_MANS_LOCKDEP
+       bool "Lock debugging: cheap warning upon forgotten locks"
+       depends on !LOCKDEP
+       default n
+       help
+        This feature enables the kernel to WARN if it detects an inconsistency
+        in the locking. It is vastly less capable than PROVE_LOCKING but only
+        has a small runtime penalty. It may be useful for situations where
+        the overhead of lockdep is too great, but information can be gleaned
+        from the noise of enabling a WARN widescale.
+
 config LOCK_STAT
        bool "Lock usage statistics"
        depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT


into something that may fly upstream? Then we do get accurate lockdep
warnings, and we could even enable the WARNs by distro-default (instead of
lockdep).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx