Izumi Tsutsui wrote:
> garrett_damore@tadpole.com wrote:
>
>   
>> How about the following rewrite?  I've not tested it yet, but I'm a
>> little concerned about the potential implications of calling hardclock
>> repeatedly to "catch up" lost interrupts.  Especially as it effects ntp
>> and timecounters.  I'd like to hear opinions on the matter.
>>     
>
> I have no idea. I'd like to hear opinions of timecounter guys :-)
>   

I talked with Frank about this at some length.  It shouldn't matter one
way or the other.

>   
>> I also added an evcnt for missed clock interrupts.
>>
>> struct evcnt mips_int5_missed_evcnt =
>>     EVCNT_INITIALIZER(EVCNT_TYPE_INTR, NULL, "mips", "missed int 5");
>>     
>
> Yeah, I wonder how many interrupts are actually lost.
>   

Actually, as I think about this, the only reason we should miss
interrupts is someone is doing splhigh()/splclock() for longer than a
clock tick.  That shouldn't happen, outside of someone doing stuff in ddb.

So I'm actually thinking that using the stat counter so we find out if
this is the case or not is the right thing to do -- don't bother with
any of the rest of the handling other than what we already have.

>   
>>     if (__predict_false(lost > 0)) {
>>         next_cp0_clk_intr = new_cnt + curcpu()->ci_cycles_per_hz;
>>     
>
> This should be "new_cnt + delta" to keep intervals precisely?
>   

I don't think so.  I think it is best to get back to a regular interrupt
period.   Basically, this ensures that the next interrupt is schedule
for tick cycles ahead.  The delta would make the period longer, which
result in an extended time to the next interrupt.  (I.e. the next clock
could be several ticks long.)

In any case, I don't think missed interrupts (at least on INT5) happen
in practice outside of ddb, so I don't think we should be worrying about it.

This conclusion is based on the assumption that no port is doing
anything stupid like sharing INT5 with some other device....  and that
driver and kernel code is well behaved and doesn't do
splclock()/splhigh() for long periods of time.

>   
>>         mips3_cp0_compare_write(next_cp0_clk_intr);
>>         for (; lost > 0; lost--) {
>>             hardclock(cfp);
>>             mips_int5_evcnt.ev_count++;
>>     
>
> hardclock(&cp) and mips_int5_missed_evcnt.ev_count?
>   

Yes.  Doh.  Nice catch.
>   
>>         }
>>     }
>>     
>
> It's probably better to control hardclock(9) except last call
> not to call spllowersoftclock(9) by tweaking clockframe:
> ---
>       if (__predict_false(lost > 0)) {
>           int sr;
>           next_cp0_clk_intr = new_cnt + delta;
>           mips3_cp0_compare_write(next_cp0_clk_intr);
>           sr = cf.sr;
>           cf.sr &= ~MIPS_SR_INT_IE;
>           for (; lost > 0; lost--) {
>               hardclock(&cf);
>               mips_int5_missed_evcnt.ev_count++;
>           }
>           cf.sr = sr;
>       }
> ---
> But I'd appreciate any comments from interrupt gurus.
>   

Actually, the complexity here more than helps convince me that we
shouldn't be doing any of this.  It probably never occurs, and we're
more likely to screw it up by trying to handle it.

Many ports have not worried about this before, and seen no ill effects, btw.

    -- Garrett
> ---
> Izumi Tsutsui
>   


-- 
Garrett D'Amore, Principal Software Engineer
Tadpole Computer / Computing Technologies Division,
General Dynamics C4 Systems
http://www.tadpolecomputer.com/
Phone: 951 325-2134  Fax: 951 325-2191