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