svn commit: r212964 - head/sys/kern

[John Baldwin]

On Tuesday, September 21, 2010 12:31:01 pm mdf at freebsd.org wrote:
> On Tue, Sep 21, 2010 at 8:40 AM, Andriy Gapon <avg at freebsd.org> wrote:
> > on 21/09/2010 18:27 Andriy Gapon said the following:
> >> on 21/09/2010 18:17 mdf at 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