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>