Date: Sun, 24 Feb 2002 22:49:27 -0500 From: Jake Burkholder <jake@locore.ca> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: Bruce Evans <bde@zeta.org.au>, Terry Lambert <tlambert2@mindspring.com>, Alfred Perlstein <alfred@FreeBSD.ORG>, Bosko Milekic <bmilekic@unixdaemons.com>, Seigo Tanimura <tanimura@r.dl.itc.u-tokyo.ac.jp>, current@FreeBSD.ORG, John Baldwin <jhb@FreeBSD.ORG> Subject: Re: Patch for critical_enter()/critical_exit() & interrupt assembly revamp, please review! Message-ID: <20020224224927.D35990@locore.ca> In-Reply-To: <200202241912.g1OJCMx95238@apollo.backplane.com>; from dillon@apollo.backplane.com on Sun, Feb 24, 2002 at 11:12:22AM -0800 References: <20020224131027.I31343-100000@gamplex.bde.org> <200202241912.g1OJCMx95238@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Apparently, On Sun, Feb 24, 2002 at 11:12:22AM -0800,
Matthew Dillon said words to the effect of;
> NOTES:
>
> I would like to thank Bruce for supplying the sample code that allowed
> me to do this in a day instead of several days.
>
> * debug.critical_mode sysctl. This will not be in the final commit,
> nor will any of the code that tests the variable. The final commit
> will use code as if the critical_mode were '1'.
>
> The default is 1, which means to use the new streamlined
> interrupt and cpu_critical_enter/exit() code. Setting it to 0
> will revert to the old hard-interrupt-disablement operation. You
> can change the mode at any time.
>
> * Additional cpu_critical_enter/exit() calls around icu_lock. Since
> critical_enter() no longer disables interrupts, special care must
> be taken when dealing with the icu_lock spin mutex because it is
> the one thing the interrupt code needs to be able to defer the
> interrupt.
>
> * MACHINE_CRITICAL_ENTER define. This exists to maintain compatibility
> with other architectures. i386 defines this to cause fork_exit to
> use the new API and to allow the i386 MD code to supply the
> critical_enter() and critical_exit() procedures rather then
> kern_switch.c
>
> I would much prefer it if the other architectures were brought around
> to use this new mechanism. The old mechanism makes assumptions
> in regards to hard disablement that is no longer correct for i386.
For sparc64 we basically already have this in hardware. There is both
an interrupt enable (IE) bit and a soft interrupt priority. Hardware
interrupts are masked by the IE bit. In all cases they just queue the
interrupt packet that is sent by the hardware in a per-cpu interrupt
queue and post a soft interrupt. The critical_* stuff only masks soft
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.
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.)
The fork case is a little different, hmm. I'd have to think about it more.
An md function/macro would probably suffice.
I notice the you are using cpu_critical_exit for the purpose of disabling
interrupts only, like cli, sti. I think that you should use api that tmm
wrote for sparc64 for this so that it can do more. i386 does not really
have cli/sti equivalents otherwise.
s = intr_disable() disables interrupts and returns the old state.
intr_restore(s) restores the state s.
I don't really like this change in general because it complicates things
more than I'd like, but I also don't have a bearing on how expensive cli/sti
are. It would seem to me that it just takes a few more clock cycles, which
isn't that important. I understand that it increases latency for the fast
part of the interrupt handler, but they are not able to run in critical
sections anyway due to the software masking.
Jake
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?20020224224927.D35990>