Date: Wed, 10 Mar 2021 11:04:41 +0100 From: Hans Petter Selasky <hps@selasky.org> To: John Baldwin <jhb@FreeBSD.org>, Kyle Evans <kevans@FreeBSD.org>, src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 1ae20f7c70ea - main - kern: malloc: fix panic on M_WAITOK during THREAD_NO_SLEEPING() Message-ID: <4a923ee1-64a0-93ab-1e73-ff164d690bfc@selasky.org> In-Reply-To: <3d67f7e4-c1fc-ca2c-8fc5-417dcf8e4145@FreeBSD.org> References: <202103091117.129BHOZa042851@gitrepo.freebsd.org> <3d67f7e4-c1fc-ca2c-8fc5-417dcf8e4145@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 3/9/21 6:33 PM, John Baldwin wrote: > On 3/9/21 3:17 AM, Kyle Evans wrote: >> The branch main has been updated by kevans: >> >> URL: >> https://cgit.FreeBSD.org/src/commit/?id=1ae20f7c70ea16fa8b702e409030e170df4f5c13 >> >> >> commit 1ae20f7c70ea16fa8b702e409030e170df4f5c13 >> Author: Kyle Evans <kevans@FreeBSD.org> >> AuthorDate: 2021-03-08 06:16:27 +0000 >> Commit: Kyle Evans <kevans@FreeBSD.org> >> CommitDate: 2021-03-09 11:16:39 +0000 >> >> kern: malloc: fix panic on M_WAITOK during THREAD_NO_SLEEPING() >> Simple condition flip; we wanted to panic here after >> epoch_trace_list(). >> Reviewed by: glebius, markj >> MFC after: 3 days >> Differential Revision: https://reviews.freebsd.org/D29125 >> --- >> sys/kern/kern_malloc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/sys/kern/kern_malloc.c b/sys/kern/kern_malloc.c >> index 48383358e3ad..0d6f9dcfcab7 100644 >> --- a/sys/kern/kern_malloc.c >> +++ b/sys/kern/kern_malloc.c >> @@ -537,7 +537,7 @@ malloc_dbg(caddr_t *vap, size_t *sizep, struct >> malloc_type *mtp, >> #ifdef EPOCH_TRACE >> epoch_trace_list(curthread); >> #endif >> - KASSERT(1, >> + KASSERT(0, >> ("malloc(M_WAITOK) with sleeping prohibited")); > > I would perhaps just use panic() directly under INVARIANTS instead of > KASSERT(false, ...) > > Either that or duplicate the condition and let the compiler deal with > avoiding checking > it twice, e.g.: > > #ifdef EPOCH_TRACE > if (!THREAD_CAN_SLEEP()) > epoc_trace_list(curthread); > #endif > KASSERT(THREAD_CAN_SLEEP(), ("malloc(M_WAITOK) with sleeping > prohibited")); > Hi, Maybe the KASSERT() can just be a regular warning with backtrace. malloc() doesn't sleep unless there is no memory, would give developers more time to fix their bugs. --HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4a923ee1-64a0-93ab-1e73-ff164d690bfc>