From owner-svn-src-user@FreeBSD.ORG Thu May 26 15:44:24 2011 Return-Path: Delivered-To: svn-src-user@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B6ED8106564A; Thu, 26 May 2011 15:44:24 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 395BC8FC16; Thu, 26 May 2011 15:44:22 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id SAA22624; Thu, 26 May 2011 18:44:21 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <4DDE7555.7090500@FreeBSD.org> Date: Thu, 26 May 2011 18:44:21 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.17) Gecko/20110504 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: Attilio Rao References: <201105181508.p4IF8UoS096841@svn.freebsd.org> <20110518182441.GB2273@garage.freebsd.pl> <4DD4243C.4070301@FreeBSD.org> <4DDD13F9.5040800@FreeBSD.org> In-Reply-To: <4DDD13F9.5040800@FreeBSD.org> X-Enigmail-Version: 1.1.2 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: Matthew Fleming , src-committers@FreeBSD.org, Pawel Jakub Dawidek , svn-src-user@FreeBSD.org Subject: Re: svn commit: r222060 - in user/avg/xcpu/sys: kern sys X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 May 2011 15:44:24 -0000 on 25/05/2011 17:36 Andriy Gapon said the following: > on 18/05/2011 23:06 Attilio Rao said the following: >> However I think that TDF_INPANIC handling is not optimal. >> You should really acquire thread_lock otherwise you are going to break >> choosethread() concurrency. >> >> I would prefer to make TDF_INPANIC a private flag and just use it with >> curthread, if possible, but I still don't have a good way to resolve >> choosethread() (I would dig the runqueue adding path and resolve the >> problem later in the codeflow, I think). > > I've been thinking about this. > I think that in the new world where only one thread runs after panic we could just > reduce TD_IS_INPANIC to panicstr != NULL, TDF_INPANIC could be removed altogether > along with the check in choosethread(). But for some initial period I would like > to have an option to disable CPU stopping (to protect from possible bugs, > regressions, etc) and for that I would like to keep TDF_INPANIC. The flag could > be set without thread_lock() because we still allow only one thread to be in/after > panic. But I completely agree with you that it is cleaner to move TDF_INPANIC to > private flags. > > So the first step: > TDF_INPANIC => to private flags > > Some time in the future: > TDF_INPANIC => removed > TD_IS_INPANIC => panicstr != NULL > Ehm... After discussing this issue with you on IRC I realized absurdity of my interim suggestion. New proposal: #define TD_IS_INPANIC() \ (panicstr != NULL && stop_cpus_on_panic) When/if stop_cpus_on_panic knob is removed, then TD_IS_INPANIC will naturally be reduced to (panicstr != NULL) and TDF_INPANIC flag will also go as we will be guaranteed that the scheduler will not be running. Given the above, maybe TD_IS_INPANIC should change name again as it doesn't check properties of a particular thread, but rather the whole system state? Also, sys/proc.h doesn't seem like the best location for it anymore. -- Andriy Gapon