git: 1ae20f7c70ea - main - kern: malloc: fix panic on M_WAITOK during THREAD_NO_SLEEPING()
John Baldwin
jhb at FreeBSD.org
Thu Mar 11 17:24:03 UTC 2021
On 3/10/21 3:57 AM, Mateusz Guzik wrote:
> 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 warns about holding locks, not about TD_NO_SLEEPING. Witness' role
is to check lock orders and warn about invalid operations when holding
certain locks. It does not currently verify anything about other inhibitions
like TD_NO_SLEEPING. See, e.g. the list of assertions in userret() which
includes a WITNESS_WARN in addition to several other checks (including
TD_NO_SLEEPING). Arguably we should just move the TD_NO_SLEEPING check
from malloc() to here along with the td_intr_nesting_level. Any case
where you can't use malloc() with M_WAITOK you can't use uma with M_WAITOK
either.
> 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
I don't know that we've had so many issues that we can't just fix them on
HEAD right now vs having to make this a warning instead of keeping it as
a panic.
--
John Baldwin
More information about the dev-commits-src-all
mailing list