Patchwork cpus: use cpu_is_stopped efficiently

login
register
mail settings
Submitter Tiejun Chen
Date July 26, 2013, 8:47 a.m.
Message ID <1374828431-25517-1-git-send-email-tiejun.chen@windriver.com>
Download mbox | patch
Permalink /patch/262088/
State New
Headers show

Comments

Tiejun Chen - July 26, 2013, 8:47 a.m.
It makes more sense and simple later.

Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
 cpus.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Tiejun Chen - Aug. 1, 2013, 8:12 a.m.
On 07/26/2013 04:47 PM, Tiejun Chen wrote:
> It makes more sense and simple later.

Any feedback :)

Tiejun

>
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
>   cpus.c |   14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index c232265..a997632 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -62,6 +62,11 @@
>
>   static CPUArchState *next_cpu;
>
> +bool cpu_is_stopped(CPUState *cpu)
> +{
> +    return !runstate_is_running() || cpu->stopped;
> +}
> +
>   static bool cpu_thread_is_idle(CPUArchState *env)
>   {
>       CPUState *cpu = ENV_GET_CPU(env);
> @@ -69,7 +74,7 @@ static bool cpu_thread_is_idle(CPUArchState *env)
>       if (cpu->stop || cpu->queued_work_first) {
>           return false;
>       }
> -    if (cpu->stopped || !runstate_is_running()) {
> +    if (cpu_is_stopped(cpu)) {
>           return true;
>       }
>       if (!cpu->halted || qemu_cpu_has_work(cpu) ||
> @@ -432,11 +437,6 @@ void cpu_synchronize_all_post_init(void)
>       }
>   }
>
> -bool cpu_is_stopped(CPUState *cpu)
> -{
> -    return !runstate_is_running() || cpu->stopped;
> -}
> -
>   static void do_vm_stop(RunState state)
>   {
>       if (runstate_is_running()) {
> @@ -455,7 +455,7 @@ static bool cpu_can_run(CPUState *cpu)
>       if (cpu->stop) {
>           return false;
>       }
> -    if (cpu->stopped || !runstate_is_running()) {
> +    if (cpu_is_stopped(cpu)) {
>           return false;
>       }
>       return true;
>
Andreas Färber - Aug. 1, 2013, 11:38 a.m.
Hi,

Am 26.07.2013 10:47, schrieb Tiejun Chen:
> It makes more sense and simple later.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
>  cpus.c |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index c232265..a997632 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -62,6 +62,11 @@
>  
>  static CPUArchState *next_cpu;
>  
> +bool cpu_is_stopped(CPUState *cpu)
> +{
> +    return !runstate_is_running() || cpu->stopped;
> +}
> +
>  static bool cpu_thread_is_idle(CPUArchState *env)
>  {
>      CPUState *cpu = ENV_GET_CPU(env);

To optimize performance slightly, I would suggest to reorder the two
conditions as they were below (avoiding the non-inline function call if
cpu->stopped).

Other than that it looks good to me, but no bugfix for 1.6.
If you send a v2 I can queue it on qom-cpu for the next merge window in
two weeks.

CC'ing me would have made me review it earlier. ;) And as you may have
noticed, Avi is no longer with Red Hat, and Gleb and Paolo are
maintaining KVM parts, which there are none in this patch. See
MAINTAINERS file for the latest list.

Regards,
Andreas

> @@ -69,7 +74,7 @@ static bool cpu_thread_is_idle(CPUArchState *env)
>      if (cpu->stop || cpu->queued_work_first) {
>          return false;
>      }
> -    if (cpu->stopped || !runstate_is_running()) {
> +    if (cpu_is_stopped(cpu)) {
>          return true;
>      }
>      if (!cpu->halted || qemu_cpu_has_work(cpu) ||
> @@ -432,11 +437,6 @@ void cpu_synchronize_all_post_init(void)
>      }
>  }
>  
> -bool cpu_is_stopped(CPUState *cpu)
> -{
> -    return !runstate_is_running() || cpu->stopped;
> -}
> -
>  static void do_vm_stop(RunState state)
>  {
>      if (runstate_is_running()) {
> @@ -455,7 +455,7 @@ static bool cpu_can_run(CPUState *cpu)
>      if (cpu->stop) {
>          return false;
>      }
> -    if (cpu->stopped || !runstate_is_running()) {
> +    if (cpu_is_stopped(cpu)) {
>          return false;
>      }
>      return true;
>
Marcelo Tosatti - Aug. 1, 2013, 4:26 p.m.
On Thu, Aug 01, 2013 at 04:12:03PM +0800, "“tiejun.chen”" wrote:
> On 07/26/2013 04:47 PM, Tiejun Chen wrote:
> >It makes more sense and simple later.
> 
> Any feedback :)
> 
> Tiejun

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
Tiejun Chen - Aug. 2, 2013, 1:45 a.m.
On 08/01/2013 07:38 PM, � wrote:
> Hi,
>
> Am 26.07.2013 10:47, schrieb Tiejun Chen:
>> It makes more sense and simple later.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>> ---
>>   cpus.c |   14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index c232265..a997632 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -62,6 +62,11 @@
>>
>>   static CPUArchState *next_cpu;
>>
>> +bool cpu_is_stopped(CPUState *cpu)
>> +{
>> +    return !runstate_is_running() || cpu->stopped;
>> +}
>> +
>>   static bool cpu_thread_is_idle(CPUArchState *env)
>>   {
>>       CPUState *cpu = ENV_GET_CPU(env);
>
> To optimize performance slightly, I would suggest to reorder the two
> conditions as they were below (avoiding the non-inline function call if
> cpu->stopped).

Good idea.

>
> Other than that it looks good to me, but no bugfix for 1.6.
> If you send a v2 I can queue it on qom-cpu for the next merge window in
> two weeks.

I already send this v2 just now.

>
> CC'ing me would have made me review it earlier. ;) And as you may have
> noticed, Avi is no longer with Red Hat, and Gleb and Paolo are
> maintaining KVM parts, which there are none in this patch. See
> MAINTAINERS file for the latest list.

Thanks for your information :)

Tiejun

>
> Regards,
> Andreas
>
>> @@ -69,7 +74,7 @@ static bool cpu_thread_is_idle(CPUArchState *env)
>>       if (cpu->stop || cpu->queued_work_first) {
>>           return false;
>>       }
>> -    if (cpu->stopped || !runstate_is_running()) {
>> +    if (cpu_is_stopped(cpu)) {
>>           return true;
>>       }
>>       if (!cpu->halted || qemu_cpu_has_work(cpu) ||
>> @@ -432,11 +437,6 @@ void cpu_synchronize_all_post_init(void)
>>       }
>>   }
>>
>> -bool cpu_is_stopped(CPUState *cpu)
>> -{
>> -    return !runstate_is_running() || cpu->stopped;
>> -}
>> -
>>   static void do_vm_stop(RunState state)
>>   {
>>       if (runstate_is_running()) {
>> @@ -455,7 +455,7 @@ static bool cpu_can_run(CPUState *cpu)
>>       if (cpu->stop) {
>>           return false;
>>       }
>> -    if (cpu->stopped || !runstate_is_running()) {
>> +    if (cpu_is_stopped(cpu)) {
>>           return false;
>>       }
>>       return true;
>>
>
>

Patch

diff --git a/cpus.c b/cpus.c
index c232265..a997632 100644
--- a/cpus.c
+++ b/cpus.c
@@ -62,6 +62,11 @@ 
 
 static CPUArchState *next_cpu;
 
+bool cpu_is_stopped(CPUState *cpu)
+{
+    return !runstate_is_running() || cpu->stopped;
+}
+
 static bool cpu_thread_is_idle(CPUArchState *env)
 {
     CPUState *cpu = ENV_GET_CPU(env);
@@ -69,7 +74,7 @@  static bool cpu_thread_is_idle(CPUArchState *env)
     if (cpu->stop || cpu->queued_work_first) {
         return false;
     }
-    if (cpu->stopped || !runstate_is_running()) {
+    if (cpu_is_stopped(cpu)) {
         return true;
     }
     if (!cpu->halted || qemu_cpu_has_work(cpu) ||
@@ -432,11 +437,6 @@  void cpu_synchronize_all_post_init(void)
     }
 }
 
-bool cpu_is_stopped(CPUState *cpu)
-{
-    return !runstate_is_running() || cpu->stopped;
-}
-
 static void do_vm_stop(RunState state)
 {
     if (runstate_is_running()) {
@@ -455,7 +455,7 @@  static bool cpu_can_run(CPUState *cpu)
     if (cpu->stop) {
         return false;
     }
-    if (cpu->stopped || !runstate_is_running()) {
+    if (cpu_is_stopped(cpu)) {
         return false;
     }
     return true;