Re: [Qemu-devel] [PATCH v3 1/5] cpu: Provide vcpu throttling interface

["Jason J. Herne" <jjherne@linux.vnet.ibm.com>]

On 07/01/2015 10:03 AM, Paolo Bonzini wrote:
>
>
> On 26/06/2015 20:07, Dr. David Alan Gilbert wrote:
>> * Jason J. Herne (jjherne@linux.vnet.ibm.com) wrote:
>>> Provide a method to throttle guest cpu execution. CPUState is augmented with
>>> timeout controls and throttle start/stop functions. To throttle the guest cpu
>>> the caller simply has to call the throttle set function and provide a percentage
>>> of throttle time.
>>
>> I'm worried about atomicity and threads and all those fun things.
>>
>> I think all the starting/stopping/setting the throttling level is done in the
>> migration thread; I think the timers run in the main/io thread?
>> So you really need to be careful with at least:
>>      throttle_timer_stop - which may have a minor effect
>>      throttle_timer  - I worry about the way cpu_timer_active checks the pointer
>>                        yet it's freed when the timer goes off.   It's probably
>>                        not too bad because it never dereferences it.
>
> Agreed.  I think the only atomic should be throttle_percentage; if zero,
> throttling is inactive.
>
> In particular, throttle_ratio can be computed in cpu_throttle_thread.
>
> If you have exactly one variable that is shared between the threads,
> everything is much simpler.
>
> There is no need to allocate and free the timer; it's very cheap and in
> fact we probably should convert to statically allocated timers sooner or
> later.  So you can just create it once, for example in cpu_ticks_init.
>

I've made all of the changes you have suggested except adding atomics. 
I'm having
a bit of trouble figuring out what is needed here. Perhaps I should be using
atomic_read() to read throttle_percentage? If so, I don't undertand why. 
Rather
than trying to explain everything here I'm going to submit my V4 patches.
If any atomic operations are still necessary with V4 please let me know.

-- 
-- Jason J. Herne (jjherne@linux.vnet.ibm.com)