[PATCH] lockdep/selftest: Add tests for the mixed read-write case.

[PATCH] lockdep/selftest: Add tests for the mixed read-write case.

[Maarten Lankhorst]

rlock_AA5 will pass unexpectedly, marking the test as FAILED and breaking lockdep for everyone.

The following test should make lockdep complain, but currently doesn't:

lock(a); readlock(x);

writelock(x); lock(a);

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---

diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index 8a1a230..d0d2540 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -301,6 +301,51 @@ static void rsem_AA3(void)
 	RSL(X2); // this one should fail
 }
 
+static void rlock_AA4(void)
+{
+	ML(A);
+	RL(X1);
+	RU(X1);
+	MU(A);
+
+	RL(X1);
+	ML(A);
+}
+
+static void rsem_AA4(void)
+{
+	ML(A);
+	RSL(X1);
+	RSU(X1);
+	MU(A);
+
+	RSL(X1);
+	ML(A);
+}
+
+static void rlock_AA5(void)
+{
+	ML(A);
+	RL(X1);
+	RU(X1);
+	MU(A);
+
+	WL(X1);
+	ML(A);
+}
+
+static void rsem_AA5(void)
+{
+	ML(A);
+	RSL(X1);
+	RSU(X1);
+	MU(A);
+
+	WSL(X1);
+	ML(A);
+}
+
+
 /*
  * ABBA deadlock:
  */
@@ -1661,6 +1706,21 @@ void locking_selftest(void)
 	dotest(rsem_AA3, FAILURE, LOCKTYPE_RWSEM);
 	printk("\n");
 
+	print_testname("mixed normal-read-lock");
+	printk("             |");
+	dotest(rlock_AA4, SUCCESS, LOCKTYPE_RWLOCK);
+	printk("             |");
+	dotest(rsem_AA4, FAILURE, LOCKTYPE_RWSEM);
+	printk("\n");
+
+	print_testname("mixed normal-r/w-lock");
+	printk("             |");
+	dotest(rlock_AA5, FAILURE, LOCKTYPE_RWLOCK);
+	printk("             |");
+	dotest(rsem_AA5, FAILURE, LOCKTYPE_RWSEM);
+	printk("\n");
+
+
 	printk("  --------------------------------------------------------------------------\n");
 
 	/*

Re: [PATCH] lockdep/selftest: Add tests for the mixed read-write case.

[Daniel Vetter]

On Tue, Mar 26, 2013 at 5:14 PM, Maarten Lankhorst
<maarten.lankhorst@canonical.com> wrote:
> rlock_AA5 will pass unexpectedly, marking the test as FAILED and breaking lockdep for everyone.
>
> The following test should make lockdep complain, but currently doesn't:
>
> lock(a); readlock(x);
>
> writelock(x); lock(a);
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

Note that workqueues use readlock annotations to supposed catch
deadlocks around flush_workqueue. But it doesn't really work, and
we've been bitten by that in drm/i915 pretty badly. See the commit
note in

commit b4a98e57fc27854b5938fc8b08b68e5e68b91e1f
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Nov 1 09:26:26 2012 +0000

    drm/i915: Flush outstanding unpin tasks before pageflipping

    If we accumulate unpin tasks because we are pageflipping faster than the
    system can schedule its workers, we can effectively create a
    pin-leak. The solution taken here is to limit the number of unpin tasks
    we have per-crtc and to flush those outstanding tasks if we accumulate
    too many. This should prevent any jitter in the normal case, and also
    prevent the hang if we should run too fast.

    Note: It is important that we switch from the system workqueue to our
    own dev_priv->wq since all work items on that queue are guaranteed to
    only need the dev->struct_mutex and not any modeset resources. For
    otherwise if we have a work item ahead in the queue which needs the
    modeset lock (like the output detect work used by both polling or
    hpd), this work and so the unpin work will never execute since the
    pageflip code already holds that lock. Unfortunately there's no
    lockdep support for this scenario in the workqueue code.

    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=46991
    Reported-and-tested-by: Tvrtko Ursulin <tvrtko.ursulin@onelan.co.uk>
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    [danvet: Added note about workqueu deadlock.]
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=56337
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Later investigations showed that there _is_ lockdep support, it just
doesn't work. Maarten's patch here boils the failure scenario down to
its essence.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch