[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 02/15] Refactor cpu_has_work/any_cpu_has_work in
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH 02/15] Refactor cpu_has_work/any_cpu_has_work in cpus.c |
Date: |
Wed, 09 Feb 2011 09:07:50 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2011-02-08 19:50, Marcelo Tosatti wrote:
> On Mon, Feb 07, 2011 at 12:19:13PM +0100, Jan Kiszka wrote:
>> Avoid duplicate use of the function name cpu_has_work, it's confusing.
>> Refactor cpu_has_work to cpu_is_idle and do the same with
>> any_cpu_has_work.
>>
>> Signed-off-by: Jan Kiszka <address@hidden>
>> ---
>> cpus.c | 43 +++++++++++++++++++++++--------------------
>> 1 files changed, 23 insertions(+), 20 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index d54ec7d..cd764f2 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -137,29 +137,30 @@ static int cpu_can_run(CPUState *env)
>> return 1;
>> }
>>
>> -static int cpu_has_work(CPUState *env)
>> +static bool cpu_is_idle(CPUState *env)
>> {
>> - if (env->stop)
>> - return 1;
>> - if (env->queued_work_first)
>> - return 1;
>> - if (env->stopped || !vm_running)
>> - return 0;
>> - if (!env->halted)
>> - return 1;
>> - if (qemu_cpu_has_work(env))
>> - return 1;
>> - return 0;
>> + if (env->stop || env->queued_work_first) {
>> + return false;
>> + }
>> + if (env->stopped || !vm_running) {
>> + return true;
>> + }
>> + if (!env->halted || qemu_cpu_has_work(env)) {
>> + return false;
>> + }
>> + return true;
>> }
>
> Do you really find it easier to read evaluations grouped with || ? I
> don't.
I do, specifically as the old version was even more confusing in that
important detail "return 0" vs. "return 1". But even the new benefits
from the grouping IMHO.
>
> Sorry but the name change does not feel right either: CPU is still idle
> if the vm is not running.
But that's exactly what the function returns. Or is it confusing if we
are talking about the vcpu or the whole thread here? What about
"cpu_thread_is_idle" then?
Jan
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state, (continued)
- [Qemu-devel] Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state, Glauber Costa, 2011/02/07
- [Qemu-devel] Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state, Jan Kiszka, 2011/02/07
- [Qemu-devel] Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state, Glauber Costa, 2011/02/07
- [Qemu-devel] Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state, Jan Kiszka, 2011/02/07
- [Qemu-devel] Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state, Glauber Costa, 2011/02/07
- [Qemu-devel] Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state, Avi Kivity, 2011/02/07
Re: [Qemu-devel] [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state, Blue Swirl, 2011/02/07
[Qemu-devel] [PATCH 02/15] Refactor cpu_has_work/any_cpu_has_work in cpus.c, Jan Kiszka, 2011/02/07
[Qemu-devel] [PATCH v2 02/15] Refactor cpu_has_work/any_cpu_has_work in cpus.c, Jan Kiszka, 2011/02/09