Re: [PATCH 3/5] Migration thread mutex

2011-08-29 Thread Stefan Hajnoczi
On Sat, Aug 27, 2011 at 7:09 PM, Umesh Deshpande udesh...@redhat.com wrote:
 This patch implements migrate_ram mutex, which protects the RAMBlock list
 traversal in the migration thread during the transfer of a ram from their
 addition/removal from the iothread.

 Note: Combination of iothread mutex and migration thread mutex works as a
 rw-lock. Both mutexes are acquired while modifying the ram_list members or RAM
 block list.

 Signed-off-by: Umesh Deshpande udesh...@redhat.com
 ---
  arch_init.c   |   21 +
  cpu-all.h     |    3 +++
  exec.c        |   23 +++
  qemu-common.h |    2 ++
  4 files changed, 49 insertions(+), 0 deletions(-)

 diff --git a/arch_init.c b/arch_init.c
 index 484b39d..9d02270 100644
 --- a/arch_init.c
 +++ b/arch_init.c
 @@ -109,6 +109,7 @@ static int is_dup_page(uint8_t *page, uint8_t ch)

  static RAMBlock *last_block;
  static ram_addr_t last_offset;
 +static uint64_t last_version;
[...]
  typedef struct RAMList {
 +    QemuMutex mutex;    /* Protects RAM block list */
     uint8_t *phys_dirty;
 +    uint32_t version;   /* To detect ram block addition/removal */

Is there a reason why RAMList.version is uint32_t but last_version is uint64_t?

Stefan
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/5] Migration thread mutex

2011-08-29 Thread Umesh Deshpande

On 08/29/2011 05:04 AM, Stefan Hajnoczi wrote:

On Sat, Aug 27, 2011 at 7:09 PM, Umesh Deshpandeudesh...@redhat.com  wrote:

This patch implements migrate_ram mutex, which protects the RAMBlock list
traversal in the migration thread during the transfer of a ram from their
addition/removal from the iothread.

Note: Combination of iothread mutex and migration thread mutex works as a
rw-lock. Both mutexes are acquired while modifying the ram_list members or RAM
block list.

Signed-off-by: Umesh Deshpandeudesh...@redhat.com
---
  arch_init.c   |   21 +
  cpu-all.h |3 +++
  exec.c|   23 +++
  qemu-common.h |2 ++
  4 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 484b39d..9d02270 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -109,6 +109,7 @@ static int is_dup_page(uint8_t *page, uint8_t ch)

  static RAMBlock *last_block;
  static ram_addr_t last_offset;
+static uint64_t last_version;

[...]

  typedef struct RAMList {
+QemuMutex mutex;/* Protects RAM block list */
 uint8_t *phys_dirty;
+uint32_t version;   /* To detect ram block addition/removal */

Is there a reason why RAMList.version is uint32_t but last_version is uint64_t?

No, my bad, they both should be consistent.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/5] Migration thread mutex

2011-08-29 Thread Marcelo Tosatti
On Sat, Aug 27, 2011 at 02:09:46PM -0400, Umesh Deshpande wrote:
 This patch implements migrate_ram mutex, which protects the RAMBlock list
 traversal in the migration thread during the transfer of a ram from their
 addition/removal from the iothread.
 
 Note: Combination of iothread mutex and migration thread mutex works as a
 rw-lock. Both mutexes are acquired while modifying the ram_list members or RAM
 block list.
 
 Signed-off-by: Umesh Deshpande udesh...@redhat.com
 ---
  arch_init.c   |   21 +
  cpu-all.h |3 +++
  exec.c|   23 +++
  qemu-common.h |2 ++
  4 files changed, 49 insertions(+), 0 deletions(-)
 
 diff --git a/arch_init.c b/arch_init.c
 index 484b39d..9d02270 100644
 --- a/arch_init.c
 +++ b/arch_init.c
 @@ -109,6 +109,7 @@ static int is_dup_page(uint8_t *page, uint8_t ch)
  
  static RAMBlock *last_block;
  static ram_addr_t last_offset;
 +static uint64_t last_version;
  
  static int ram_save_block(QEMUFile *f)
  {
 @@ -170,6 +171,7 @@ static int ram_save_block(QEMUFile *f)
  
  last_block = block;
  last_offset = offset;
 +last_version = ram_list.version;
  
  return bytes_sent;
  }
 @@ -270,6 +272,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, 
 void *opaque)
  bytes_transferred = 0;
  last_block = NULL;
  last_offset = 0;
 +last_version = ram_list.version = 0;
  sort_ram_list();
  
  /* Make sure all dirty bits are set */
 @@ -298,6 +301,17 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, 
 void *opaque)
  bytes_transferred_last = bytes_transferred;
  bwidth = qemu_get_clock_ns(rt_clock);
  
 +if (stage != 3) {
 +qemu_mutex_lock_migrate_ram();
 +qemu_mutex_unlock_iothread();
 +}

Dropping the iothread lock from within a timer handler is not safe.
This change to ram_save_live should be moved to the patch where 
migration thread is introduced.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html