Op 28-05-13 21:18, Daniel Vetter schreef:
On Tue, May 28, 2013 at 04:48:45PM +0200, Maarten Lankhorst wrote:
This stresses the lockdep code in some ways specifically useful to ww_mutexes. It adds checks for most of the common locking errors.
Changes since v1:
- Add tests to verify reservation_id is untouched.
- Use L() and U() macros where possible.
Changes since v2:
- Use the ww_mutex api directly.
- Use macros for most of the code.
Changes since v3:
- Rework tests for the api changes.
<snip>
+static void ww_test_normal(void) +{
- int ret;
- WWAI(&t);
- /*
* test if ww_id is kept identical if not
* called with any of the ww_* locking calls
*/
- /* mutex_lock (and indirectly, mutex_lock_nested) */
- o.ctx = (void *)~0UL;
- mutex_lock(&o.base);
- mutex_unlock(&o.base);
- WARN_ON(o.ctx != (void *)~0UL);
- /* mutex_lock_interruptible (and *_nested) */
- o.ctx = (void *)~0UL;
- ret = mutex_lock_interruptible(&o.base);
- if (!ret)
mutex_unlock(&o.base);
- else
WARN_ON(1);
- WARN_ON(o.ctx != (void *)~0UL);
- /* mutex_lock_killable (and *_nested) */
- o.ctx = (void *)~0UL;
- ret = mutex_lock_killable(&o.base);
- if (!ret)
mutex_unlock(&o.base);
- else
WARN_ON(1);
- WARN_ON(o.ctx != (void *)~0UL);
- /* trylock, succeeding */
- o.ctx = (void *)~0UL;
- ret = mutex_trylock(&o.base);
- WARN_ON(!ret);
- if (ret)
mutex_unlock(&o.base);
- else
WARN_ON(1);
- WARN_ON(o.ctx != (void *)~0UL);
- /* trylock, failing */
- o.ctx = (void *)~0UL;
- mutex_lock(&o.base);
- ret = mutex_trylock(&o.base);
- WARN_ON(ret);
- mutex_unlock(&o.base);
- WARN_ON(o.ctx != (void *)~0UL);
- /* nest_lock */
- o.ctx = (void *)~0UL;
- mutex_lock_nest_lock(&o.base, &t);
- mutex_unlock(&o.base);
- WARN_ON(o.ctx != (void *)~0UL);
+}
Since we don't really allow this any more (instead allow ww_mutex_lock without context) do we need this test here really?
Yes. This test verifies all the normal locking paths are not affected by the ww_ctx changes.
+static void ww_test_two_contexts(void) +{
- WWAI(&t);
- WWAI(&t2);
+}
+static void ww_test_context_unlock_twice(void) +{
- WWAI(&t);
- WWAD(&t);
- WWAF(&t);
- WWAF(&t);
+}
+static void ww_test_object_unlock_twice(void) +{
- WWL1(&o);
- WWU(&o);
- WWU(&o);
+}
+static void ww_test_spin_nest_unlocked(void) +{
- raw_spin_lock_nest_lock(&lock_A, &o.base);
- U(A);
+}
I don't quite see the point of this one here ...
It's a lockdep test that was missing. o.base is not locked. So lock_A is being nested into an unlocked lock, resulting in a lockdep error.
+static void ww_test_unneeded_slow(void) +{
- int ret;
- WWAI(&t);
- ww_mutex_lock_slow(&o, &t);
+}
I think checking the _slow debug stuff would be neat, i.e.
- fail/success tests for properly unlocking all held locks
- fail/success tests for lock_slow acquiring the right lock.
Otherwise I didn't spot anything that seems missing in these self-tests here.
Yes it would be nice, doing so is left as an excercise for the reviewer, who failed to raise this point sooner. ;-)
~Maarten