svn commit: r238907 - projects/calloutng/sys/kern
Attilio Rao
attilio at freebsd.org
Mon Sep 10 02:07:21 UTC 2012
On Sun, Sep 9, 2012 at 8:15 PM, John Baldwin <jhb at freebsd.org> wrote:
> On 9/9/12 11:03 AM, Attilio Rao wrote:
>> On 8/2/12, Attilio Rao <attilio at freebsd.org> wrote:
>>> On 7/30/12, John Baldwin <jhb at 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?
>
> This is isn't really good enough then. An idle thread should not
> acquire any lock that isn't a spin lock. Instead, you would be
> better off removing the assert I added above and adding an assert to
> mtx_lock(), rw_{rw}lock(), sx_{sx}lock(), lockmgr(), rm_{rw}lock() and
> all the try variants of those.
What do you think about these then?
Attilio
Subject: [PATCH 1/3] Tweak the commit message in case of panic for sleeping
from threads with TDP_NOSLEEPING on.
The current message has no informations on the thread and wchan
involed, which may prove to be useful in case where dumps have
mangled dwarf informations.
Reported by: kib
---
sys/kern/subr_sleepqueue.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sys/kern/subr_sleepqueue.c b/sys/kern/subr_sleepqueue.c
index b868289..69e0d36 100644
--- a/sys/kern/subr_sleepqueue.c
+++ b/sys/kern/subr_sleepqueue.c
@@ -297,7 +297,8 @@ sleepq_add(void *wchan, struct lock_object *lock,
const char *wmesg, int flags,
/* If this thread is not allowed to sleep, die a horrible death. */
KASSERT(!(td->td_pflags & TDP_NOSLEEPING),
- ("Trying sleep, but thread marked as sleeping prohibited"));
+ ("sleepq_add: td(%p) to sleep on wchan(%p) with TDP_NOSLEEPING on",
+ td, wchan));
/* Look up the sleep queue associated with the wait channel 'wchan'. */
sq = sleepq_lookup(wchan);
--
1.7.7.4
Subject: [PATCH 2/3] Tweak comments.
- Remove a sporious "with"
- Remove a comment which is no-longer true
---
sys/kern/subr_trap.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/sys/kern/subr_trap.c b/sys/kern/subr_trap.c
index 24960fd..13d273d1 100644
--- a/sys/kern/subr_trap.c
+++ b/sys/kern/subr_trap.c
@@ -153,7 +153,7 @@ userret(struct thread *td, struct trapframe *frame)
KASSERT((td->td_pflags & TDP_NOSLEEPING) == 0,
("userret: Returning with sleep disabled"));
KASSERT(td->td_pinned == 0,
- ("userret: Returning with with pinned thread"));
+ ("userret: Returning with pinned thread"));
#ifdef VIMAGE
/* Unfortunately td_vnet_lpush needs VNET_DEBUG. */
VNET_ASSERT(curvnet == NULL,
@@ -166,7 +166,6 @@ userret(struct thread *td, struct trapframe *frame)
/*
* Process an asynchronous software trap.
* This is relatively easy.
- * This function will return with preemption disabled.
*/
void
ast(struct trapframe *framep)
--
1.7.7.4
Subject: [PATCH 3/3] Improve check coverage about idle threads.
Idle threads are not allowed to acquire any lock but spinlocks.
Deny any attempt to do so by panicing at the locking operation
when INVARIANTS is on.
Discussed with: jhb
Signed-off-by: Attilio Rao <attilio at pcbsd-2488.(none)>
---
sys/kern/kern_lock.c | 4 ++++
sys/kern/kern_mutex.c | 6 ++++++
sys/kern/kern_rmlock.c | 6 ++++++
sys/kern/kern_rwlock.c | 13 +++++++++++++
sys/kern/kern_sx.c | 13 +++++++++++++
sys/kern/subr_turnstile.c | 1 -
6 files changed, 42 insertions(+), 1 deletions(-)
diff --git a/sys/kern/kern_lock.c b/sys/kern/kern_lock.c
index 24526b0..46a7567 100644
--- a/sys/kern/kern_lock.c
+++ b/sys/kern/kern_lock.c
@@ -478,6 +478,10 @@ __lockmgr_args(struct lock *lk, u_int flags,
struct lock_object *ilk,
("%s: LK_INTERLOCK passed without valid interlock @ %s:%d",
__func__, file, line));
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("lockmgr() by idle thread %p on lockmgr %s @ %s:%d",
+ curthread, lk->lock_object.lo_name, file, line));
+
class = (flags & LK_INTERLOCK) ? LOCK_CLASS(ilk) : NULL;
if (panicstr != NULL) {
if (flags & LK_INTERLOCK)
diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c
index f718ca0..9827a9f 100644
--- a/sys/kern/kern_mutex.c
+++ b/sys/kern/kern_mutex.c
@@ -201,6 +201,9 @@ _mtx_lock_flags(struct mtx *m, int opts, const
char *file, int line)
if (SCHEDULER_STOPPED())
return;
MPASS(curthread != NULL);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("mtx_lock() by idle thread %p on sleep mutex %s @ %s:%d",
+ curthread, m->lock_object.lo_name, file, line));
KASSERT(m->mtx_lock != MTX_DESTROYED,
("mtx_lock() of destroyed mutex @ %s:%d", file, line));
KASSERT(LOCK_CLASS(&m->lock_object) == &lock_class_mtx_sleep,
@@ -301,6 +304,9 @@ mtx_trylock_flags_(struct mtx *m, int opts, const
char *file, int line)
return (1);
MPASS(curthread != NULL);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("mtx_trylock() by idle thread %p on sleep mutex %s @ %s:%d",
+ curthread, m->lock_object.lo_name, file, line));
KASSERT(m->mtx_lock != MTX_DESTROYED,
("mtx_trylock() of destroyed mutex @ %s:%d", file, line));
KASSERT(LOCK_CLASS(&m->lock_object) == &lock_class_mtx_sleep,
diff --git a/sys/kern/kern_rmlock.c b/sys/kern/kern_rmlock.c
index 27d0462..ef1920b 100644
--- a/sys/kern/kern_rmlock.c
+++ b/sys/kern/kern_rmlock.c
@@ -498,6 +498,9 @@ void _rm_wlock_debug(struct rmlock *rm, const char
*file, int line)
if (SCHEDULER_STOPPED())
return;
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("rm_wlock() by idle thread %p on rmlock %s @ %s:%d",
+ curthread, rm->lock_object.lo_name, file, line));
WITNESS_CHECKORDER(&rm->lock_object, LOP_NEWORDER | LOP_EXCLUSIVE,
file, line, NULL);
@@ -540,6 +543,9 @@ _rm_rlock_debug(struct rmlock *rm, struct
rm_priotracker *tracker,
if (SCHEDULER_STOPPED())
return (1);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("rm_rlock() by idle thread %p on rmlock %s @ %s:%d",
+ curthread, rm->lock_object.lo_name, file, line));
if (!trylock && (rm->lock_object.lo_flags & RM_SLEEPABLE))
WITNESS_CHECKORDER(&rm->rm_lock_sx.lock_object, LOP_NEWORDER,
file, line, NULL);
diff --git a/sys/kern/kern_rwlock.c b/sys/kern/kern_rwlock.c
index c337041..e0be154 100644
--- a/sys/kern/kern_rwlock.c
+++ b/sys/kern/kern_rwlock.c
@@ -242,6 +242,9 @@ _rw_wlock(struct rwlock *rw, const char *file, int line)
if (SCHEDULER_STOPPED())
return;
MPASS(curthread != NULL);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("rw_wlock() by idle thread %p on rwlock %s @ %s:%d",
+ curthread, rw->lock_object.lo_name, file, line));
KASSERT(rw->rw_lock != RW_DESTROYED,
("rw_wlock() of destroyed rwlock @ %s:%d", file, line));
WITNESS_CHECKORDER(&rw->lock_object, LOP_NEWORDER | LOP_EXCLUSIVE, file,
@@ -260,6 +263,9 @@ _rw_try_wlock(struct rwlock *rw, const char *file, int line)
if (SCHEDULER_STOPPED())
return (1);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("rw_try_wlock() by idle thread %p on rwlock %s @ %s:%d",
+ curthread, rw->lock_object.lo_name, file, line));
KASSERT(rw->rw_lock != RW_DESTROYED,
("rw_try_wlock() of destroyed rwlock @ %s:%d", file, line));
@@ -333,6 +339,9 @@ _rw_rlock(struct rwlock *rw, const char *file, int line)
if (SCHEDULER_STOPPED())
return;
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("rw_rlock() by idle thread %p on rwlock %s @ %s:%d",
+ curthread, rw->lock_object.lo_name, file, line));
KASSERT(rw->rw_lock != RW_DESTROYED,
("rw_rlock() of destroyed rwlock @ %s:%d", file, line));
KASSERT(rw_wowner(rw) != curthread,
@@ -521,6 +530,10 @@ _rw_try_rlock(struct rwlock *rw, const char
*file, int line)
if (SCHEDULER_STOPPED())
return (1);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("rw_try_rlock() by idle thread %p on rwlock %s @ %s:%d",
+ curthread, rw->lock_object.lo_name, file, line));
+
for (;;) {
x = rw->rw_lock;
KASSERT(rw->rw_lock != RW_DESTROYED,
diff --git a/sys/kern/kern_sx.c b/sys/kern/kern_sx.c
index bcd7acd..487a324 100644
--- a/sys/kern/kern_sx.c
+++ b/sys/kern/kern_sx.c
@@ -250,6 +250,9 @@ _sx_slock(struct sx *sx, int opts, const char
*file, int line)
if (SCHEDULER_STOPPED())
return (0);
MPASS(curthread != NULL);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("sx_slock() by idle thread %p on sx %s @ %s:%d",
+ curthread, sx->lock_object.lo_name, file, line));
KASSERT(sx->sx_lock != SX_LOCK_DESTROYED,
("sx_slock() of destroyed sx @ %s:%d", file, line));
WITNESS_CHECKORDER(&sx->lock_object, LOP_NEWORDER, file, line, NULL);
@@ -271,6 +274,10 @@ sx_try_slock_(struct sx *sx, const char *file, int line)
if (SCHEDULER_STOPPED())
return (1);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("sx_try_slock() by idle thread %p on sx %s @ %s:%d",
+ curthread, sx->lock_object.lo_name, file, line));
+
for (;;) {
x = sx->sx_lock;
KASSERT(x != SX_LOCK_DESTROYED,
@@ -297,6 +304,9 @@ _sx_xlock(struct sx *sx, int opts, const char
*file, int line)
if (SCHEDULER_STOPPED())
return (0);
MPASS(curthread != NULL);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("sx_xlock() by idle thread %p on sx %s @ %s:%d",
+ curthread, sx->lock_object.lo_name, file, line));
KASSERT(sx->sx_lock != SX_LOCK_DESTROYED,
("sx_xlock() of destroyed sx @ %s:%d", file, line));
WITNESS_CHECKORDER(&sx->lock_object, LOP_NEWORDER | LOP_EXCLUSIVE, file,
@@ -321,6 +331,9 @@ sx_try_xlock_(struct sx *sx, const char *file, int line)
return (1);
MPASS(curthread != NULL);
+ KASSERT(!TD_IS_IDLETHREAD(curthread),
+ ("sx_try_xlock() by idle thread %p on sx %s @ %s:%d",
+ curthread, sx->lock_object.lo_name, file, line));
KASSERT(sx->sx_lock != SX_LOCK_DESTROYED,
("sx_try_xlock() of destroyed sx @ %s:%d", file, line));
diff --git a/sys/kern/subr_turnstile.c b/sys/kern/subr_turnstile.c
index 31d16fe..76fb964 100644
--- a/sys/kern/subr_turnstile.c
+++ b/sys/kern/subr_turnstile.c
@@ -684,7 +684,6 @@ turnstile_wait(struct turnstile *ts, struct thread
*owner, int queue)
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
--
1.7.7.4
More information about the svn-src-projects
mailing list