From owner-freebsd-current@freebsd.org Mon Feb 8 18:37:08 2016 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 87A27AA1A20 for ; Mon, 8 Feb 2016 18:37:08 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 66DD91F0A for ; Mon, 8 Feb 2016 18:37:08 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id AFB54B91F; Mon, 8 Feb 2016 13:37:05 -0500 (EST) From: John Baldwin To: freebsd-current@freebsd.org Cc: Konstantin Belousov , Oliver Pinter Subject: Re: Broken suspend-resume (suspend to RAM) with enabled INVARIANTS on 11-CURRENT - with workaround Date: Mon, 08 Feb 2016 10:36:59 -0800 Message-ID: <1638099.vX3eOuueKK@ralph.baldwin.cx> User-Agent: KMail/4.14.3 (FreeBSD/10.2-STABLE; KDE/4.14.3; amd64; ; ) In-Reply-To: <20160208105230.GG91220@kib.kiev.ua> References: <20160208105230.GG91220@kib.kiev.ua> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 08 Feb 2016 13:37:05 -0500 (EST) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Feb 2016 18:37:08 -0000 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