Re: + paravirt-vmi-timer-patches.patch added to -mm tree

Re: + paravirt-vmi-timer-patches.patch added to -mm tree

[Daniel Walker]

On Fri, 2006-12-15 at 14:27 -0800, akpm@osdl.org wrote:
> +
> +unsigned long long vmi_sched_clock(void)
> +{
> +       return read_available_cycles();
> +}
> + 


This sched_clock is likely broken if it's returning something other than
nanoseconds. It looks like cycles, but it's also getting piped through
an ops pointer so I'm not sure what's getting returned here.

Daniel

Re: + paravirt-vmi-timer-patches.patch added to -mm tree

[Daniel Walker]

On Fri, 2006-12-15 at 14:27 -0800, akpm@osdl.org wrote:
> +
> +config NO_IDLE_HZ
> +       bool
> +       depends on PARAVIRT
> +       default y
> +       help
> +         Switches the regular HZ timer off when the system is going
> idle.
> +         This helps a hypervisor detect that the Linux system is
> idle,
> +         reducing the overhead of idle systems. 


There is already a dynamic tick (NO_HZ) system in the -mm tree .. Given
that this implementation seems unnecessary. Why do you need another
different system to do this?

Daniel

Re: + paravirt-vmi-timer-patches.patch added to -mm tree

[Zachary Amsden]

Daniel Walker wrote:
> On Fri, 2006-12-15 at 14:27 -0800, akpm@osdl.org wrote:
>   
>> +
>> +config NO_IDLE_HZ
>> +       bool
>> +       depends on PARAVIRT
>> +       default y
>> +       help
>> +         Switches the regular HZ timer off when the system is going
>> idle.
>> +         This helps a hypervisor detect that the Linux system is
>> idle,
>> +         reducing the overhead of idle systems. 
>>     
>
>
> There is already a dynamic tick (NO_HZ) system in the -mm tree .. Given
> that this implementation seems unnecessary. Why do you need another
> different system to do this?
>   

We don't.  This was written before the dynamic tick code, and now they 
need to be merged.  Until then, they can safely coexist.

Zach

Re: + paravirt-vmi-timer-patches.patch added to -mm tree

[Daniel Walker]

On Sat, 2007-01-06 at 12:37 -0800, Zachary Amsden wrote:

> >
> > There is already a dynamic tick (NO_HZ) system in the -mm tree .. Given
> > that this implementation seems unnecessary. Why do you need another
> > different system to do this?
> >   
> 
> We don't.  This was written before the dynamic tick code, and now they 
> need to be merged.  Until then, they can safely coexist.

So really this can't go upstream till that merge happens. What's
preventing you from just directly using NO_HZ without changes?

Daniel  

Re: + paravirt-vmi-timer-patches.patch added to -mm tree

[Zachary Amsden]

Daniel Walker wrote:
> On Sat, 2007-01-06 at 12:37 -0800, Zachary Amsden wrote:
>
>   
>>> There is already a dynamic tick (NO_HZ) system in the -mm tree .. Given
>>> that this implementation seems unnecessary. Why do you need another
>>> different system to do this?
>>>   
>>>       
>> We don't.  This was written before the dynamic tick code, and now they 
>> need to be merged.  Until then, they can safely coexist.
>>     
>
> So really this can't go upstream till that merge happens. What's
> preventing you from just directly using NO_HZ without changes?
>   

For one thing, the fact that it doesn't account for stolen time.  But 
mostly because going through the regular PIT / APIC timer paths has a 
lot of overhead.  So we need a separate timer device, and weaving this 
in with the local APIC timer dependency for SMP on i386 requires changes 
on top of NO_HZ.

Zach

Re: + paravirt-vmi-timer-patches.patch added to -mm tree

[Zachary Amsden]

Daniel Walker wrote:
> On Fri, 2006-12-15 at 14:27 -0800, akpm@osdl.org wrote:
>   
>> +
>> +unsigned long long vmi_sched_clock(void)
>> +{
>> +       return read_available_cycles();
>> +}
>> + 
>>     
>
>
> This sched_clock is likely broken if it's returning something other than
> nanoseconds. It looks like cycles, but it's also getting piped through
> an ops pointer so I'm not sure what's getting returned here.
>   

Thanks, I'll fix that.

Zach