From owner-svn-src-projects@FreeBSD.ORG Sun Sep 9 15:03:17 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B8E1C106566B; Sun, 9 Sep 2012 15:03:17 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-lb0-f182.google.com (mail-lb0-f182.google.com [209.85.217.182]) by mx1.freebsd.org (Postfix) with ESMTP id 9750C8FC08; Sun, 9 Sep 2012 15:03:16 +0000 (UTC) Received: by lbbgg13 with SMTP id gg13so1103337lbb.13 for ; Sun, 09 Sep 2012 08:03:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:reply-to:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=7H7FOaca9Ru46USaNFLVzF2S9htKOf9wWfXqrUFR40w=; b=aELQUbHJ+3r28zNXxwezxMz3C+5aetwrIC3zVbtqnpMhSqLZjnFpRphGaOG2/BBcdY xD48pIyAJJ42TOwOvMOiHlvoZdN3arxmT6pQ0/2GSf9TgjvWQ6KuFyCHlAZHks7Cp040 6sXYYbFnOFkmy+stmQRoLJq0nyKy+1lCSCD3+Irdd4PQpbhYTNMO+xfXFNkzGk1GXWns mQpex3VexPcWguyBMEg8jmWCMtpzzPryefu+3yqX+I4AOcM3Itn7rXmrFwbo19iQzzYL m5bWFxoCaRVbQHmrWC+sI7UCNUIBNnz20HfvXfekjYiRUdnT/SNx6oIOmeXcXJe55tn0 cItw== MIME-Version: 1.0 Received: by 10.112.86.41 with SMTP id m9mr4035122lbz.108.1347202995200; Sun, 09 Sep 2012 08:03:15 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.112.102.39 with HTTP; Sun, 9 Sep 2012 08:03:14 -0700 (PDT) In-Reply-To: References: <201207301350.q6UDobCI099069@svn.freebsd.org> <201207301732.33474.jhb@freebsd.org> Date: Sun, 9 Sep 2012 16:03:14 +0100 X-Google-Sender-Auth: zJLgs-gyubSS6i04OEBpXDHJhUY Message-ID: From: Attilio Rao To: John Baldwin Content-Type: text/plain; charset=UTF-8 Cc: Konstantin Belousov , Davide Italiano , src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: attilio@FreeBSD.org List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 09 Sep 2012 15:03:17 -0000 On 8/2/12, Attilio Rao wrote: > On 7/30/12, John Baldwin 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)) {