Re: [Qemu-devel] [RFC 0/8] arm AioContext with its own timer stuff

[Paolo Bonzini]

Il 25/07/2013 14:38, Jan Kiszka ha scritto:
> On 2013-07-25 14:35, Paolo Bonzini wrote:
>> Il 25/07/2013 14:32, Jan Kiszka ha scritto:
>>> On 2013-07-25 14:21, Alex Bligh wrote:
>>>>
>>>>
>>>> --On 25 July 2013 14:05:30 +0200 Stefan Hajnoczi <address@hidden>
>>>> wrote:
>>>>
>>>>> Alex Bligh's series gives each AioContext its own rt_clock.  This avoids
>>>>> the need for synchronization in the simple case.  If we require timer
>>>>> access between threads then we really need to synchronize.
>>>>>
>>>>> You pointed out in another email that vm_clock stops when the guest is
>>>>> paused.  I think we can find a solution for I/O throttling and QED,
>>>>> which use vm_clock in the block layer.  Note that block jobs already use
>>>>> rt_clock.
>>>>
>>>> I would happily at a QEMUClock of each type to AioContext. They are after
>>>> all pretty lightweight.
>>>
>>> What's the point of adding tones of QEMUClock instances? Considering
>>> proper abstraction, how are they different for each AioContext? Will
>>> they run against different clock sources, start/stop at different times?
>>> If the answer is "they have different timer list", then fix this
>>> incorrect abstraction.
>>
>> s/QEMUClock/QEMUTimerList/ ? :)
> 
> What do you mean? If the content of struct QEMUClock remained the same,
> that would just paper over a design mistake.

I mean that really the "clock" part of QEMUClock is really just

#define QEMU_CLOCK_REALTIME 0
#define QEMU_CLOCK_VIRTUAL  1
#define QEMU_CLOCK_HOST     2

and

    switch(clock_type) {
    case QEMU_CLOCK_REALTIME:
        return get_clock();
    default:
    case QEMU_CLOCK_VIRTUAL:
        if (use_icount) {
            return cpu_get_icount();
        } else {
            return cpu_get_clock();
        }
    case QEMU_CLOCK_HOST:
        now = get_clock_realtime();
        last = clock->last;
        clock->last = now;
        if (now < last) {
            notifier_list_notify(&clock->reset_notifiers, &now);
        }
        return now;
    }

Everything else in QEMUClock is just a list of timers.  You also cannot
put timers from different clocks in the same list.

Different AioContexts needs different lists of timers even for the same
source (for obvious reasons of thread-safety).  Different sources need
different lists of timers because they do not advance together.  The
most obvious example is vm_clock which can stop for an arbitrary amount
of time while other rt_clock timers can be executed.  It is a
fundamental invariant of QEMUClock that if the first timer in the list
hasn't expired, the following ones also haven't.

Paolo