Re: DragonFly-2.3.0.864.gc5b83 master sys/platform/pc32/apic mpapic.c sys/platform/pc32/isa clock.c sys/platform/pc64/isa clock.c sys/platform/vkernel/platform systimer.c sys/sys systimer.h

[Sepherosa Ziehau <sepherosa@xxxxxxxxx>]

On Sun, May 3, 2009 at 12:13 AM, Matthew Dillon
<dillon@apollo.backplane.com> wrote:
> :    lapic timer: Finish the lapic timer support
> :
> :...
>
>    Yah!  It works!  Awesome work!
>
> lapic: divisor index 0, frequency 100489256 Hz
> vmstat -i ... shows no more "clk" interrupt (as expected!)

Ha, thank you for testing!

>
>    Three notes.
>
>    (a) In your commit you said 'hw.lapci_timer_enable' instead of what
>        you really meant which was 'hw.lapic_timer_enable' :-)

Yeah, looks like APCI is my invention!

>
>    (b) Here is a slight change to the interrupt vector code.  The
>        FAST interrupt code also checks for a non-zero td->td_nest_count
>        before allowing the processing to occur, so I added that to the lapic
>        vector code too.
>
>        The use of td_nest_count is a bit of a hack to allow interrupt
>        thread preemption to work for slow interrupts.  I bump td_nest_count
>        instead of entering a critical section when scheduling the ithread
>        (for slow interrupts) so the critical section tests in lwkt_schedule()
>        in the preemption checks work as expected.
>
>        (see below)

Thank you for the explain.

>
>    (c) Lastly now that the APIC is in use we need some accounting for the
>        apic interrupt in 'systat -i' output.  I added a line to increment
>        the V_INTR statistic.  We might want to augment systat -vm 1 to
>        explicitly report clock interrupts somehow, maybe with a dummy
>        kernel-layer interrupt structure that we can name (e.g. "lapic"
>        vs the i8254's "clk").  I don't know the best solution.
>
>        The quick change in the patch below makes it show up in
>        sysctl vm.stats.sys.v_intr, but systat -vm 1 still doesn't
>        pick it up (maybe systat should just report vm.stats.sys.v_intr
>        for its 'total' instead of totalizing the interrupt stats itself).
>
>        (see same patch below)

I am thinking about it too.  However, I am more interested in per-cpu
rate instead of the total rate (I am not saying total rate is not
useful :).  So how about add another systat item?  I would like it to
show per-cpu lapic timer rate and possibly ipi rate.

>
>    I think you can enable the lapic timer by default in a few days, or even
>    now.  I'm running it on my test boxes and will report any issues but it
>    appears to be solid as a rock.

:D

The following patch is comitted!

>
> diff --git a/sys/platform/pc32/apic/apic_vector.s b/sys/platform/pc32/apic/apic_vector.s
> index a208f78..26b1d77 100644
> --- a/sys/platform/pc32/apic/apic_vector.s
> +++ b/sys/platform/pc32/apic/apic_vector.s
> @@ -411,9 +411,12 @@ Xtimer:
>        movl    $0, lapic_eoi           /* End Of Interrupt to APIC */
>        FAKE_MCOUNT(15*4(%esp))
>
> +       incl    PCPU(cnt) + V_INTR
>        movl    PCPU(curthread),%ebx
>        cmpl    $TDPRI_CRIT,TD_PRI(%ebx)
>        jge     1f
> +       testl   $-1,TD_NEST_COUNT(%ebx)
> +       jne     1f
>        subl    $8,%esp                 /* make same as interrupt frame */
>        pushl   %esp                    /* pass frame by reference */
>        incl    PCPU(intr_nesting_level)
>



-- 
Live Free or Die