Re: Broken suspend-resume (suspend to RAM) with enabled INVARIANTS on 11-CURRENT - with workaround

On Sun, Feb 07, 2016 at 10:59:48PM +0100, Oliver Pinter wrote:
> On 2/6/16, Konstantin Belousov <kostikbel_at_gmail.com> wrote:
> > On Fri, Feb 05, 2016 at 07:34:02PM +0100, Oliver Pinter wrote:
> >> Not yet tested, but possible fix:
> >>
> >> diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
> >> index cb952da..25bae84 100644
> >> --- a/sys/kern/init_main.c
> >> +++ b/sys/kern/init_main.c
> >> _at__at_ -482,7 +482,7 _at__at_ proc0_init(void *dummy __unused)
> >>         session0.s_leader = p;
> >>
> >>         p->p_sysent = &null_sysvec;
> >> -       p->p_flag = P_SYSTEM | P_INMEM;
> >> +       p->p_flag = P_SYSTEM | P_INMEM | P_KTHREAD;
> >>         p->p_flag2 = 0;
> >>         p->p_state = PRS_NORMAL;
> > So did you tested this ?  Did you do an audit to see whether P_KTHREAD
> > other usages possibly conflict with the proc0 specifics ?
> 
> Tested and working as expected.
In fact, I do not want to mark proc0 as P_KTHREAD.  This would allow
the kthread_suspend() on the proc0, and I did not audited uses of
the KPI to guarantee the effect, also I am not sure about third-party
code which could have relied on kthread_suspend() rejecting proc0.

I do agree that the assert outlived its usefulness. I restructured
the comments to put it before stop_all_proc() and removed the assert,
together with disabling the compilation of sysctl_debug_stop_all_proc().

> Other uses would not conflict, since the codes already checks for
> P_SYSTEM and the P_KTHREAD flag is almost kern_kthread.c's "private"
> flag.
> 
> And this change probably fixes one issue with hwpmc too, in the kernel case:
> 
> --
> dev/hwpmc/hwpmc_mod.c-
> dev/hwpmc/hwpmc_mod.c-  /* issue an attach event to a configured log file */
> dev/hwpmc/hwpmc_mod.c-  if (pm->pm_owner->po_flags & PMC_PO_OWNS_LOGFILE) {
> dev/hwpmc/hwpmc_mod.c:          if (p->p_flag & P_KTHREAD) {
> dev/hwpmc/hwpmc_mod.c-                  fullpath = kernelname;
> dev/hwpmc/hwpmc_mod.c-                  freepath = NULL;
> dev/hwpmc/hwpmc_mod.c-          } else {
> dev/hwpmc/hwpmc_mod.c-                  pmc_getfilename(p->p_textvp,
> &fullpath, &freepath);
> dev/hwpmc/hwpmc_mod.c-                  pmclog_process_pmcattach(pm,
> p->p_pid, fullpath);
> dev/hwpmc/hwpmc_mod.c-          }
What is wrong with this code ? proc0 has NULL p_textvp, so the call
to pmc_getfilename() does not do anything except setting pointers to NULL.