On Sun, 24 Feb 2002, Matthew Dillon wrote:
> 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.
Thanks.
> * 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'.
Won't it be needed for longer for non-i386's arches?
> * 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.
clock_lock needs these too, at least in my version where the soft
critical_enter() doesn't mask fast interrupts.
> * 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.
The old mechanism basically forces mtx_lock_spin() to do the hard disablement.
I never liked this.
> * Trampoline 'sti'. In the final commit, the trampoline will simply
> 'sti' after setting up td_critnest. The other junk to handle the
> hard-disablement case will be gone.
Maybe all of the fiddling with sched_lock belongs in MD code.
IIRC, in my version, the hard interrupt enablement in fork_exit() is
only needed at boot time. Something neglects to do it earlier in that
case, at least for some threads. I was surprised by this -- hard
interrupts are enabled early in the boot, and everything later should
disable and enable them reentrantly.
> * Additional cpu_critical_enter()/exit() calls in CY and TIMER code.
> Bruce had additional hard interrupt disablements in these modules.
>
> I'm not sure why so if I need to do that as well I would like to
> know.
There are also some in sio. The ones in sio an cy are needed because
my critical_enter() doesn't mask fast interrupts. Something is needed
to mask them, and cpu_critical_enter() is used. I think you don't need
these yet. The ones in timer code are to make the timer code as hard-
real-time as possible. I think this is important in
i8254_get_timecounter() but not elsewhere in clock.c.
> Index: i386/i386/machdep.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/i386/i386/machdep.c,v
> retrieving revision 1.497
> diff -u -r1.497 machdep.c
> --- i386/i386/machdep.c 17 Feb 2002 17:40:28 -0000 1.497
> +++ i386/i386/machdep.c 24 Feb 2002 19:04:20 -0000
> ...
> @@ -270,6 +275,121 @@
> }
>
> /*
> + * Critical section handling.
> + *
> + * Note that our interrupt code handles any interrupt race that occurs
> + * after we decrement td_critnest.
> + */
> +void
> +critical_enter(void)
i386/machdep.c is a different wrong place for this and unpend(). I put it
in isa/ithread.c. It is not very MD, and is even less isa-dependent.
> +void
> +critical_exit(void)
> +{
> + struct thread *td = curthread;
> + KASSERT(td->td_critnest > 0, ("bad td_critnest value!"));
> + if (--td->td_critnest == 0) {
> + if (td->td_savecrit != (critical_t)-1) {
> + cpu_critical_exit(td->td_savecrit);
> + td->td_savecrit = (critical_t)-1;
> + } else {
> + /*
> + * We may have to schedule pending interrupts. Create
> + * conditions similar to an interrupt context and call
> + * unpend().
> + */
> + if (PCPU_GET(int_pending) && td->td_intr_nesting_level == 0) {
I'm trying to kill td_intr_nesting_level. I think/hope you don't need it.
In ithread_schedule(), we begin with interrupts hard-enabled but don't
have any other locks, so things are delicate -- an mtx_unlock_spin()
immediately after the mtx_lock_spin() would do too much without the above
test. Perhaps add a critical_enter()/critical_exit() wrapper to sync the
software interrupt disablement with the hardware disablement. Xfastintr*
in -current already does this.
> Index: i386/isa/apic_vector.s
> ===================================================================
> RCS file: /home/ncvs/src/sys/i386/isa/apic_vector.s,v
> retrieving revision 1.75
> diff -u -r1.75 apic_vector.s
> --- i386/isa/apic_vector.s 5 Jan 2002 08:47:10 -0000 1.75
> +++ i386/isa/apic_vector.s 24 Feb 2002 17:58:34 -0000
Please do this without so many changes to the assembly code. Especially
not the reordering and style changes. I think I see how to do this
(see call_fast_unpend())
> ...
> @@ -417,6 +508,41 @@
> INTR(30,intr30,)
> INTR(31,intr31,)
> MCOUNT_LABEL(eintr)
> +
> +MCOUNT_LABEL(bfunpend)
> + FAST_UNPEND(0,fastunpend0)
>...
> + FAST_UNPEND(31,fastunpend31)
> +MCOUNT_LABEL(efunpend)
THe interrupt routines must go between the special labels bintr and eintr
so that mcount understands them.
> Index: i386/isa/intr_machdep.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/i386/isa/intr_machdep.c,v
> retrieving revision 1.65
> diff -u -r1.65 intr_machdep.c
> --- i386/isa/intr_machdep.c 8 Feb 2002 18:30:35 -0000 1.65
> +++ i386/isa/intr_machdep.c 24 Feb 2002 10:50:26 -0000
> ...
> @@ -672,3 +711,10 @@
>
> return (ithread_remove_handler(cookie));
> }
> +
> +void
> +call_fast_unpend(int irq)
> +{
> + fastunpend[irq]();
> +}
> +
I think this can be done using with few changes to the assembler by
using INTREN(irq) followed by a call to the usual Xfastintr[irq] entry
point, or (perhaps after a few adjustments) even to the driver's entry
point. Efficiency is not very important, since most entries are handled
directly and the overhead for INTREN() is dominant anyway. We just
need to be careful to provide a normal interrupt frame for clock
interrupt handlers (it needs to have %cs in it so that hardclock() can
tell if the interrupt was from user mode). OTOH, non-clock fast
interrupt handlers don't care about the frame at all, so just calling
them (with interrupts disabled) works right.
RELENG_4 still has complications related to this and I was pleased to
see them go away in -current. See vec0, vec8 and BUILD_VEC() in
icu_ipl.s (the vecN routines are stubs to avoid duplication of Xintr*).
-current made things simpler for normal interrupt handlers, and fast
interrupt handlers are simpler at this level except for the problem
with the frame.
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
