skipping locks, mutex_owned, usb
Andriy Gapon
avg at FreeBSD.org
Tue Aug 23 13:33:38 UTC 2011
on 23/08/2011 15:58 mdf at FreeBSD.org said the following:
> On Tue, Aug 23, 2011 at 5:09 AM, Andriy Gapon <avg at 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.
But, OTOH, there are snippets like this (found with grep, haven't looked at the code):
/usr/src/sys/kern/kern_sx.c: while (mtx_owned(&Giant)) {
>> 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.
Yes, I agree with your and John's practical approach to this.
Maybe print a warning about each skipped locking operation? But not sure if that
would be useful, if there would be no intention of changing the code.
--
Andriy Gapon
More information about the freebsd-arch
mailing list