Date: Mon, 25 Feb 2002 01:00:31 -0500 From: Jake Burkholder <jake@locore.ca> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: current@FreeBSD.ORG Subject: Re: Patch for critical_enter()/critical_exit() & interrupt assembly revamp, please review! Message-ID: <20020225010031.A43767@locore.ca> In-Reply-To: <200202250503.g1P53rd29132@apollo.backplane.com>; from dillon@apollo.backplane.com on Sun, Feb 24, 2002 at 09:03:53PM -0800 References: <20020224131027.I31343-100000@gamplex.bde.org> <200202241912.g1OJCMx95238@apollo.backplane.com> <20020224224927.D35990@locore.ca> <200202250503.g1P53rd29132@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Apparently, On Sun, Feb 24, 2002 at 09:03:53PM -0800,
Matthew Dillon said words to the effect of;
> :...
> :interrupts; hard interrupts are rarely masked. The queueing is written
> :in assmebler and runs outside of the kernel so it is fast. Traps in
> :general are fast because they don't touch memory until the trapframe
> :is written out, so I don't see much point in changing this do the masking
> :in software and avoid the soft interrupt.
>
> I have no idea what you are talking about Jake. Could you supply
> some context?
Sorry, maybe I got ahead of myself. I was responding to your suggestion
that other architectures should pick up this design. This does not make
sense for sparc64.
>
> :In regards to the patch I think that making critical_enter/exit MD in
> :certain cases is a mistake. cpu_critical_enter/exit are already MD
> :and it looks you could hide this behind them with some minor changes.
> :
> :Like in the critical_enter case, make cpu_critical_enter take a thread
> :as its argument. It could be a null macro in the i386 case, which would
> :optimize out the test.
> :
> :if (td->td_critnest == 0)
> : cpu_critical_enter(td);
> :td->td_critnest++;
> :
> :Likewise with cpu_critical_exit, make it take the thread as its argument.
> :
> :If (td->td_critnest == 1) {
> : td->td_critnest = 0;
> : cpu_critical_exit(td);
> :} else
> : td->td_critnest--;
> :(This is equivalent to if (--td->td_critnest == 0), its a matter of taste.)
>
> I'm afraid I have to strongly disagree. I believe that
> critical_enter()/exit is *ALREADY* being severely misused in current.
> For example, fork_exit() makes assumptions about the underlying hardware
> in order to initialize td_critnest and the scheduler mutex safely. This
> is just plain broken. fork_exit() should not make any such assumptions.
> It is the clear responsibility of either the trampoline code or
> cpu_fork() to properly set up td_critnest. One can make an argument
> that the sched_lock fixup is in fork_exit()'s domain but you cannot
> make the argument that the initialization of critnest (and any associated
> hardware state) is MI. It just isn't.
>
> Additionally, critical_enter()/exit themselves are certainly much
> more MD then MI. It makes little sense to abstract out an MI interface
> that forces unnecessary overhead when all you have to do is define an
> MD API that has a few MI requirements, like td_critnest being a critical
> nesting count that the MI code can count on. But as MI code the existing
> critical_enter()/exit impose additional MD fields and do not give the
> MD code the option of not using them (except in a degenerate fashion).
> Specifically, the use of the td_savecrit field is under the partial
> contorl of the MI code when it shouldn't be, not just in fork_exit()
> but also in critical_enter() and critical_exit() themselves.
>
> The existing critical_enter/exit code is far too abstracted for its
> low-level purpose and I *WILL* be moving them into MD sections of the
> system where they belong. These are really MD routines, not MI routines.
Fine. As long as their functionality with respect to MI code does not
change I don't care.
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?20020225010031.A43767>