1. Patchew
  2. QEMU
  3. View series

[PATCH] COLO: Move some trace code behind qemu_mutex_unlock_iothread()

Rao, Lei posted 1 patch 4 years, 1 month ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20211210221435.142472-1-lei.rao@intel.com
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Juan Quintela <quintela@redhat.com>, Hailiang Zhang <zhang.zhanghailiang@huawei.com>
There is a newer version of this series
migration/colo.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] COLO: Move some trace code behind qemu_mutex_unlock_iothread()
Posted by Rao, Lei 4 years, 1 month ago 
Signed-off-by: Lei Rao <lei.rao@intel.com>
---
 migration/colo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index 2415325262..3ccacb29c8 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -683,8 +683,8 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
 
     qemu_mutex_lock_iothread();
     vm_stop_force_state(RUN_STATE_COLO);
-    trace_colo_vm_state_change("run", "stop");
     qemu_mutex_unlock_iothread();
+    trace_colo_vm_state_change("run", "stop");
 
     /* FIXME: This is unnecessary for periodic checkpoint mode */
     colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_REPLY,
@@ -786,8 +786,8 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
 
     vmstate_loading = false;
     vm_start();
-    trace_colo_vm_state_change("stop", "run");
     qemu_mutex_unlock_iothread();
+    trace_colo_vm_state_change("stop", "run");
 
     if (failover_get_state() == FAILOVER_STATUS_RELAUNCH) {
         return;
@@ -870,8 +870,8 @@ void *colo_process_incoming_thread(void *opaque)
         abort();
 #endif
     vm_start();
-    trace_colo_vm_state_change("stop", "run");
     qemu_mutex_unlock_iothread();
+    trace_colo_vm_state_change("stop", "run");
 
     colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_READY,
                       &local_err);
-- 
2.32.0
Re: [PATCH] COLO: Move some trace code behind qemu_mutex_unlock_iothread()
Posted by Dr. David Alan Gilbert 4 years, 1 month ago 
* Rao, Lei (lei.rao@intel.com) wrote:
> Signed-off-by: Lei Rao <lei.rao@intel.com>

You don't say why you want to move it - it's just a trace, what's the
advantage?

Dave

> ---
>  migration/colo.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index 2415325262..3ccacb29c8 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -683,8 +683,8 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>  
>      qemu_mutex_lock_iothread();
>      vm_stop_force_state(RUN_STATE_COLO);
> -    trace_colo_vm_state_change("run", "stop");
>      qemu_mutex_unlock_iothread();
> +    trace_colo_vm_state_change("run", "stop");
>  
>      /* FIXME: This is unnecessary for periodic checkpoint mode */
>      colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_REPLY,
> @@ -786,8 +786,8 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>  
>      vmstate_loading = false;
>      vm_start();
> -    trace_colo_vm_state_change("stop", "run");
>      qemu_mutex_unlock_iothread();
> +    trace_colo_vm_state_change("stop", "run");
>  
>      if (failover_get_state() == FAILOVER_STATUS_RELAUNCH) {
>          return;
> @@ -870,8 +870,8 @@ void *colo_process_incoming_thread(void *opaque)
>          abort();
>  #endif
>      vm_start();
> -    trace_colo_vm_state_change("stop", "run");
>      qemu_mutex_unlock_iothread();
> +    trace_colo_vm_state_change("stop", "run");
>  
>      colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_READY,
>                        &local_err);
> -- 
> 2.32.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [PATCH] COLO: Move some trace code behind qemu_mutex_unlock_iothread()
Posted by Rao, Lei 4 years, 1 month ago 

On 12/13/2021 7:45 PM, Dr. David Alan Gilbert wrote:
> * Rao, Lei (lei.rao@intel.com) wrote:
>> Signed-off-by: Lei Rao <lei.rao@intel.com>
> 
> You don't say why you want to move it - it's just a trace, what's the
> advantage?

I think it's not necessary to put trace code in the critical section.
So, moving it behind qemu_mutex_unlock_iothread() may reduce the lock time.
I will send the V2 and add some commit messages to say why.

Thanks,
Lei

> 
> Dave
> 
>> ---
>>   migration/colo.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/colo.c b/migration/colo.c
>> index 2415325262..3ccacb29c8 100644
>> --- a/migration/colo.c
>> +++ b/migration/colo.c
>> @@ -683,8 +683,8 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>>   
>>       qemu_mutex_lock_iothread();
>>       vm_stop_force_state(RUN_STATE_COLO);
>> -    trace_colo_vm_state_change("run", "stop");
>>       qemu_mutex_unlock_iothread();
>> +    trace_colo_vm_state_change("run", "stop");
>>   
>>       /* FIXME: This is unnecessary for periodic checkpoint mode */
>>       colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_REPLY,
>> @@ -786,8 +786,8 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>>   
>>       vmstate_loading = false;
>>       vm_start();
>> -    trace_colo_vm_state_change("stop", "run");
>>       qemu_mutex_unlock_iothread();
>> +    trace_colo_vm_state_change("stop", "run");
>>   
>>       if (failover_get_state() == FAILOVER_STATUS_RELAUNCH) {
>>           return;
>> @@ -870,8 +870,8 @@ void *colo_process_incoming_thread(void *opaque)
>>           abort();
>>   #endif
>>       vm_start();
>> -    trace_colo_vm_state_change("stop", "run");
>>       qemu_mutex_unlock_iothread();
>> +    trace_colo_vm_state_change("stop", "run");
>>   
>>       colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_READY,
>>                         &local_err);
>> -- 
>> 2.32.0
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.