Date: Mon, 08 Feb 2016 10:36:59 -0800 From: John Baldwin <jhb@freebsd.org> To: freebsd-current@freebsd.org Cc: Konstantin Belousov <kostikbel@gmail.com>, Oliver Pinter <oliver.pinter@hardenedbsd.org> Subject: Re: Broken suspend-resume (suspend to RAM) with enabled INVARIANTS on 11-CURRENT - with workaround Message-ID: <1638099.vX3eOuueKK@ralph.baldwin.cx> In-Reply-To: <20160208105230.GG91220@kib.kiev.ua> References: <CAPQ4ffvAbuxDbsj=O-RMG_SXtC3xhprSESab3cv4poEQ20pYHg@mail.gmail.com> <CAPQ4ffu5mv72E0EFekUKZazipPd5NYUA%2BEs3%2BM_O0P_xDZJtww@mail.gmail.com> <20160208105230.GG91220@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, February 08, 2016 12:52:30 PM Konstantin Belousov wrote: > > 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. proc0 should use the kernel as its filename, not a NULL filename. I wonder if this bug is why 'pmcstat -t 0' in top mode doesn't show any events? I doubt any third party code uses kthread_suspend() or kproc_suspend(). kthread/proc_suspend() already relies on the kthread in question using kthread/proc_suspend_check() in its main loop so it doesn't work on a variety of kthreads that have P_KTHREAD set (e.g. taskqueue threads, ithreads). Note that thread0 does have TDP_KTHREAD set explicitly, so you can already do 'kthread_suspend(&thread0)' without EINVAL, just not 'kproc(&proc0)'. Both cases would hang of course since swapper() doesn't check for suspend. Given that, I think it would be more consistent to set P_KTHREAD for proc0 than to not. Also, P_KTHREAD should probably be renamed to P_KPROC. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1638099.vX3eOuueKK>