Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Feb 2002 13:24:16 +1100 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        Matthew Dillon <dillon@apollo.backplane.com>
Cc:        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:  <20020226121245.F42820-100000@gamplex.bde.org>
In-Reply-To: <200202252027.g1PKRKU52902@apollo.backplane.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 25 Feb 2002, Matthew Dillon wrote:

>     Unless an unforseen problem arises, I am going to commit this tomorrow
>     and then start working on a cleanup patch.   I have decided to

Please wait for jhb's opinion on it.  He seems to be offline again.
I think he has plans and maybe even code for more code in critical_enter().
I think we don't agree with these plans, but they are just as valid
as ours, and our versions undo many of his old changes.

>     I've looked at cleaning up cpu_critical_enter() & exit but its use
>     pollutes a number of MI source files: kern/kern_ktr.c, kern/subr_trap.c,
>     and kern/subr_witness.c are all misusing the functions.  If these
>     functions can safely be changed to use critical_enter() and critical_exit()
>     I will rename the i386's cpu_critical*() functions into intr_disable()
>     and intr_restore() and then use the new names for internal i386 use.

It is already one layer higher than the MD versions for i386's at least.
The MD versions are disable_intr() (should be cli() to inhibit use in MI
code), enable_intr() (should be sti() ...), read_eflags() and write_eflags().
cpu_critical*() is supposed to be the version with an MI interface and
semantics, but one level lower than critical*().  In my version,
critical*() essentially doesn't exist and cpu_critical*() is only used
by fast interrupt handlers and hacks.  The MI code in kern mainly depends
on the MD function (with an MI interface and semantics) unpend().

>     Is there any reason why the above uses of cpu_critical*() cannot be
>     changed to use standard critical*()?  I believe we *always* have a
>     curthread structure to play with so it should just work.

I think this would mostly work on i386's.  It would more obviously work
if td_critness were a per-cpu global and not in the thread struce.  It
essentially is a per-cpu global, but we optimize it by putting it in
the thread struct.  But ktr, especially, is very low level and should
work in the middle of a thread switch.

According to jake, it would only work on sparc64's if critical*() does
something hardware-related.  Lazy masking of interrupts is partly in
hardware, so the hardware must be talked to to turn it on an off.  On
i386's, the masking is in software, so it can use td_critnest as a
flag.

The use of cpu_critical*() in subr_trap.c is a bit trickier/kludgier.
We need to return from the interrupt handler atomically with clearing
the lazy-masking flag.  The hardware interrupt enable flag must be
used for this on i386's.  The code for this is logically:

In ast:
	/* Prevent changes to flags while we are deciding if they are set. */
	critical_enter();

	while ((ke->ke_flags & (KEF_ASTPENDING | KEF_NEEDRESCHED)) != 0) {
		critical_exit();
		...
		/* As above. */
		critical_enter();
	}

In doreti:
	/*
	 * Must use MD code to prevent race window after critical_exit().
	 * We switch from lazy sofware masking using td_critnest (or
	 * whatever critical_enter() uses) to hardware masking using cli.
	 */
	cli();
	critical_exit();
	...
	iretd();

but this is optimized in -current by using cpu_critical*() instead of
critical_enter*() in ast() and not doing the cli() and critical_exit()
in doreti.  This depends on cpu_critical_enter() being precisely cli().

Bruce


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?20020226121245.F42820-100000>