[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface |
Date: |
Thu, 23 Jul 2015 11:59:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 |
On 16/07/2015 16:21, Jason J. Herne wrote:
> 1. Using atomic operations to manage throttle_percentage. I'm not sure
> where atomics are applicable here. If this is still a concern hopefully
> someone can explain.
I would use atomic_read/atomic_set in cpu_throttle_set,
cpu_throttle_stop, cpu_throttle_active, cpu_throttle_get_percentage.
In addition, the function naming seems to be a bit inconsistent: please
rename cpu_throttle_set to cpu_throttle_set_percentage.
Second, here:
>> +static void cpu_throttle_thread(void *opaque)
>> +{
>> + double pct = (double)throttle_percentage/100;
Please use cpu_throttle_get_percentage(), and
>> + double throttle_ratio = pct / (1 - pct);
>> + long sleeptime_ms = (long)(throttle_ratio * CPU_THROTTLE_TIMESLICE);
... move these computations below the if.
I'm also not sure about throttle_ratio, why is it needed? If pct >= 0.5 you
end up with throttle_ratio >= 1, i.e. no way for the CPU to do any work. This
would definitely cause a problem with callbacks piling up.
>> + if (!throttle_percentage) {
>> + return;
>> + }
>> +
>> + qemu_mutex_unlock_iothread();
>> + g_usleep(sleeptime_ms * 1000); /* Convert ms to us for usleep call */
>> + qemu_mutex_lock_iothread();
>> +}
>> +
> 2. Callback stacking. And it seems like we are convinced that it is not
> a big issue. Anyone disagree?
I think it's not a big issue to have many timers, but it is a big issue to have
many callbacks. What I suggested is this:
if (!atomic_xchg(&cpu->throttle_thread_scheduled, 1)) {
async_run_on_cpu(cpu, cpu_throttle_thread, NULL);
}
and in the callback:
atomic_set(&cpu->throttle_thread_scheduled, 0);
g_usleep(...);
Paolo
- [Qemu-devel] [PATCH v4 0/5] migration: Dynamic cpu throttling for auto-converge, Jason J. Herne, 2015/07/02
- [Qemu-devel] [PATCH v4 5/5] migration: Disambiguate MAX_THROTTLE, Jason J. Herne, 2015/07/02
- [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Jason J. Herne, 2015/07/02
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Paolo Bonzini, 2015/07/02
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Dr. David Alan Gilbert, 2015/07/02
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Jason J. Herne, 2015/07/13
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Paolo Bonzini, 2015/07/13
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Jason J. Herne, 2015/07/15
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Paolo Bonzini, 2015/07/15
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Jason J. Herne, 2015/07/16
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Jason J. Herne, 2015/07/31
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Paolo Bonzini, 2015/07/31
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Jason J. Herne, 2015/07/31
- Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Jason J. Herne, 2015/07/31
Re: [Qemu-devel] [PATCH v4 1/5] cpu: Provide vcpu throttling interface, Andreas Färber, 2015/07/02
[Qemu-devel] [PATCH v4 3/5] migration: Dynamic cpu throttling for auto-converge, Jason J. Herne, 2015/07/02
[Qemu-devel] [PATCH v4 4/5] qmp/hmp: Add throttle ratio to query-migrate and info migrate, Jason J. Herne, 2015/07/02
[Qemu-devel] [PATCH v4 2/5] migration: Parameters for auto-converge cpu throttling, Jason J. Herne, 2015/07/02