Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 23 Aug 2011 05:58:14 -0700
From:      mdf@FreeBSD.org
To:        Andriy Gapon <avg@freebsd.org>
Cc:        freebsd-arch@freebsd.org
Subject:   Re: skipping locks, mutex_owned, usb
Message-ID:  <CAMBSHm-nT5-wTbAFqsJ6ZjCPE9Vfxeva1F3zwWSAK8Ecw=8VsA@mail.gmail.com>
In-Reply-To: <4E53986B.5000804@FreeBSD.org>

index | next in thread | previous in thread | raw e-mail

On Tue, Aug 23, 2011 at 5:09 AM, Andriy Gapon <avg@freebsd.org> wrote:
> III.  With my stop_scheduler_on_panic patch ukbd_poll() produces infinite chains
> of 'infinite' recursion because mtx_owned() always returns false.  This is because
> I skip all lock/unlock/etc operations in the post-panic context.  I think that
> it's a good philosophical question: what operations like mtx_owned(),
> mtx_recursed(), mtx_trylock() 'should' return when we actually act as if no locks
> exist at all?
>
> My first knee-jerk reaction was to change mtx_owned() to return true in this
> "lock-less" context, but _hypothetically_ there could exist some symmetric code
> that does something like:
> func()
> {
>        if (mtx_owned(&Giant)) {
>                mtx_unlock(&Giant);
>                func();
>                mtx_lock(&Giant);
>                return;
>        }
>
>        // etc ...
>
> What do you think about this problem?

Given that checking for a lock being held is a lot more common than
checking if it's not held (in the context of mtx_assert(9) and
friends), it seems reasonable to report that a lock is held in the
special context of after panic.

> That question III actually brings another thought: perhaps the whole of idea of
> skipping locks in a certain context is not a correct direction?
> Perhaps, instead we should identify all the code that could be legitimately
> executed in the after-panic and/or kdb contexts and make that could explicitly
> aware of its execution context.  That is, instead of trying to make _lock,
> _unlock, _owned, _trylock, etc do the right thing auto-magically, we should try to
> make the calling code check panicstr and kdb_active and do the right thing on that
> level (which would include not invoking those lock-related operations or other
> inappropriate operations).

I don't think it's possible to identify all the code, since what runs
after panic isn't tested very much. :-)  I don't disagree that one
should think about the issue, but providing reasonable defaults like
skipping locks reduces the burden on the programmer.

Thanks,
matthew


home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAMBSHm-nT5-wTbAFqsJ6ZjCPE9Vfxeva1F3zwWSAK8Ecw=8VsA>