panic()/KERNEL_PANICKED(): Move back to using 'panicstr' as a flag

Currently, no performance-critical path tests for a panic.  Moreover, we
today have KERNEL_PANICKED() which wraps the test into
__predict_false(), already catering to those (potential) use cases.
Also, in practice we don't support 64-bit architectures without caches,
so reading an 'int' instead of a pointer doesn't (directly) save any
memory access.  Finally, 'panicked' is redundant with 'panicstr' (and
wastes a tiny amount of memory).

Consequently:
1. Use again 'panicstr' as a flag indicating that the system is
panicking.  To this end:
  - Modify panic() so that it ensures this pointer is set to some
    non-NULL value even if the caller didn't pass any panic string.
  - Modify KERNEL_PANICKED() to test for 'panicstr'.
  - Remove 'panicked'.
2. Annotate 'panicstr' with '__read_mostly' (instead of using
'__read_frequently' as for 'panicked').  This may have to be changed if,
in the future, some performance-intensive path needs to test it.
3. Convert a few more direct tests of 'panicstr' to using
KERNEL_PANICKED().

Reviewed by:            kib, markj, emaste
Approved by:            markj (mentor)
MFC after:              2 weeks
Sponsored by:           The FreeBSD Foundation
Differential Revision:  https://reviews.freebsd.org/D43569
This commit is contained in:
Olivier Certner 2024-01-18 11:15:18 +01:00
parent ab0841bdbe
commit eaed922eda
No known key found for this signature in database
GPG key ID: 8CA13040971E2627
2 changed files with 12 additions and 6 deletions

View file

@ -224,8 +224,7 @@ SYSCTL_INT(_kern, OID_AUTO, kerneldump_gzlevel, CTLFLAG_RWTUN,
* Variable panicstr contains argument to first call to panic; used as flag
* to indicate that the kernel has already called panic.
*/
const char *panicstr;
bool __read_frequently panicked;
const char *panicstr __read_mostly;
int __read_mostly dumping; /* system is dumping */
int rebooting; /* system is rebooting */
@ -899,6 +898,15 @@ vpanic(const char *fmt, va_list ap)
int bootopt, newpanic;
static char buf[256];
/*
* 'fmt' must not be NULL as it is put into 'panicstr' which is then
* used as a flag to detect if the kernel has panicked. Also, although
* vsnprintf() supports a NULL 'fmt' argument, use a more informative
* message.
*/
if (fmt == NULL)
fmt = "<no panic string!>";
spinlock_enter();
#ifdef SMP
@ -907,7 +915,7 @@ vpanic(const char *fmt, va_list ap)
* concurrently entering panic. Only the winner will proceed
* further.
*/
if (panicstr == NULL && !kdb_active) {
if (!KERNEL_PANICKED() && !kdb_active) {
other_cpus = all_cpus;
CPU_CLR(PCPU_GET(cpuid), &other_cpus);
stop_cpus_hard(other_cpus);
@ -927,7 +935,6 @@ vpanic(const char *fmt, va_list ap)
else {
bootopt |= RB_DUMP;
panicstr = fmt;
panicked = true;
newpanic = 1;
}

View file

@ -35,8 +35,7 @@
#ifdef _KERNEL
extern const char *panicstr; /* panic message */
extern bool panicked;
#define KERNEL_PANICKED() __predict_false(panicked)
#define KERNEL_PANICKED() __predict_false(panicstr != NULL)
/*
* Trap accesses going through a pointer. Moreover if kasan is available trap