Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 26 May 2011 18:44:21 +0300
From:      Andriy Gapon <avg@FreeBSD.org>
To:        Attilio Rao <attilio@FreeBSD.org>
Cc:        Matthew Fleming <mdf@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:  <4DDE7555.7090500@FreeBSD.org>
In-Reply-To: <4DDD13F9.5040800@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>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DDE7555.7090500>