From owner-svn-src-user@FreeBSD.ORG Thu May 26 16:45:31 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 2C860106564A; Thu, 26 May 2011 16:45:31 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-gx0-f182.google.com (mail-gx0-f182.google.com [209.85.161.182]) by mx1.freebsd.org (Postfix) with ESMTP id 893028FC14; Thu, 26 May 2011 16:45:30 +0000 (UTC) Received: by gxk28 with SMTP id 28so468994gxk.13 for ; Thu, 26 May 2011 09:45:29 -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=TF5rV0+GhuHgql4zYuU6PTDaVF1aQERHWQJRZQ677ZM=; b=K701usdW/L/qqnH6RK4hBsBt5sOeqCcH06LJx1N5DGFZ8X1oPXjMeb8Q/S5db44t6T WMXOF2TJkslEuDLVzjVw96wjBcJdDJJ7zigJf1MX+NX6dMGDEQziegof/I9eZ+WRRQ2G d5IRHdXx6bIawfy7ZJpf/4Bb1/HxwXYJ+GE0g= 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=pveEvNuhuivucgh9QbVzo5C7tjiL8DdqgFD757E3jsOPsJMx7nRK5wyU+V/IFT8Vl1 zyLr7Wmm3qBxa0ZWaVBODZ/eZFUuKHLCfZrdVgKvcuLnY7Y8bzILAbQ1dro8Q/v3Ff+Z da/8vfwZStgM1hxqrf42/u3/E5xR//8+AHIJo= MIME-Version: 1.0 Received: by 10.236.151.74 with SMTP id a50mr1414828yhk.13.1306428329871; Thu, 26 May 2011 09:45:29 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.236.103.136 with HTTP; Thu, 26 May 2011 09:45:29 -0700 (PDT) In-Reply-To: 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 12:45:29 -0400 X-Google-Sender-Auth: VbRiqkMKwt7i369rAPmy1tV9tqI Message-ID: From: Attilio Rao To: Andriy Gapon Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 16:45:31 -0000 2011/5/26 Attilio Rao : > 2011/5/26 Andriy Gapon : >> 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 remove= d altogether >>> along with the check in =C2=A0choosethread(). =C2=A0But for some initia= l period I would like >>> to have an option to disable CPU stopping (to protect from possible bug= s, >>> regressions, etc) and for that I would like to keep TDF_INPANIC. =C2=A0= The flag could >>> be set without thread_lock() because we still allow only one thread to = be in/after >>> panic. =C2=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... =C2=A0After discussing this issue with you on IRC I realized absu= rdity of my >> interim suggestion. >> >> New proposal: >> #define TD_IS_INPANIC() \ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0(panicstr !=3D NULL && stop_cpus_on_panic) >> >> When/if stop_cpus_on_panic knob is removed, then TD_IS_INPANIC will natu= rally be >> reduced to (panicstr !=3D NULL) and TDF_INPANIC flag will also go as we = will be >> guaranteed that the scheduler will not be running. > > Yes, that is a much better proposal. > >> Given the above, maybe TD_IS_INPANIC should change name again as it does= n't check >> properties of a particular thread, but rather the whole system state? = =C2=A0Also, >> sys/proc.h doesn't seem like the best location for it anymore. > > Yes, I think it would be better something like SYSTEM_IN_PANIC() or such. > The natural location for this would be kern_shutdown.c but it really > doesn't have a corresponding header (maybe sys/reboot.h could be, with > some more lifting, but for the moment, no), thus you can still pickup > something easy to use like proc.h or systm.h. I think that systm.h may be the best place as long as it is where panic() is declared. Attilio --=20 Peace can only be achieved by understanding - A. Einstein