Date: Mon, 8 Feb 2016 12:52:30 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Oliver Pinter <oliver.pinter@hardenedbsd.org> Cc: current <current@freebsd.org> Subject: Re: Broken suspend-resume (suspend to RAM) with enabled INVARIANTS on 11-CURRENT - with workaround Message-ID: <20160208105230.GG91220@kib.kiev.ua> In-Reply-To: <CAPQ4ffu5mv72E0EFekUKZazipPd5NYUA%2BEs3%2BM_O0P_xDZJtww@mail.gmail.com> References: <CAPQ4ffvAbuxDbsj=O-RMG_SXtC3xhprSESab3cv4poEQ20pYHg@mail.gmail.com> <CAPQ4ffsfGJBt44oLC83O3zC57JFGCy%2BjrKw3fKMQC4ZTrFNYRw@mail.gmail.com> <20160206152345.GW91220@kib.kiev.ua> <CAPQ4ffu5mv72E0EFekUKZazipPd5NYUA%2BEs3%2BM_O0P_xDZJtww@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Feb 07, 2016 at 10:59:48PM +0100, Oliver Pinter wrote: > On 2/6/16, Konstantin Belousov <kostikbel@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 > >> @@ -482,7 +482,7 @@ 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160208105230.GG91220>