From owner-svn-src-user@FreeBSD.ORG Thu May 26 15:47:01 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 368991065673; Thu, 26 May 2011 15:47:01 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-ww0-f50.google.com (mail-ww0-f50.google.com [74.125.82.50]) by mx1.freebsd.org (Postfix) with ESMTP id 214068FC0C; Thu, 26 May 2011 15:46:59 +0000 (UTC) Received: by wwc33 with SMTP id 33so867497wwc.31 for ; Thu, 26 May 2011 08:46:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=NE3fBL/uHJ24N74sOM0imYZ7kiXqlkaeLJiFT83fQ+s=; b=ao3eBFIjvERNHLSz+IWvfeIvJi/YrF3MRHr7k3EOCQ3zZYsfgEa9O0ObxIVz4PVnG7 bE3IdCYC9K83/UmUMqBF66hkOMKnYvbxed/alleCB+mk2KED3ftLx/a6JkmUKDI/1XIP cyd6LqIa9uV6xtNpYpsU/E10c54Eq9wjtMWBY= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=BS4msEz3Q4K4U8Za/TbpZQpVHlZtT+wYG4jq1ntO/qWplBjSOTl9bvJP0xWkVjemSd 47VpyOHdsoevAQiQzcoSnne6SjfR7E8kTzM4/ChtKUwEUWYM+anf96YC6E7sCplkOQV+ ECJIbsBdZ5L6+9WLClvfBH79chRFRCJABBpPc= MIME-Version: 1.0 Received: by 10.216.79.10 with SMTP id h10mr998471wee.20.1306424818991; Thu, 26 May 2011 08:46:58 -0700 (PDT) Sender: mdf356@gmail.com Received: by 10.216.93.193 with HTTP; Thu, 26 May 2011 08:46:58 -0700 (PDT) In-Reply-To: <4DDE7555.7090500@FreeBSD.org> References: <201105181508.p4IF8UoS096841@svn.freebsd.org> <20110518182441.GB2273@garage.freebsd.pl> <4DD4243C.4070301@FreeBSD.org> <4DDD13F9.5040800@FreeBSD.org> <4DDE7555.7090500@FreeBSD.org> Date: Thu, 26 May 2011 08:46:58 -0700 X-Google-Sender-Auth: htrgpNocUVaTB-Zbf7kUUUUjSgQ Message-ID: From: mdf@FreeBSD.org To: Andriy Gapon Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Attilio Rao , 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:47:01 -0000 On Thu, May 26, 2011 at 8:44 AM, Andriy Gapon wrote: > 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 !=3D NULL, TDF_INPANIC could be removed= altogether >> along with the check in =A0choosethread(). =A0But for some initial perio= d 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. =A0The = flag could >> be set without thread_lock() because we still allow only one thread to b= e in/after >> panic. =A0But I completely agree with you that it is cleaner to move TDF= _INPANIC to >> private flags. >> >> So the first step: >> TDF_INPANIC =3D> to private flags >> >> Some time in the future: >> TDF_INPANIC =3D> removed >> TD_IS_INPANIC =3D> panicstr !=3D NULL >> > > Ehm... =A0After discussing this issue with you on IRC I realized absurdit= y of my > interim suggestion. > > New proposal: > #define TD_IS_INPANIC() \ > =A0 =A0 =A0 =A0(panicstr !=3D NULL && stop_cpus_on_panic) > > When/if stop_cpus_on_panic knob is removed, then TD_IS_INPANIC will natur= ally be > reduced to (panicstr !=3D NULL) and TDF_INPANIC flag will also go as we w= ill 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? =A0= Also, > sys/proc.h doesn't seem like the best location for it anymore. A per-thread flag is needed as long as other CPUs can be running or even just the scheduler on the remaining CPU. So I would thing that flag needs to be checked until the system has been massaged to the state you describe above. Cheers, matthew