Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 22 Dec 2012 10:57:07 +0100
From:      Attilio Rao <attilio@freebsd.org>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r244582 - head/sys/kern
Message-ID:  <CAJ-FndCazK%2BE9w8fGvO4uou-sCOgnf8cVFu0nzXjUeTu2RYF4w@mail.gmail.com>
In-Reply-To: <20121222204409.V1410@besplex.bde.org>
References:  <201212220937.qBM9bYQK050680@svn.freebsd.org> <20121222204409.V1410@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Dec 22, 2012 at 10:51 AM, Bruce Evans <brde@optusnet.com.au> wrote:
> On Sat, 22 Dec 2012, Attilio Rao wrote:
>
>> Log:
>>  Fixup r240424: On entering KDB backends, the hijacked thread to run
>>  interrupt context can still be idlethread. At that point, without the
>>  panic condition, it can still happen that idlethread then will try to
>>  acquire some locks to carry on some operations.
>>
>>  Skip the idlethread check on block/sleep lock operations when KDB is
>>  active.
>
>
> This seems backwards to me.  It is an error to go near normal locking
> code when kdb is active.

I completely agree, but this is not what happens nowadays with FreeBSD kernel.
In my view, KDB should not call into normal code, but in special
wrappers which skip locking entirely, in particular because other cpus
are stopped, so there is no race going on.
However, this requires a big change and as long as this doesn't happen
we need to stuck with similar hacks.

>
>
>> Modified: head/sys/kern/kern_lock.c
>>
>> ==============================================================================
>> --- head/sys/kern/kern_lock.c   Sat Dec 22 07:48:09 2012        (r244581)
>> +++ head/sys/kern/kern_lock.c   Sat Dec 22 09:37:34 2012        (r244582)
>> @@ -35,6 +35,7 @@
>> __FBSDID("$FreeBSD$");
>>
>> #include <sys/param.h>
>> +#include <sys/kdb.h>
>> #include <sys/ktr.h>
>> #include <sys/lock.h>
>> #include <sys/lock_profile.h>
>> @@ -477,7 +478,7 @@ __lockmgr_args(struct lock *lk, u_int fl
>>         KASSERT((flags & LK_INTERLOCK) == 0 || ilk != NULL,
>>             ("%s: LK_INTERLOCK passed without valid interlock @ %s:%d",
>>             __func__, file, line));
>> -       KASSERT(!TD_IS_IDLETHREAD(curthread),
>> +       KASSERT(kdb_active != 0 || !TD_IS_IDLETHREAD(curthread),
>>             ("%s: idle thread %p on lockmgr %s @ %s:%d", __func__,
>> curthread,
>>             lk->lock_object.lo_name, file, line));
>
>
> This is backwards from:
>
>         KASSERT(kdb_active == 0);
>
> which makes it fatal for any thread to call here.

I do not understand. For kdb_active == 0 it still checks for
IDLETHREAD if it is not idlethread it doesn't panic, it panics
otherwise, which seems the right to me.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndCazK%2BE9w8fGvO4uou-sCOgnf8cVFu0nzXjUeTu2RYF4w>