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> References: <4E53986B.5000804@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Aug 23, 2011 at 5:09 AM, Andriy Gapon <avg@freebsd.org> wrote:
> III. =A0With my stop_scheduler_on_panic patch ukbd_poll() produces infini=
te chains
> of 'infinite' recursion because mtx_owned() always returns false. =A0This=
is because
> I skip all lock/unlock/etc operations in the post-panic context. =A0I thi=
nk 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 t=
his
> "lock-less" context, but _hypothetically_ there could exist some symmetri=
c code
> that does something like:
> func()
> {
> =A0 =A0 =A0 =A0if (mtx_owned(&Giant)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mtx_unlock(&Giant);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0func();
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mtx_lock(&Giant);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return;
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0// 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 i=
dea of
> skipping locks in a certain context is not a correct direction?
> Perhaps, instead we should identify all the code that could be legitimate=
ly
> executed in the after-panic and/or kdb contexts and make that could expli=
citly
> aware of its execution context. =A0That is, instead of trying to make _lo=
ck,
> _unlock, _owned, _trylock, etc do the right thing auto-magically, we shou=
ld try to
> make the calling code check panicstr and kdb_active and do the right thin=
g 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAMBSHm-nT5-wTbAFqsJ6ZjCPE9Vfxeva1F3zwWSAK8Ecw=8VsA>