Date: Wed, 10 Mar 2021 12:57:44 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Kyle Evans <kevans@freebsd.org> Cc: 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: <CAGudoHFbEt1%2BmSDx04x1-YrxwrxHMdfoRj69EV183ngZ4TkxkA@mail.gmail.com> In-Reply-To: <202103091117.129BHOZa042851@gitrepo.freebsd.org> References: <202103091117.129BHOZa042851@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
There is something very wrong going on here.
Key thing to note is that malloc is ultimately a wrapper around
uma_zalloc. Whatever asserts you may want to add to malloc to catch
problems sooner, should also present in uma.
uma has the following:
if (flags & M_WAITOK) {
WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL,
"uma_zalloc_debug: zone \"%s\"", zone->uz_name);
}
This code used to execute prior to this commit and fail to catch the
problems which got reported already.
In other words:
- WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK is incomplete in terms of
internals its internals
- the above should be in malloc, perhaps after being abstracted into a
an assert-helper
- fixing witness deficiency will probably find a slew of new problems
- this should remain as a warning (maybe rate-limited) for the foreseable future
On 3/9/21, Kyle Evans <kevans@freebsd.org> 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"));
> }
> }
>
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHFbEt1%2BmSDx04x1-YrxwrxHMdfoRj69EV183ngZ4TkxkA>