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

On 2/5/16, Oliver Pinter <oliver.pinter_at_hardenedbsd.org> wrote:
> Hi all!
>
> I used this gdb macro, to traverse the proc list, and print out the
> relevant p_flag's flags:
>
> set $curr_proc=0
> def next_proc
>         if $curr_proc == 0
>                 set $curr_proc = allproc.lh_first
>         else
>                 set $curr_proc = $curr_proc.p_list.le_next
>         end
>         set $curr_p_flag = $curr_proc->p_flag
>         set $curr_name = $curr_proc->p_comm
>         p $curr_name
>         p/x $curr_p_flag
>         if ($curr_p_flag & 0x00004) != 0
>                 printf "is P_KTHREAD\n"
>         else
>                 printf "isn't P_KTHREAD\n"
>         end
>         if ($curr_p_flag & 0x00200) != 0
>                 printf "is P_SYSTEM\n"
>         else
>                 printf "isn't P_SYSTEM\n"
>         end
> end
>
> And seems like the "kernel" process don't have the P_KTHREAD flag,
> which fires the KASSERT in sys/kern/kern_proc.c's stop_all_proc(void)
> function.
>
> The relevant part of the code is:
> 2986 void
> 2987 stop_all_proc(void)
> 2988 {
> 2989         struct proc *cp, *p;
> 2990         int r, gen;
> 2991         bool restart, seen_stopped, seen_exiting, stopped_some;
> 2992
> 2993         cp = curproc;
> 2994         /*
> 2995          * stop_all_proc() assumes that all process which have
> 2996          * usermode must be stopped, except current process, for
> 2997          * obvious reasons.  Since other threads in the process
> 2998          * establishing global stop could unstop something, disable
> 2999          * calls from multithreaded processes as precaution.  The
> 3000          * service must not be user-callable anyway.
> 3001          */
> *3002         KASSERT((cp->p_flag & P_HADTHREADS) == 0 ||
> *3003             (cp->p_flag & P_KTHREAD) != 0, ("mt stop_all_proc"));
> 3004
> 3005 allproc_loop:
>
> If I comment out this KASSERT or just whitelist the "kernel" process,
> then I get a completely working suspend to ram and resume on my laptop
> (which is a HP 430G1) with one additional small tweak
> (hw.acpi.reset_video=1).
>
> So the question is that the KASSERT is bogus or too strict, or the
> P_KTHREAD flags is missing from "kernel" process?
>
> The gdb macros output is:
> [...]
> (kgdb)
> $139 = 0xfffff800029773c8 "rand_harvestq"
> $140 = 0x10000204
> is P_KTHREAD
> is P_SYSTEM
> (kgdb)
> $141 = 0xfffff80002977940 "init"
> $142 = 0x10004200
> isn't P_KTHREAD
> is P_SYSTEM
> (kgdb)
> $143 = 0xfffff800029783c8 "audit"
> $144 = 0x10000204
> is P_KTHREAD
> is P_SYSTEM
> (kgdb)
> $145 = 0xffffffff80f1b4d0 "kernel"
> $146 = 0x10000280
> isn't P_KTHREAD
> is P_SYSTEM
>
> I've CC-ed Konstantin to this discussion, because he added this
> functionality to the kernel.
>
> Additional info:
> If I try to trigger the KASSERT with the debug.stop_all_proc=1 sysctl,
> then it's never fires, but when I try to put my machine to S3 with
> ctrl+alt+space then it fires.
>

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;
 #ifdef PAX