Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/external/bsd/drm2/include/linux Fix mistakes in Linux ww...
details: https://anonhg.NetBSD.org/src/rev/7281d6593f82
branches: trunk
changeset: 332318:7281d6593f82
user: riastradh <riastradh%NetBSD.org@localhost>
date: Mon Sep 15 20:24:55 2014 +0000
description:
Fix mistakes in Linux ww_mutex code.
I wrote some cases for the WW_WANTOWN state forgetting that there is
an acquisition context associated with it.
- On acceptance of a lock, allow WW_WANTOWN as well as WW_CTX.
- On unlock when WW_WANTOWN, decrement the context's acquire count.
Fixes KASSERT(ctx->wwx_acquired == 0) failure in ww_acquire_fini, and
would fix unlikely but possible kasserts in ww_mutex_lock* if another
lwp swoops in to lock without a context quickly enough.
While here, add some comments and kassert up the wazoo.
diffstat:
sys/external/bsd/drm2/include/linux/ww_mutex.h | 150 ++++++++++++++++++++----
1 files changed, 124 insertions(+), 26 deletions(-)
diffs (truncated from 387 to 300 lines):
diff -r a3e709203cb5 -r 7281d6593f82 sys/external/bsd/drm2/include/linux/ww_mutex.h
--- a/sys/external/bsd/drm2/include/linux/ww_mutex.h Mon Sep 15 19:30:16 2014 +0000
+++ b/sys/external/bsd/drm2/include/linux/ww_mutex.h Mon Sep 15 20:24:55 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ww_mutex.h,v 1.6 2014/09/13 00:33:45 riastradh Exp $ */
+/* $NetBSD: ww_mutex.h,v 1.7 2014/09/15 20:24:55 riastradh Exp $ */
/*-
* Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -47,6 +47,7 @@
struct ww_acquire_ctx {
struct ww_class *wwx_class __diagused;
+ struct lwp *wwx_owner __diagused;
uint64_t wwx_ticket;
unsigned wwx_acquired;
bool wwx_acquire_done;
@@ -92,6 +93,7 @@
{
ctx->wwx_class = class;
+ ctx->wwx_owner = curlwp;
ctx->wwx_ticket = atomic_inc_64_nv(&class->wwc_ticket);
ctx->wwx_acquired = 0;
ctx->wwx_acquire_done = false;
@@ -101,6 +103,9 @@
ww_acquire_done(struct ww_acquire_ctx *ctx)
{
+ KASSERTMSG((ctx->wwx_owner == curlwp),
+ "ctx %p owned by %p, not self (%p)", ctx, ctx->wwx_owner, curlwp);
+
ctx->wwx_acquire_done = true;
}
@@ -108,18 +113,22 @@
ww_acquire_fini(struct ww_acquire_ctx *ctx)
{
+ KASSERTMSG((ctx->wwx_owner == curlwp),
+ "ctx %p owned by %p, not self (%p)", ctx, ctx->wwx_owner, curlwp);
KASSERTMSG((ctx->wwx_acquired == 0), "ctx %p still holds %u locks",
ctx, ctx->wwx_acquired);
+
ctx->wwx_acquired = ~0U; /* Fail if called again. */
+ ctx->wwx_owner = NULL;
}
struct ww_mutex {
kmutex_t wwm_lock;
enum ww_mutex_state {
- WW_UNLOCKED,
- WW_OWNED,
- WW_CTX,
- WW_WANTOWN,
+ WW_UNLOCKED, /* nobody owns it */
+ WW_OWNED, /* owned by a lwp without a context */
+ WW_CTX, /* owned by a context */
+ WW_WANTOWN, /* owned by ctx, waiters w/o ctx waiting */
} wwm_state;
union {
struct lwp *owner;
@@ -218,7 +227,9 @@
KASSERT(mutex_owned(&mutex->wwm_lock));
- KASSERT(mutex->wwm_state == WW_CTX);
+ KASSERT((mutex->wwm_state == WW_CTX) ||
+ (mutex->wwm_state == WW_WANTOWN));
+ KASSERT(mutex->wwm_u.ctx != ctx);
KASSERTMSG((ctx->wwx_class == mutex->wwm_u.ctx->wwx_class),
"ww mutex class mismatch: %p != %p",
ctx->wwx_class, mutex->wwm_u.ctx->wwx_class);
@@ -233,7 +244,9 @@
ctx->wwx_ticket, ctx, collision->wwx_ticket, collision);
do cv_wait(&mutex->wwm_cv, &mutex->wwm_lock);
- while (!((mutex->wwm_state == WW_CTX) && (mutex->wwm_u.ctx == ctx)));
+ while (!(((mutex->wwm_state == WW_CTX) ||
+ (mutex->wwm_state == WW_WANTOWN)) &&
+ (mutex->wwm_u.ctx == ctx)));
rb_tree_remove_node(&mutex->wwm_waiters, ctx);
}
@@ -246,7 +259,9 @@
KASSERT(mutex_owned(&mutex->wwm_lock));
- KASSERT(mutex->wwm_state == WW_CTX);
+ KASSERT((mutex->wwm_state == WW_CTX) ||
+ (mutex->wwm_state == WW_WANTOWN));
+ KASSERT(mutex->wwm_u.ctx != ctx);
KASSERTMSG((ctx->wwx_class == mutex->wwm_u.ctx->wwx_class),
"ww mutex class mismatch: %p != %p",
ctx->wwx_class, mutex->wwm_u.ctx->wwx_class);
@@ -265,7 +280,9 @@
ret = -cv_wait_sig(&mutex->wwm_cv, &mutex->wwm_lock);
if (ret)
goto out;
- } while (!((mutex->wwm_state == WW_CTX) && (mutex->wwm_u.ctx == ctx)));
+ } while (!(((mutex->wwm_state == WW_CTX) ||
+ (mutex->wwm_state == WW_WANTOWN)) &&
+ (mutex->wwm_u.ctx == ctx)));
out: rb_tree_remove_node(&mutex->wwm_waiters, ctx);
return ret;
@@ -283,13 +300,16 @@
break;
case WW_OWNED:
KASSERTMSG((mutex->wwm_u.owner != curlwp),
- "locking against myself: %p", curlwp);
+ "locking %p against myself: %p", mutex, curlwp);
ww_mutex_state_wait(mutex, WW_OWNED);
goto retry;
case WW_CTX:
KASSERT(mutex->wwm_u.ctx != NULL);
mutex->wwm_state = WW_WANTOWN;
+ /* FALLTHROUGH */
case WW_WANTOWN:
+ KASSERTMSG((mutex->wwm_u.ctx->wwx_owner != curlwp),
+ "locking %p against myself: %p", mutex, curlwp);
ww_mutex_state_wait(mutex, WW_WANTOWN);
goto retry;
default:
@@ -314,7 +334,7 @@
break;
case WW_OWNED:
KASSERTMSG((mutex->wwm_u.owner != curlwp),
- "locking against myself: %p", curlwp);
+ "locking %p against myself: %p", mutex, curlwp);
ret = ww_mutex_state_wait_sig(mutex, WW_OWNED);
if (ret)
goto out;
@@ -322,7 +342,10 @@
case WW_CTX:
KASSERT(mutex->wwm_u.ctx != NULL);
mutex->wwm_state = WW_WANTOWN;
+ /* FALLTHROUGH */
case WW_WANTOWN:
+ KASSERTMSG((mutex->wwm_u.ctx->wwx_owner != curlwp),
+ "locking %p against myself: %p", mutex, curlwp);
ret = ww_mutex_state_wait_sig(mutex, WW_WANTOWN);
if (ret)
goto out;
@@ -349,7 +372,15 @@
return 0;
}
- KASSERT(!ctx->wwx_acquire_done);
+ KASSERTMSG((ctx->wwx_owner == curlwp),
+ "ctx %p owned by %p, not self (%p)", ctx, ctx->wwx_owner, curlwp);
+ KASSERTMSG(!ctx->wwx_acquire_done,
+ "ctx %p done acquiring locks, can't acquire more", ctx);
+ KASSERTMSG((ctx->wwx_acquired != ~0U),
+ "ctx %p finished, can't be used any more", ctx);
+ KASSERTMSG((ctx->wwx_class == mutex->wwm_class),
+ "ctx %p in class %p, mutex %p in class %p",
+ ctx, ctx->wwx_class, mutex, mutex->wwm_class);
mutex_enter(&mutex->wwm_lock);
retry: switch (mutex->wwm_state) {
@@ -359,7 +390,7 @@
goto locked;
case WW_OWNED:
KASSERTMSG((mutex->wwm_u.owner != curlwp),
- "locking against myself: %p", curlwp);
+ "locking %p against myself: %p", mutex, curlwp);
ww_mutex_state_wait(mutex, WW_OWNED);
goto retry;
case WW_CTX:
@@ -373,6 +404,8 @@
}
KASSERT(mutex->wwm_state == WW_CTX);
KASSERT(mutex->wwm_u.ctx != NULL);
+ KASSERT((mutex->wwm_u.ctx == ctx) ||
+ (mutex->wwm_u.ctx->wwx_owner != curlwp));
if (mutex->wwm_u.ctx == ctx) {
/*
* We already own it. Yes, this can happen correctly
@@ -400,7 +433,8 @@
ww_mutex_lock_wait(mutex, ctx);
}
locked: ctx->wwx_acquired++;
- KASSERT(mutex->wwm_state == WW_CTX);
+ KASSERT((mutex->wwm_state == WW_CTX) ||
+ (mutex->wwm_state == WW_WANTOWN));
KASSERT(mutex->wwm_u.ctx == ctx);
mutex_exit(&mutex->wwm_lock);
return 0;
@@ -416,7 +450,15 @@
if (ctx == NULL)
return ww_mutex_lock_noctx_sig(mutex);
- KASSERT(!ctx->wwx_acquire_done);
+ KASSERTMSG((ctx->wwx_owner == curlwp),
+ "ctx %p owned by %p, not self (%p)", ctx, ctx->wwx_owner, curlwp);
+ KASSERTMSG(!ctx->wwx_acquire_done,
+ "ctx %p done acquiring locks, can't acquire more", ctx);
+ KASSERTMSG((ctx->wwx_acquired != ~0U),
+ "ctx %p finished, can't be used any more", ctx);
+ KASSERTMSG((ctx->wwx_class == mutex->wwm_class),
+ "ctx %p in class %p, mutex %p in class %p",
+ ctx, ctx->wwx_class, mutex, mutex->wwm_class);
mutex_enter(&mutex->wwm_lock);
retry: switch (mutex->wwm_state) {
@@ -426,7 +468,7 @@
goto locked;
case WW_OWNED:
KASSERTMSG((mutex->wwm_u.owner != curlwp),
- "locking against myself: %p", curlwp);
+ "locking %p against myself: %p", mutex, curlwp);
ret = ww_mutex_state_wait_sig(mutex, WW_OWNED);
if (ret)
goto out;
@@ -444,6 +486,8 @@
}
KASSERT(mutex->wwm_state == WW_CTX);
KASSERT(mutex->wwm_u.ctx != NULL);
+ KASSERT((mutex->wwm_u.ctx == ctx) ||
+ (mutex->wwm_u.ctx->wwx_owner != curlwp));
if (mutex->wwm_u.ctx == ctx) {
/*
* We already own it. Yes, this can happen correctly
@@ -472,7 +516,8 @@
if (ret)
goto out;
}
-locked: KASSERT(mutex->wwm_state == WW_CTX);
+locked: KASSERT((mutex->wwm_state == WW_CTX) ||
+ (mutex->wwm_state == WW_WANTOWN));
KASSERT(mutex->wwm_u.ctx == ctx);
ctx->wwx_acquired++;
ret = 0;
@@ -491,7 +536,18 @@
return;
}
- KASSERT(!ctx->wwx_acquire_done);
+ KASSERTMSG((ctx->wwx_owner == curlwp),
+ "ctx %p owned by %p, not self (%p)", ctx, ctx->wwx_owner, curlwp);
+ KASSERTMSG(!ctx->wwx_acquire_done,
+ "ctx %p done acquiring locks, can't acquire more", ctx);
+ KASSERTMSG((ctx->wwx_acquired != ~0U),
+ "ctx %p finished, can't be used any more", ctx);
+ KASSERTMSG((ctx->wwx_acquired == 0),
+ "ctx %p still holds %u locks, not allowed in slow path",
+ ctx, ctx->wwx_acquired);
+ KASSERTMSG((ctx->wwx_class == mutex->wwm_class),
+ "ctx %p in class %p, mutex %p in class %p",
+ ctx, ctx->wwx_class, mutex, mutex->wwm_class);
mutex_enter(&mutex->wwm_lock);
retry: switch (mutex->wwm_state) {
@@ -501,7 +557,7 @@
goto locked;
case WW_OWNED:
KASSERTMSG((mutex->wwm_u.owner != curlwp),
- "locking against myself: %p", curlwp);
+ "locking %p against myself: %p", mutex, curlwp);
ww_mutex_state_wait(mutex, WW_OWNED);
goto retry;
case WW_CTX:
@@ -515,12 +571,15 @@
}
KASSERT(mutex->wwm_state == WW_CTX);
KASSERT(mutex->wwm_u.ctx != NULL);
+ KASSERTMSG((mutex->wwm_u.ctx->wwx_owner != curlwp),
+ "locking %p against myself: %p", mutex, curlwp);
/*
* Owned by another party, of any priority. Ask that party to
* wake us when it's done.
*/
ww_mutex_lock_wait(mutex, ctx);
-locked: KASSERT(mutex->wwm_state == WW_CTX);
+locked: KASSERT((mutex->wwm_state == WW_CTX) ||
+ (mutex->wwm_state == WW_WANTOWN));
KASSERT(mutex->wwm_u.ctx == ctx);
ctx->wwx_acquired++;
mutex_exit(&mutex->wwm_lock);
@@ -537,7 +596,18 @@
if (ctx == NULL)
return ww_mutex_lock_noctx_sig(mutex);
- KASSERT(!ctx->wwx_acquire_done);
+ KASSERTMSG((ctx->wwx_owner == curlwp),
+ "ctx %p owned by %p, not self (%p)", ctx, ctx->wwx_owner, curlwp);
+ KASSERTMSG(!ctx->wwx_acquire_done,
+ "ctx %p done acquiring locks, can't acquire more", ctx);
+ KASSERTMSG((ctx->wwx_acquired != ~0U),
+ "ctx %p finished, can't be used any more", ctx);
+ KASSERTMSG((ctx->wwx_acquired == 0),
+ "ctx %p still holds %u locks, not allowed in slow path",
+ ctx, ctx->wwx_acquired);
+ KASSERTMSG((ctx->wwx_class == mutex->wwm_class),
+ "ctx %p in class %p, mutex %p in class %p",
+ ctx, ctx->wwx_class, mutex, mutex->wwm_class);
mutex_enter(&mutex->wwm_lock);
retry: switch (mutex->wwm_state) {
Home |
Main Index |
Thread Index |
Old Index