On Sun, Jul 09, 2023 at 05:24:43PM -0500, Scott Cheloha wrote: > On Sun, Jul 09, 2023 at 08:11:43PM +0200, Claudio Jeker wrote: > > On Sun, Jul 09, 2023 at 12:52:20PM -0500, Scott Cheloha wrote: > > > This patch fixes resume/unhibernate on GPROF kernels where kgmon(8) > > > has activated kernel profiling. > > > > > > I think the problem is that code called from cpu_hatch() does not play > > > nicely with _mcount(), so GPROF kernels crash during resume. I can't > > > point you to which code in particular, but keeping all CPUs out of > > > _mcount() until the primary CPU has completed resume/unhibernate fixes > > > the crash. > > > > > > ok? > > > > To be honest, I'm not sure we need something like this. GPROF is already a > > special case and poeple running a GPROF kernel should probably stop the > > collection of profile data before suspend/hibernate. > > [..]
Also, deraadt@ insisted that GPROF not hang the system across suspend/resume and hibernate/unhibernate: 1. https://marc.info/?l=openbsd-tech&m=168721604322193&w=2 >> Make sure to STOP all kernel profiling before attempting to >> suspend or hibernate your machine. Otherwise I expect it >> will hang. > > That is not acceptable. People suspend and hibernate machines without > being aware of what applications are doing. > >> GPROF is a kernel compile-time option. If you don't enable it, >> you have nothing to worry about. > > Well that's a great hidden reason why noone would ever turn on this > subsystem -- so why is it getting done on it?? 2. https://marc.info/?l=openbsd-tech&m=168721612222214&w=2 >> Make sure to STOP all kernel profiling before attempting to >> suspend or hibernate your machine. Otherwise I expect it >> will hang. > > It is completely acceptable if it produces wrong results, but it must > not hang the system. ... this patch fits the bill. Index: sys/lib/libkern/mcount.c =================================================================== RCS file: /cvs/src/sys/lib/libkern/mcount.c,v retrieving revision 1.14 diff -u -p -r1.14 mcount.c --- sys/lib/libkern/mcount.c 11 Jan 2022 09:21:34 -0000 1.14 +++ sys/lib/libkern/mcount.c 9 Jul 2023 22:03:22 -0000 @@ -33,6 +33,28 @@ #include <sys/param.h> #include <sys/gmon.h> +#ifdef _KERNEL +#ifdef SUSPEND +#include <sys/atomic.h> + +volatile int mcount_disabled; + +void +mcount_disable(void) +{ + mcount_disabled = 1; + membar_producer(); +} + +void +mcount_enable(void) +{ + mcount_disabled = 0; + membar_producer(); +} +#endif /* SUSPEND */ +#endif /* _KERNEL */ + /* * mcount is called on entry to each function compiled with the profiling * switch set. _mcount(), which is declared in a machine-dependent way @@ -63,7 +85,10 @@ _MCOUNT_DECL(u_long frompc, u_long selfp */ if (gmoninit == 0) return; - +#ifdef SUSPEND + if (mcount_disabled) + return; +#endif if ((p = curcpu()->ci_gmon) == NULL) return; #else Index: sys/kern/subr_suspend.c =================================================================== RCS file: /cvs/src/sys/kern/subr_suspend.c,v retrieving revision 1.15 diff -u -p -r1.15 subr_suspend.c --- sys/kern/subr_suspend.c 2 Jul 2023 19:02:27 -0000 1.15 +++ sys/kern/subr_suspend.c 9 Jul 2023 22:03:22 -0000 @@ -26,6 +26,9 @@ #include <sys/mount.h> #include <sys/syscallargs.h> #include <dev/wscons/wsdisplayvar.h> +#ifdef GPROF +#include <sys/gmon.h> +#endif #ifdef HIBERNATE #include <sys/hibernate.h> #endif @@ -63,6 +66,9 @@ top: if (sleep_showstate(v, sleepmode)) return EOPNOTSUPP; +#ifdef GPROF + mcount_disable(); +#endif #if NWSDISPLAY > 0 wsdisplay_suspend(); #endif @@ -192,6 +198,9 @@ fail_hiballoc: start_periodic_resettodr(); #if NWSDISPLAY > 0 wsdisplay_resume(); +#endif +#ifdef GPROF + mcount_enable(); #endif sys_sync(curproc, NULL, NULL); if (cpu_setperf != NULL) Index: sys/sys/gmon.h =================================================================== RCS file: /cvs/src/sys/sys/gmon.h,v retrieving revision 1.9 diff -u -p -r1.9 gmon.h --- sys/sys/gmon.h 11 Jan 2022 23:59:55 -0000 1.9 +++ sys/sys/gmon.h 9 Jul 2023 22:03:22 -0000 @@ -158,6 +158,10 @@ struct gmonparam { #ifdef _KERNEL extern int gmoninit; /* Is the kernel ready for being profiled? */ +#ifdef SUSPEND +void mcount_disable(void); +void mcount_enable(void); +#endif #else /* !_KERNEL */ #include <sys/cdefs.h> Index: lib/libc/gmon/mcount.c =================================================================== RCS file: /cvs/src/lib/libc/gmon/mcount.c,v retrieving revision 1.16 diff -u -p -r1.16 mcount.c --- lib/libc/gmon/mcount.c 11 Jan 2022 09:21:34 -0000 1.16 +++ lib/libc/gmon/mcount.c 9 Jul 2023 22:03:23 -0000 @@ -31,6 +31,28 @@ #include <sys/types.h> #include <sys/gmon.h> +#ifdef _KERNEL +#ifdef SUSPEND +#include <sys/atomic.h> + +volatile int mcount_disabled; + +void +mcount_disable(void) +{ + mcount_disabled = 1; + membar_producer(); +} + +void +mcount_enable(void) +{ + mcount_disabled = 0; + membar_producer(); +} +#endif /* SUSPEND */ +#endif /* _KERNEL */ + /* * mcount is called on entry to each function compiled with the profiling * switch set. _mcount(), which is declared in a machine-dependent way @@ -61,7 +83,10 @@ _MCOUNT_DECL(u_long frompc, u_long selfp */ if (gmoninit == 0) return; - +#ifdef SUSPEND + if (mcount_disabled) + return; +#endif if ((p = curcpu()->ci_gmon) == NULL) return; #else
