Date: Tue, 21 Sep 2010 10:38:36 -0700 From: mdf@FreeBSD.org To: John Baldwin <jhb@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: <AANLkTimzMJoQ4ctqN_r=BYvc1RgLQHuOdjOEGmjVuDRi@mail.gmail.com> In-Reply-To: <201009211250.40704.jhb@freebsd.org> References: <201009211507.o8LF7iVv097676@svn.freebsd.org> <4C98D200.4040909@freebsd.org> <AANLkTim%2BZYppETzFOYrGjhsEXr9hVPi8L0Mvaa6RkhMq@mail.gmail.com> <201009211250.40704.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Sep 21, 2010 at 9:50 AM, John Baldwin <jhb@freebsd.org> wrote: > 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 a= nd 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. =A0_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? =A0I hav= e an > old patch to do that but have never committed it. =A0If we want that we s= hould > 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 =A0 =A02010-05-11 18= :31:25.000000000 0000 > +++ //depot/projects/smpng/sys/kern/kern_mutex.c =A0 =A0 =A0 =A02010-06-0= 1 20:12:02.000000000 0000 > @@ -348,6 +348,15 @@ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return; > =A0 =A0 =A0 =A0} > > + =A0 =A0 =A0 /* > + =A0 =A0 =A0 =A0* If we have already panic'd and this is the thread that= called > + =A0 =A0 =A0 =A0* panic(), then don't block on any mutexes but silently = succeed. > + =A0 =A0 =A0 =A0* Otherwise, the kernel will deadlock since the schedule= r isn't > + =A0 =A0 =A0 =A0* going to run the thread that holds the lock we need. > + =A0 =A0 =A0 =A0*/ > + =A0 =A0 =A0 if (panicstr !=3D NULL && curthread->td_flags & TDF_INPANIC= ) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return; > + > =A0 =A0 =A0 =A0lock_profile_obtain_lock_failed(&m->lock_object, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&contested, &waittime); > =A0 =A0 =A0 =A0if (LOCK_LOG_TEST(&m->lock_object, opts)) > @@ -664,6 +673,15 @@ > =A0 =A0 =A0 =A0} > > =A0 =A0 =A0 =A0/* > + =A0 =A0 =A0 =A0* If we failed to unlock this lock and we are a thread t= hat has > + =A0 =A0 =A0 =A0* called panic(), it may be due to the bypass in _mtx_lo= ck_sleep() > + =A0 =A0 =A0 =A0* above. =A0In that case, just return and leave the lock= alone to > + =A0 =A0 =A0 =A0* avoid changing the state. > + =A0 =A0 =A0 =A0*/ > + =A0 =A0 =A0 if (panicstr !=3D NULL && curthread->td_flags & TDF_INPANIC= ) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return; > + > + =A0 =A0 =A0 /* > =A0 =A0 =A0 =A0 * We have to lock the chain before the turnstile so this = turnstile > =A0 =A0 =A0 =A0 * can be removed from the hash list if it is empty. > =A0 =A0 =A0 =A0 */ > > -- > John Baldwin >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimzMJoQ4ctqN_r=BYvc1RgLQHuOdjOEGmjVuDRi>