Date: Sun, 9 Sep 2012 16:03:14 +0100 From: Attilio Rao <attilio@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, Davide Italiano <davide@freebsd.org>, src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern Message-ID: <CAJ-FndARWZGwdiLeaQcnM%2BM%2Bm8zmBLuMrTkgoCFeesXPR=08pA@mail.gmail.com> In-Reply-To: <CAJ-FndD5EO12xsWOAe6u0EvX00q33wxO4OivnGjzj0=T2Oe8uA@mail.gmail.com> References: <201207301350.q6UDobCI099069@svn.freebsd.org> <CAJ-FndBj8tpC_BJXs_RH8sG2TBG8yA=Lxu3-GTVT9Ap_zOCuVQ@mail.gmail.com> <CAJ-FndDnO7wjnWPV0tTu%2BUGHjsxa3YDarMxmyei3ZmjLAFvRkQ@mail.gmail.com> <201207301732.33474.jhb@freebsd.org> <CAJ-FndD5EO12xsWOAe6u0EvX00q33wxO4OivnGjzj0=T2Oe8uA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 8/2/12, Attilio Rao <attilio@freebsd.org> wrote:
> On 7/30/12, John Baldwin <jhb@freebsd.org> wrote:
[ trimm ]
>> --- //depot/projects/smpng/sys/kern/subr_turnstile.c 2012-06-04
>> 18:27:32.000000000 0000
>> +++ //depot/user/jhb/lock/kern/subr_turnstile.c 2012-06-05
>> 00:27:57.000000000 0000
>> @@ -684,6 +684,7 @@
>> if (owner)
>> MPASS(owner->td_proc->p_magic == P_MAGIC);
>> MPASS(queue == TS_SHARED_QUEUE || queue == TS_EXCLUSIVE_QUEUE);
>> + KASSERT(!TD_IS_IDLETHREAD(td), ("idle threads cannot block on locks"));
>>
>> /*
>> * If the lock does not already have a turnstile, use this thread's
>
> I'm wondering if we should also use similar checks in places doing
> adaptive spinning (including the TD_NO_SLEEPING check). Likely yes.
So what do you think about this?
Thanks,
Attilio
In r239585 some further sanity checks to idlethreads not sleeping or blocking
have been added.
However, adaptive spinning on locks is not covered by this and it really
should be, as it is operationally equivalent to blocking or sleeping on the
primitive.
---
sys/kern/kern_lock.c | 4 ++++
sys/kern/kern_mutex.c | 2 ++
sys/kern/kern_rwlock.c | 4 ++++
sys/kern/kern_sx.c | 4 ++++
4 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/sys/kern/kern_lock.c b/sys/kern/kern_lock.c
index 24526b0..a771daa 100644
--- a/sys/kern/kern_lock.c
+++ b/sys/kern/kern_lock.c
@@ -565,6 +565,8 @@ __lockmgr_args(struct lock *lk, u_int flags,
struct lock_object *ilk,
CTR3(KTR_LOCK,
"%s: spinning on %p held by %p",
__func__, lk, owner);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("idle threads cannot sleep on locks"));
/*
* If we are holding also an interlock drop it
@@ -784,6 +786,8 @@ __lockmgr_args(struct lock *lk, u_int flags,
struct lock_object *ilk,
CTR3(KTR_LOCK,
"%s: spinning on %p held by %p",
__func__, lk, owner);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("idle threads cannot sleep on locks"));
/*
* If we are holding also an interlock drop it
diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c
index f718ca0..5a665c0 100644
--- a/sys/kern/kern_mutex.c
+++ b/sys/kern/kern_mutex.c
@@ -397,6 +397,8 @@ _mtx_lock_sleep(struct mtx *m, uintptr_t tid, int
opts, const char *file,
CTR3(KTR_LOCK,
"%s: spinning on %p held by %p",
__func__, m, owner);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("idle threads cannot block on locks"));
while (mtx_owner(m) == owner &&
TD_IS_RUNNING(owner)) {
cpu_spinwait();
diff --git a/sys/kern/kern_rwlock.c b/sys/kern/kern_rwlock.c
index c337041..3ef14d6 100644
--- a/sys/kern/kern_rwlock.c
+++ b/sys/kern/kern_rwlock.c
@@ -391,6 +391,8 @@ _rw_rlock(struct rwlock *rw, const char *file, int line)
CTR3(KTR_LOCK,
"%s: spinning on %p held by %p",
__func__, rw, owner);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("idle threads cannot block on locks"));
while ((struct thread*)RW_OWNER(rw->rw_lock) ==
owner && TD_IS_RUNNING(owner)) {
cpu_spinwait();
@@ -713,6 +715,8 @@ _rw_wlock_hard(struct rwlock *rw, uintptr_t tid,
const char *file, int line)
if (LOCK_LOG_TEST(&rw->lock_object, 0))
CTR3(KTR_LOCK, "%s: spinning on %p held by %p",
__func__, rw, owner);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("idle threads cannot block on locks"));
while ((struct thread*)RW_OWNER(rw->rw_lock) == owner &&
TD_IS_RUNNING(owner)) {
cpu_spinwait();
diff --git a/sys/kern/kern_sx.c b/sys/kern/kern_sx.c
index bcd7acd..176ff9c 100644
--- a/sys/kern/kern_sx.c
+++ b/sys/kern/kern_sx.c
@@ -551,6 +551,8 @@ _sx_xlock_hard(struct sx *sx, uintptr_t tid, int
opts, const char *file,
CTR3(KTR_LOCK,
"%s: spinning on %p held by %p",
__func__, sx, owner);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("idle threads cannot
sleep on locks"));
GIANT_SAVE();
while (SX_OWNER(sx->sx_lock) == x &&
TD_IS_RUNNING(owner)) {
@@ -840,6 +842,8 @@ _sx_slock_hard(struct sx *sx, int opts, const char
*file, int line)
CTR3(KTR_LOCK,
"%s: spinning on %p held by %p",
__func__, sx, owner);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("idle threads cannot sleep on locks"));
GIANT_SAVE();
while (SX_OWNER(sx->sx_lock) == x &&
TD_IS_RUNNING(owner)) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndARWZGwdiLeaQcnM%2BM%2Bm8zmBLuMrTkgoCFeesXPR=08pA>