Date: Tue, 21 Sep 2010 12:50:40 -0400 From: John Baldwin <jhb@freebsd.org> To: mdf@freebsd.org Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Andriy Gapon <avg@freebsd.org> Subject: Re: svn commit: r212964 - head/sys/kern Message-ID: <201009211250.40704.jhb@freebsd.org> In-Reply-To: <AANLkTim%2BZYppETzFOYrGjhsEXr9hVPi8L0Mvaa6RkhMq@mail.gmail.com> References: <201009211507.o8LF7iVv097676@svn.freebsd.org> <4C98D200.4040909@freebsd.org> <AANLkTim%2BZYppETzFOYrGjhsEXr9hVPi8L0Mvaa6RkhMq@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday, September 21, 2010 12:31:01 pm mdf@freebsd.org wrote: > On Tue, Sep 21, 2010 at 8:40 AM, Andriy Gapon <avg@freebsd.org> wrote: > > on 21/09/2010 18:27 Andriy Gapon said the following: > >> on 21/09/2010 18:17 mdf@FreeBSD.org said the following: > >>> > >>> I'd recommend using stack_print_ddb(), as that avoids any locking > >>> which may hang depending on how the kernel panic'd. > >> > >> It seems that stack_print_ddb() depends on DDB? > > > > But the point about locking is very good. > > How do you suggest we can deal with it? > > > > A dirty hack would be to check panicstr in linker_search_symbol_name and avoid > > locking, but I don't like that at all. > > Perhaps, some code in subr_stack.c could be taken outside DDB ifdef? > > I keep forgetting, but actually _mtx_lock_sleep() will just return if > panicstr is set. _mtx_assert() is similarly guarded, so actually it > should be mostly okay. Err, I don't think _mtx_lock_sleep() is guarded in that fashion? I have an old patch to do that but have never committed it. If we want that we should probably change rwlocks and sxlocks to have also not block when panicstr is set. --- //depot/vendor/freebsd/src/sys/kern/kern_mutex.c 2010-05-11 18:31:25.000000000 0000 +++ //depot/projects/smpng/sys/kern/kern_mutex.c 2010-06-01 20:12:02.000000000 0000 @@ -348,6 +348,15 @@ return; } + /* + * If we have already panic'd and this is the thread that called + * panic(), then don't block on any mutexes but silently succeed. + * Otherwise, the kernel will deadlock since the scheduler isn't + * going to run the thread that holds the lock we need. + */ + if (panicstr != NULL && curthread->td_flags & TDF_INPANIC) + return; + lock_profile_obtain_lock_failed(&m->lock_object, &contested, &waittime); if (LOCK_LOG_TEST(&m->lock_object, opts)) @@ -664,6 +673,15 @@ } /* + * If we failed to unlock this lock and we are a thread that has + * called panic(), it may be due to the bypass in _mtx_lock_sleep() + * above. In that case, just return and leave the lock alone to + * avoid changing the state. + */ + if (panicstr != NULL && curthread->td_flags & TDF_INPANIC) + return; + + /* * We have to lock the chain before the turnstile so this turnstile * can be removed from the hash list if it is empty. */ -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009211250.40704.jhb>