On 2013-07-25 15:31, Stefan Hajnoczi wrote:
> On Thu, Jul 25, 2013 at 3:06 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>> On 2013-07-25 15:02, Paolo Bonzini wrote:
>>> Il 25/07/2013 14:48, Jan Kiszka ha scritto:
>>>> The concept of clocks (with start/stop property) and active timers shall
>>>> not be mixed, they are independent.
>>>
>>> Are you referring to this in particular:
>>>
>>> void pause_all_vcpus(void)
>>> {
>>> CPUState *cpu = first_cpu;
>>>
>>> qemu_clock_enable(vm_clock, false);
>>> ...
>>> }
>>>
>>> void resume_all_vcpus(void)
>>> {
>>> CPUState *cpu = first_cpu;
>>>
>>> qemu_clock_enable(vm_clock, true);
>>> ...
>>> }
>>>
>>> where _all_ the vm_clock timer lists need to be stopped at the same time?
>>
>> Still wrong abstraction: the vm_clock is stopped. And that implies that
>> no timer using this clock will fire anymore.
>
> Perhaps we should split QEMUClock into ClockSource and TimerList.
> Instead of using the int type field in TimerList we keep a ClockSource
> pointer.
>
> A ClockSource can be enabled/disabled.
>
> Then there's the question of ClockSource thread-safety. Host clock is
> weird because it notifies listeners when the clock source jumps.
Detection can be handled with a per-clock lock, dispatching can continue
to run under BQL for now.
> vm clock is hard because of icount.
True.
>
> It's also weird to think about multiple threads dispatching timer
> callbacks while a ClockSource is disabled. Basically, do we need a
> guarantee that timer callbacks have completed and will not be invoked
> in any thread after the disable function returns?
We need to make sure that no timer callback dispatcher works with a
clock value that is past the one at which we stopped. The simple answer
to this is a lock...
>
> But assuming thread-safety is addressed, this would fit the design
> you're suggesting.
Yes, this looks good.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
