Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 22 Dec 2012 13:24:31 +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-FndAbouQgdbVVZcjWUkvZxjofHCfVbvdGQsVd3FbqQ04Qkg@mail.gmail.com>
In-Reply-To: <20121222230402.P1765@besplex.bde.org>
References:  <201212220937.qBM9bYQK050680@svn.freebsd.org> <20121222204409.V1410@besplex.bde.org> <CAJ-FndCazK%2BE9w8fGvO4uou-sCOgnf8cVFu0nzXjUeTu2RYF4w@mail.gmail.com> <20121222230402.P1765@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Dec 22, 2012 at 1:20 PM, Bruce Evans <brde@optusnet.com.au> wrote:
> On Sat, 22 Dec 2012, Attilio Rao wrote:
>
>> 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.
>
>
> But this sort of hack only breaks accidental detection of a bug (maybe
> the bug causes deadlock or data corruption soon).  The type of hack
> that helps is 'if (kdb_active) skip_locking();' deep in code that
> shouldn't even be called if kdb is active.  Here it is 'if (kdb_active)
> skip_checking();'

I agree, but our kernel relies on this stuff for kdb heavily (see all
the duplicate checks around). We need to change that all together. We
need to provide lockless version for KDB command.

>>
>>
>> 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.
>
>
> I just mean that the correct kdb_active KASSERT() is independent of the
> idlethread one.  It should also have a different message.  I forgot to
> provide a message.

If you want to use a better message, be my guest.

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-FndAbouQgdbVVZcjWUkvZxjofHCfVbvdGQsVd3FbqQ04Qkg>