git: eaed922eda58 - main - panic()/KERNEL_PANICKED(): Move back to using 'panicstr' as a flag
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 26 Jan 2024 21:11:27 UTC
The branch main has been updated by olce:
URL: https://cgit.FreeBSD.org/src/commit/?id=eaed922eda584da7306e7c371ff6adf3fc4dc4c3
commit eaed922eda584da7306e7c371ff6adf3fc4dc4c3
Author: Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-01-18 10:15:18 +0000
Commit: Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-01-26 21:07:56 +0000
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
---
sys/kern/kern_shutdown.c | 15 +++++++++++----
sys/sys/kassert.h | 3 +--
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c
index 3b0e3997c852..d66c7ba0d344 100644
--- a/sys/kern/kern_shutdown.c
+++ b/sys/kern/kern_shutdown.c
@@ -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;
}
diff --git a/sys/sys/kassert.h b/sys/sys/kassert.h
index 7b54ac6ae519..da8701ce3858 100644
--- a/sys/sys/kassert.h
+++ b/sys/sys/kassert.h
@@ -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