Re: [Qemu-devel] timer issue on 1.7.0 and later

[Alex Bligh]

Rob,

>> A while ago a wrote some instrumentation. I can't recall how far I got with
>> testing it, but here it is rebased onto master without so much as a compile 
>> test
>> (top 4 commits - ignore the 5th):
>>   
>> https://github.com/abligh/qemu/commit/e711816da9831c97f7d8a111b610ef42f7ba69fa
> 
> This doesn't appear to be too useful.

Ha - I didn't expect you to run this code (this was mainly to ask Paolo
what he wanted in) as I haven't even used it myself. I'm surprised it
didn't SEGV everywhere!

> The AvgLength is large because
> INT64_MAX / GTIMER_SCALE is used as the next timer value when no timer
> is needed.

That's pretty ugly. Perhaps timer_mod should take an expire time of 0 to
mean 'make the timer already expired'; currently this causes it to
immediately expire as far as I can see.

> I also changed the code to just call timer_del instead of
> setting a max timeout and the length looks normal (~10ms).

OK

> Timer list at 0x7fae6463d680 clock 1:
>           Address       Expiries      AvgLength       NumShort Source
>    0x7fae646659e0            673 13704855334071269              0 cpu.c:229
>    0x7fae646650f0            691    -6386455212              0 cpu.c:229
>    0x7fae6480e630              1  4294967294556              0 ptimer.c:229

This is without your change to use timer_del? I guess the negative one
is wrapping ints.

The interesting point here is 0 sub-millisecond expiries (if that
bit of code is working).

Do you think that line 229 is bogus? My copy of master says there's
a timer at line 225/226 and line 227/228 of the arm cpu.c and at
line 229 of ptimer.c, but it's a bit of a coincidence and I'd
have thought cpu.c would have 2 timers going.

> This seems to be more related to interaction between the 2 timers as
> the same timer works fine for single core use. The relevant code is
> target-arm/helper.c:gt_recalc_timer.

This code confuses me a little. GTIMER_SCALE is 16. That means the clock
operates in periods of 16ns, which is not something I'd envisaged, but
I suppose should work. That does indeed give a 62.5MHz timer or would
do if programme with '1'.  Given that, I am not /quite/ sure why the
clamping is so worrisome given as far as I can tell 2^63 nanoseconds
is about 292 years.

I wonder if what is happening is that with 2 cores (for some reason)
it's constantly modifying the timers (when they have yet to expire)
despite this not actually changing when anything expires.

Is use_icount set to true? If so, it might be worth looking at
the icount code.

If not, I'm wondering whether what is happening is that it is
calling timer_mod sufficiently frequently where each timer_mod
causes a qemu_notify_event. This breaks out of the mainloop
ppoll() because the expiry time for the ppoll has changed (marginally).

One optimisation here would be to not do a qemu_notify_event if
the deadline has extended.

Summary: I'm interested in quite how frequently timer_mod is being
called. If it's 'far more frequently than the timer actually
expires' I think that could cause an issue.

> I did find one bug that would seem to cause interrupts to be missed or
> fire twice, but it didn't make any difference:
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ca5b000..325515f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -905,7 +905,7 @@ static int gt_ctl_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
>     if ((oldval ^ value) & 1) {
>         /* Enable toggled */
>         gt_recalc_timer(cpu, timeridx);
> -    } else if ((oldval & value) & 2) {
> +    } else if ((oldval ^ value) & 2) {
>         /* IMASK toggled: don't need to recalculate,
>          * just set the interrupt line based on ISTATUS
>          */


I'm not familiar enough with the arm code to help here.

-- 
Alex Bligh