From owner-svn-src-user@FreeBSD.ORG Wed May 25 14:36:44 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 537E31065674; Wed, 25 May 2011 14:36:44 +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 11EAB8FC1D; Wed, 25 May 2011 14:36:42 +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 RAA29335; Wed, 25 May 2011 17:36:41 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <4DDD13F9.5040800@FreeBSD.org> Date: Wed, 25 May 2011 17:36:41 +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> In-Reply-To: X-Enigmail-Version: 1.1.2 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: 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: Wed, 25 May 2011 14:36:44 -0000 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 -- Andriy Gapon From owner-svn-src-user@FreeBSD.ORG Wed May 25 14:52:17 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 5F863106564A; Wed, 25 May 2011 14:52:17 +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 DAE748FC0C; Wed, 25 May 2011 14:52:15 +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 RAA29653; Wed, 25 May 2011 17:52:13 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <4DDD179D.5030901@FreeBSD.org> Date: Wed, 25 May 2011 17:52:13 +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: mdf@FreeBSD.org References: <201105181508.p4IF8UoS096841@svn.freebsd.org> <20110518182441.GB2273@garage.freebsd.pl> <4DD4243C.4070301@FreeBSD.org> In-Reply-To: X-Enigmail-Version: 1.1.2 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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: Wed, 25 May 2011 14:52:17 -0000 on 18/05/2011 23:43 mdf@FreeBSD.org said the following: > I know it's almost required now (sync on reboot?!?!), but I would > strongly question, from an architectural standpoint, why the scheduler > should be running at all in panic. Once a thread pulls the panic > trigger, nothing else should run except ddb in that thread's context. I agree, but as I've said in other email I would prefer to keep a backwards-compatibility knob for some time. Personally I don't like things like sync-on-panic, but other people may need some transition time. -- Andriy Gapon 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 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 From owner-svn-src-user@FreeBSD.ORG Thu May 26 16:05:14 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 2A440106566C; Thu, 26 May 2011 16:05:14 +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 A8BF58FC13; Thu, 26 May 2011 16:05:12 +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 TAA22972; Thu, 26 May 2011 19:05:11 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <4DDE7A36.2050104@FreeBSD.org> Date: Thu, 26 May 2011 19:05:10 +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: mdf@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> In-Reply-To: X-Enigmail-Version: 1.1.2 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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 16:05:14 -0000 on 26/05/2011 18:46 mdf@FreeBSD.org said the following: > 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. I am not sure that I understand your reasoning if you mean that the flag needs to be checked in TD_IS_INPANIC. That is, right now there is no TD_IS_INPANIC and things work after panic to a certain degree. I do not intend to improve that degree and just want to keep an option to revert to the current state of matters. When TD_IS_INPANIC is introduced and stop_cpus_on_panic==1, then there will be only one thread left running after panic, that will be the thread that called panic, checking TDF_INPANIC just doesn't add anything. -- Andriy Gapon From owner-svn-src-user@FreeBSD.ORG Thu May 26 16:18:05 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 EFF82106564A; Thu, 26 May 2011 16:18:05 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id D8A4D8FC12; Thu, 26 May 2011 16:18:04 +0000 (UTC) Received: by wyf23 with SMTP id 23so864375wyf.13 for ; Thu, 26 May 2011 09:18:03 -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=Gz449z3QiVj9y7Kb0EzkB5Rcpr7RQBi5bMw19Fqidc0=; b=Gn2AGlHe1HgtkXWH+3UoPojck7quLM/YB9Q7tOMFWHkwBceC2QBE/Y77zegigPf9vh MjDwwxXaPjrhPiK94FI7Og0UxWfU5M0o/TedJnD/ddDcrf+j9Y5ZYAW1B+ltzO7A4HNm QaSB0NUC7gVTD/9fWst9TTIP5Hjr0R+Zyd4QI= 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=Q0n2Drb6CmalQqt5fCEiLRAg9UauzDMqevNzYXwU+S2ldPr7G36Zn1NfLYNY2bks8j zyKJg57GPUbqQn4x8gZer9pOT6jTttZFef9U4djiZEDX4snG+e2fG2EkbxVjdQQgIt3Q DPbssB2FvDDcM3HPG/OJ7ssQUlcMYl5pWCOCw= MIME-Version: 1.0 Received: by 10.216.141.1 with SMTP id f1mr1039402wej.35.1306426683665; Thu, 26 May 2011 09:18:03 -0700 (PDT) Sender: mdf356@gmail.com Received: by 10.216.93.193 with HTTP; Thu, 26 May 2011 09:18:03 -0700 (PDT) In-Reply-To: <4DDE7A36.2050104@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> <4DDE7A36.2050104@FreeBSD.org> Date: Thu, 26 May 2011 09:18:03 -0700 X-Google-Sender-Auth: UFZeO5faqcCMpys_XVKVqvuEAd8 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 16:18:06 -0000 On Thu, May 26, 2011 at 9:05 AM, Andriy Gapon wrote: > on 26/05/2011 18:46 mdf@FreeBSD.org said the following: >> A per-thread flag is needed as long as other CPUs can be running or >> even just the scheduler on the remaining CPU. =A0So I would thing that >> flag needs to be checked until the system has been massaged to the >> state you describe above. > > I am not sure that I understand your reasoning if you mean that the flag = needs to > be checked in TD_IS_INPANIC. =A0That is, right now there is no TD_IS_INPA= NIC and > things work after panic to a certain degree. =A0I do not intend to improv= e that > degree and just want to keep an option to revert to the current state of = matters. > When TD_IS_INPANIC is introduced and stop_cpus_on_panic=3D=3D1, then ther= e will be > only one thread left running after panic, that will be the thread that ca= lled > panic, checking TDF_INPANIC just doesn't add anything. Won't the scheduler still run even if other CPUs are halted? Is there any intent to prevent switching to another thread? (I suppose this could be achieved by setting td_critnest++ and wouldn't require a flag). It's possible I misunderstood your email so my response may have been non-sensical. Thanks, matthew From owner-svn-src-user@FreeBSD.ORG Thu May 26 16:31: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 6BAB91065688; Thu, 26 May 2011 16:31:01 +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 E9E418FC13; Thu, 26 May 2011 16:30:59 +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 TAA23285; Thu, 26 May 2011 19:30:58 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <4DDE8041.7060903@FreeBSD.org> Date: Thu, 26 May 2011 19:30:57 +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: mdf@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> <4DDE7A36.2050104@FreeBSD.org> In-Reply-To: X-Enigmail-Version: 1.1.2 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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 16:31:01 -0000 on 26/05/2011 19:18 mdf@FreeBSD.org said the following: > On Thu, May 26, 2011 at 9:05 AM, Andriy Gapon wrote: >> on 26/05/2011 18:46 mdf@FreeBSD.org said the following: >>> 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. >> >> I am not sure that I understand your reasoning if you mean that the flag needs to >> be checked in TD_IS_INPANIC. That is, right now there is no TD_IS_INPANIC and >> things work after panic to a certain degree. I do not intend to improve that >> degree and just want to keep an option to revert to the current state of matters. >> When TD_IS_INPANIC is introduced and stop_cpus_on_panic==1, then there will be >> only one thread left running after panic, that will be the thread that called >> panic, checking TDF_INPANIC just doesn't add anything. > > Won't the scheduler still run even if other CPUs are halted? Is there > any intent to prevent switching to another thread? (I suppose this > could be achieved by setting td_critnest++ and wouldn't require a > flag). > > It's possible I misunderstood your email so my response may have been > non-sensical. No, no, it does make sense. I haven't mentioned this, but yes, new world order with stop_cpus_on_panic==1 implies disabling interrupts for a panic thread. I also took it to mean that there will not be any thread switches, but perhaps I am mistaken here... In any case my intention is to ensure that only the panic thread runs. -- Andriy Gapon From owner-svn-src-user@FreeBSD.ORG Thu May 26 16:36:16 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 6BBFB106567A; Thu, 26 May 2011 16:36:16 +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 C7AF48FC12; Thu, 26 May 2011 16:36:15 +0000 (UTC) Received: by gxk28 with SMTP id 28so464290gxk.13 for ; Thu, 26 May 2011 09:36:15 -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=jNEbaKvQfuHH5sG1cvW9B5NDK1Zid4Pj5gCoeuxMqwg=; b=xAFpKi1OGMcFdvYmlt0A7vq/tqk4H/cKy5uk7NNb/pym1wHTkS0BnR10QTaFf7txtT MkTNudKTKQQRCvBIEHxtvWg5/3LLr7OOAznCXfMa2yGUc0L8LGTqPwx0yyK9XtaHpEsz oLafpc5SA2LYqz7lPppIm5gjFcaRHHml4qIYQ= 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=scfpGk9ZXI3FIGa45FH9YfA5mvKXd3lskYO58QvnxlLScIsRtoFBE7tqQux1asvIme nQgJDJm19l9NdDwgL9XR9xbbtPBSzv0OIeDECc5kPn5n+yZs7EOY7PuK6vmP/ImH/3JS hMQ5SoyBG4zuTA7NfSeROut5QUbaVO/Zj8kbQ= MIME-Version: 1.0 Received: by 10.236.186.38 with SMTP id v26mr1376500yhm.415.1306427773531; Thu, 26 May 2011 09:36:13 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.236.103.136 with HTTP; Thu, 26 May 2011 09:36:13 -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> <4DDE7A36.2050104@FreeBSD.org> Date: Thu, 26 May 2011 12:36:13 -0400 X-Google-Sender-Auth: ic8IO-PZowuW33McfGkUX5gMNEY Message-ID: From: Attilio Rao To: mdf@freebsd.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: src-committers@freebsd.org, Pawel Jakub Dawidek , Andriy Gapon , 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:36:16 -0000 2011/5/26 : > On Thu, May 26, 2011 at 9:05 AM, Andriy Gapon wrote: >> on 26/05/2011 18:46 mdf@FreeBSD.org said the following: >>> A per-thread flag is needed as long as other CPUs can be running or >>> even just the scheduler on the remaining CPU. =C2=A0So I would thing th= at >>> flag needs to be checked until the system has been massaged to the >>> state you describe above. >> >> I am not sure that I understand your reasoning if you mean that the flag= needs to >> be checked in TD_IS_INPANIC. =C2=A0That is, right now there is no TD_IS_= INPANIC and >> things work after panic to a certain degree. =C2=A0I do not intend to im= prove that >> degree and just want to keep an option to revert to the current state of= matters. >> When TD_IS_INPANIC is introduced and stop_cpus_on_panic=3D=3D1, then the= re will be >> only one thread left running after panic, that will be the thread that c= alled >> panic, checking TDF_INPANIC just doesn't add anything. > > Won't the scheduler still run even if other CPUs are halted? =C2=A0Is the= re > any intent to prevent switching to another thread? =C2=A0(I suppose this > could be achieved by setting td_critnest++ and wouldn't require a > flag). > I think it is much better to disable interrupts, in order to prevent fast handlers "preemption" rather than just avoiding to be rescheduled. Attilio --=20 Peace can only be achieved by understanding - A. Einstein From owner-svn-src-user@FreeBSD.ORG Thu May 26 16:41:26 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 9449F106566C; Thu, 26 May 2011 16:41:26 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-gw0-f54.google.com (mail-gw0-f54.google.com [74.125.83.54]) by mx1.freebsd.org (Postfix) with ESMTP id B88568FC13; Thu, 26 May 2011 16:41:25 +0000 (UTC) Received: by gwb15 with SMTP id 15so468035gwb.13 for ; Thu, 26 May 2011 09:41:24 -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=LYUdghP48V05/IOUPn7wBhx0/FNHZ+40QrHilnM9Pyw=; b=bJIY1+viQcxiz2Bc+uHxHSnPxLu7+ED3l5VzHzYLXgptmaFhRC/OArqY1C+BZnq4b2 Qt6rHxAWx3hjHr+Dlk9TsetDQclZg54NuokMKFnR2olsXnR4HrCiONjOi0xqmD8SNI7p 892EA8d+v/d4SpLSvdoxBTG4XQsiLzaQ+ZrXY= 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=TWrqN+2sy+4O7r4y3tSFqq8ee1EIJ3f05qPExaI8G3qG19hsHJTjV79HulgVOA6erA 2/nvBj5uo2AIUju1z5TaNj8ASMp+vgDDetORsFzRhOJbvMZtz5IgMgLaXVwRtotzI8y0 WnKTBAVpY68H0+GScyi1KLREBREdX3TiIihRg= MIME-Version: 1.0 Received: by 10.236.183.193 with SMTP id q41mr1499355yhm.80.1306428084874; Thu, 26 May 2011 09:41:24 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.236.103.136 with HTTP; Thu, 26 May 2011 09:41:24 -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 12:41:24 -0400 X-Google-Sender-Auth: PuCkw6lDARpOn_F7782Q38cVXC4 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:41:26 -0000 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 removed= altogether >> along with the check in =C2=A0choosethread(). =C2=A0But 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. =C2=A0T= he flag could >> be set without thread_lock() because we still allow only one thread to b= e 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 absur= dity 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 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. Yes, that is a much better proposal. > 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? =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. It is your decision, anyway, I don't have strong opinion on where to put th= is. Attilio --=20 Peace can only be achieved by understanding - A. Einstein 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 From owner-svn-src-user@FreeBSD.ORG Fri May 27 06:30:05 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 52052106564A; Fri, 27 May 2011 06:30:05 +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 C4F0F8FC16; Fri, 27 May 2011 06:30:03 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id JAA02747; Fri, 27 May 2011 09:30:01 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1QPqYb-0003NO-Cp; Fri, 27 May 2011 09:30:01 +0300 Message-ID: <4DDF44E8.9030208@FreeBSD.org> Date: Fri, 27 May 2011 09:30:00 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.17) Gecko/20110503 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> <4DDE7555.7090500@FreeBSD.org> In-Reply-To: X-Enigmail-Version: 1.1.2 Content-Type: text/plain; charset=UTF-8 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: Fri, 27 May 2011 06:30:05 -0000 on 26/05/2011 19:41 Attilio Rao said the following: > Yes, I think it would be better something like SYSTEM_IN_PANIC() or such. How about SCHEDULER_STOPPED() ? -- Andriy Gapon From owner-svn-src-user@FreeBSD.ORG Sat May 28 08:37:03 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 C2183106566B; Sat, 28 May 2011 08:37:03 +0000 (UTC) (envelope-from gabor@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id B2AA38FC14; Sat, 28 May 2011 08:37:03 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p4S8b3SQ075849; Sat, 28 May 2011 08:37:03 GMT (envelope-from gabor@svn.freebsd.org) Received: (from gabor@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p4S8b3oJ075844; Sat, 28 May 2011 08:37:03 GMT (envelope-from gabor@svn.freebsd.org) Message-Id: <201105280837.p4S8b3oJ075844@svn.freebsd.org> From: Gabor Kovesdan Date: Sat, 28 May 2011 08:37:03 +0000 (UTC) To: src-committers@freebsd.org, svn-src-user@freebsd.org X-SVN-Group: user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r222414 - user/gabor/tre-integration/usr.bin/grep 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: Sat, 28 May 2011 08:37:03 -0000 Author: gabor Date: Sat May 28 08:37:03 2011 New Revision: 222414 URL: http://svn.freebsd.org/changeset/base/222414 Log: - Use libc in grep instead of GNU regex - Eliminate literal matching code and depend on TRE's REG_LITERAL Deleted: user/gabor/tre-integration/usr.bin/grep/fastgrep.c Modified: user/gabor/tre-integration/usr.bin/grep/Makefile user/gabor/tre-integration/usr.bin/grep/grep.c user/gabor/tre-integration/usr.bin/grep/grep.h user/gabor/tre-integration/usr.bin/grep/util.c Modified: user/gabor/tre-integration/usr.bin/grep/Makefile ============================================================================== --- user/gabor/tre-integration/usr.bin/grep/Makefile Sat May 28 08:34:30 2011 (r222413) +++ user/gabor/tre-integration/usr.bin/grep/Makefile Sat May 28 08:37:03 2011 (r222414) @@ -3,7 +3,7 @@ # $OpenBSD: Makefile,v 1.6 2003/06/25 15:00:04 millert Exp $ PROG= grep -SRCS= fastgrep.c file.c grep.c queue.c util.c +SRCS= file.c grep.c queue.c util.c LINKS= ${BINDIR}/grep ${BINDIR}/egrep \ ${BINDIR}/grep ${BINDIR}/fgrep \ @@ -22,12 +22,6 @@ WARNS?= 6 LDADD= -lz -lbz2 DPADD= ${LIBZ} ${LIBBZ2} -.if !defined(WITHOUT_GNU_COMPAT) -CFLAGS+= -I/usr/include/gnu -LDADD+= -lgnuregex -DPADD+= ${LIBGNUREGEX} -.endif - .if !defined(WITHOUT_NLS) .include "${.CURDIR}/nls/Makefile.inc" .else Modified: user/gabor/tre-integration/usr.bin/grep/grep.c ============================================================================== --- user/gabor/tre-integration/usr.bin/grep/grep.c Sat May 28 08:34:30 2011 (r222413) +++ user/gabor/tre-integration/usr.bin/grep/grep.c Sat May 28 08:37:03 2011 (r222414) @@ -83,7 +83,6 @@ bool matchall; unsigned int patterns, pattern_sz; char **pattern; regex_t *r_pattern; -fastgrep_t *fg_pattern; /* Filename exclusion/inclusion patterns */ unsigned int fpatterns, fpattern_sz; @@ -638,6 +637,8 @@ main(int argc, char *argv[]) switch (grepbehave) { case GREP_FIXED: + cflags |= REG_LITERAL; + break; case GREP_BASIC: break; case GREP_EXTENDED: @@ -648,27 +649,14 @@ main(int argc, char *argv[]) usage(); } - fg_pattern = grep_calloc(patterns, sizeof(*fg_pattern)); r_pattern = grep_calloc(patterns, sizeof(*r_pattern)); -/* - * XXX: fgrepcomp() and fastcomp() are workarounds for regexec() performance. - * Optimizations should be done there. - */ - /* Check if cheating is allowed (always is for fgrep). */ - if (grepbehave == GREP_FIXED) { - for (i = 0; i < patterns; ++i) - fgrepcomp(&fg_pattern[i], pattern[i]); - } else { - for (i = 0; i < patterns; ++i) { - if (fastcomp(&fg_pattern[i], pattern[i])) { - /* Fall back to full regex library */ - c = regcomp(&r_pattern[i], pattern[i], cflags); - if (c != 0) { - regerror(c, &r_pattern[i], re_error, - RE_ERROR_BUF); - errx(2, "%s", re_error); - } - } + + for (i = 0; i < patterns; ++i) { + c = regcomp(&r_pattern[i], pattern[i], cflags); + if (c != 0) { + regerror(c, &r_pattern[i], re_error, + RE_ERROR_BUF); + errx(2, "%s", re_error); } } Modified: user/gabor/tre-integration/usr.bin/grep/grep.h ============================================================================== --- user/gabor/tre-integration/usr.bin/grep/grep.h Sat May 28 08:34:30 2011 (r222413) +++ user/gabor/tre-integration/usr.bin/grep/grep.h Sat May 28 08:37:03 2011 (r222414) @@ -95,17 +95,6 @@ struct epat { int mode; }; -typedef struct { - size_t len; - unsigned char *pattern; - int qsBc[UCHAR_MAX + 1]; - /* flags */ - bool bol; - bool eol; - bool reversed; - bool word; -} fastgrep_t; - /* Flags passed to regcomp() and regexec() */ extern int cflags, eflags; @@ -125,7 +114,6 @@ extern unsigned int dpatterns, fpatterns extern char **pattern; extern struct epat *dpattern, *fpattern; extern regex_t *er_pattern, *r_pattern; -extern fastgrep_t *fg_pattern; /* For regex errors */ #define RE_ERROR_BUF 512 @@ -150,8 +138,3 @@ void clearqueue(void); void grep_close(struct file *f); struct file *grep_open(const char *path); char *grep_fgetln(struct file *f, size_t *len); - -/* fastgrep.c */ -int fastcomp(fastgrep_t *, const char *); -void fgrepcomp(fastgrep_t *, const char *); -int grep_search(fastgrep_t *, const unsigned char *, size_t, regmatch_t *); Modified: user/gabor/tre-integration/usr.bin/grep/util.c ============================================================================== --- user/gabor/tre-integration/usr.bin/grep/util.c Sat May 28 08:34:30 2011 (r222413) +++ user/gabor/tre-integration/usr.bin/grep/util.c Sat May 28 08:37:03 2011 (r222414) @@ -297,22 +297,10 @@ procline(struct str *l, int nottext) /* Loop to compare with all the patterns */ for (i = 0; i < patterns; i++) { -/* - * XXX: grep_search() is a workaround for speed up and should be - * removed in the future. See fastgrep.c. - */ - if (fg_pattern[i].pattern) { - r = grep_search(&fg_pattern[i], - (unsigned char *)l->dat, - l->len, &pmatch); - r = (r == 0) ? 0 : REG_NOMATCH; - st = pmatch.rm_eo; - } else { - r = regexec(&r_pattern[i], l->dat, 1, - &pmatch, eflags); - r = (r == 0) ? 0 : REG_NOMATCH; - st = pmatch.rm_eo; - } + r = regexec(&r_pattern[i], l->dat, 1, + &pmatch, eflags); + r = (r == 0) ? 0 : REG_NOMATCH; + st = pmatch.rm_eo; if (r == REG_NOMATCH) continue; /* Check for full match */ @@ -321,7 +309,7 @@ procline(struct str *l, int nottext) (size_t)pmatch.rm_eo != l->len) r = REG_NOMATCH; /* Check for whole word match */ - if (r == 0 && fg_pattern[i].word && + if (r == 0 && wflag && pmatch.rm_so != 0) { wint_t wbegin, wend;