Date: Thu, 26 May 2011 08:46:58 -0700 From: mdf@FreeBSD.org To: Andriy Gapon <avg@freebsd.org> Cc: Attilio Rao <attilio@freebsd.org>, src-committers@freebsd.org, Pawel Jakub Dawidek <pjd@freebsd.org>, svn-src-user@freebsd.org Subject: Re: svn commit: r222060 - in user/avg/xcpu/sys: kern sys Message-ID: <BANLkTim_zEDPANqZTpyYdOKqDaPEc8EhVg@mail.gmail.com> In-Reply-To: <4DDE7555.7090500@FreeBSD.org> References: <201105181508.p4IF8UoS096841@svn.freebsd.org> <20110518182441.GB2273@garage.freebsd.pl> <4DD4243C.4070301@FreeBSD.org> <BANLkTikAnB-3XbvDwGHgyqyJquH9BhqzOQ@mail.gmail.com> <4DDD13F9.5040800@FreeBSD.org> <4DDE7555.7090500@FreeBSD.org>
index | next in thread | previous in thread | raw e-mail
On Thu, May 26, 2011 at 8:44 AM, Andriy Gapon <avg@freebsd.org> 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 != 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. 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, matthewhome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTim_zEDPANqZTpyYdOKqDaPEc8EhVg>