Date: Thu, 7 Mar 2002 19:04:49 -0800 (PST) From: Matthew Dillon <dillon@apollo.backplane.com> To: Jake Burkholder <jake@locore.ca> Cc: Robert Watson <rwatson@FreeBSD.ORG>, FreeBSD current users <current@FreeBSD.ORG> Subject: Re: Patch for critical_enter()/critical_exit() & interrupt assem Message-ID: <200203080304.g2834n571759@apollo.backplane.com> References: <200203072143.g27LhaL97112@harmony.village.org> <Pine.NEB.3.96L.1020307171254.23264D-100000@fledge.watson.org> <20020307205844.C12044@locore.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
:Yes. What I would like and what I mentioned before is for this to be
:hidden behind cpu_critical_enter/exit. Specifically, cpu_critical_enter
:would be a null macro for i386, which would turn critical_enter into
:just an increment, as Matt wanted. cpu_critical_exit would do all the
:magic of unpending interrupts. The reason for this is that I would
:like to see critical_exit handled any pending preemptions for a thread.
:We do not yet know exactly how this will work so I would like this to be
:done in MI code to start with. The code must not make assumptions that are
:not valid on all platforms. This is easiest if they use the same code.
:This is not about duplication of code in several MD files.
We can't, because the MI code makes some rather blatent assumptions
in regards to cpu_critical_enter and exit. Take the witness code,
for example. So we cannot safely null-out those macros. In fact,
the MI code also makes some rather blatent assumptions in regards
to critical_enter() and exit which I have also had to clean up
(and which will need considerably more cleanup).
These assumptions include, just as an example: The witness code
calling cpu_critical_enter()/exit without holding td_critnest count,
and Peter's now withdrawn code which explicitly released the cpu
critical section while holding a non-zero td_critnest count. In
otherwords, really aweful hacks. So we can't just NULL out those
inlines.
Trying to keep critical_enter()/critical_exit() MI and
cpu_critical_enter()/cpu_critical_exit() MD and separated doesn't
make much sense to me, because frankly critical_enter() and
critical_exit() are tiny little routines (and will remain tiny even
after SWI and preemption is added) and I can't think of any reason
to force higher complexity in the routines due to the separation.
In short, critical_enter()/critical_exit() are TIGHTLY INTEGRATED
with cpu_critical_enter/exit despite your attempt to make
critical_enter/exit MI. What my patch does is accept as true the
fact that the two sets of routines are, in fact, tightly integrated,
and then implements them in a more sensible way (or, more to the
point, allows each architecture to implement them in the proper
manner as befits the architecture).
The API's are still MI, but the implementation is MD.
:Bruce also had some comments which were shrugged off, I thought they
:were important. Specifically, please do not make unnecessary changes
:to the assembler code. Macros do not need to be defined before they
:are used, I believe this was the justification for some of the reordering
:in apic_vector.s which makes the patch confusing. Please do not tab
:out the "; \" at the end of the lines in the INTR and FAST_INTR macros
:in icu_vector.s. This just makes unnecessary diffs. The PUSH_DUMMY
:macro must push a reasonable eip value, in addition to the code segment,
:so that profiling and stack traces work right. If you have not already,
:please make sure that a trace from inside an interrupt handler that was
:unpended looks somewhat normal. Please also make sure that kgdb is able
:to decode the frame properly. It assumes that the eip of the frame will
:be near certain kernel symbols in order to determine what kind of frame
:it is.
:
:Jake
Actually all I did there was square up icu_vector.s so it looked
almost the same as apic_vector.s. I would consider that an improvement.
Besides, I find it very difficult to work on those macros without
tabbing out the backslashes. I moved some of the #defines around due
to it otherwise being a forward reference. This turned out not to be
an issue since the macros aren't actually used until later in the code,
but I don't like forward referenced macros anyway so I left it in.
Insofar as diffs go, I'm afraid even the most conservative changes
to the assembly to support interrupt deferral would make for a pretty
big diff set.
I'm not going to change them now, no, but I don't see this as being
a show stopper in the least.
-Matt
Matthew Dillon
<dillon@backplane.com>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200203080304.g2834n571759>