On Tue, Sep 21, 2010 at 9:50 AM, John Baldwin <j...@freebsd.org> wrote: > On Tuesday, September 21, 2010 12:31:01 pm m...@freebsd.org wrote: >> On Tue, Sep 21, 2010 at 8:40 AM, Andriy Gapon <a...@freebsd.org> wrote: >> > on 21/09/2010 18:27 Andriy Gapon said the following: >> >> on 21/09/2010 18:17 m...@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.
D'oh! Once again I was looking at Isilon source but not CURRENT. We had patches for mtx (back on 6.1) and haven't updated them for sx/rw for 7. We're also running with local patches to stop CPUs in panic() that Mr Jacob has a copy of. Regarding the original issue, then, I think in the short term using a safe stack_print() would be the correct thing. Changing the locking and stop_cpus logic will not happen in the next week. :-) Thanks, matthew > > --- //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 > _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
