separate thread for VM migration

RFC PATCH Superseded Umesh Deshpande 4 participants 11 messages Jul 29, 2011 – Aug 2, 2011 KVM

Share & tools

Umesh Deshpande

author

2011-07-29 20:57 UTC #0 lore raw

v2 cover separate thread for VM migration

Following patch deals with VCPU and iothread starvation during the migration of a guest. Currently the iothread is responsible for performing the guest migration. It holds qemu_mutex during the migration and doesn't allow VCPU to enter the qemu mode and delays its return to the guest. The guest migration, executed as an iohandler also delays the execution of other iohandlers. In the following patch series,

The migration has been moved to a separate thread to reduce the qemu_mutex contention and iohandler starvation.

Also current dirty bitmap is split into per memslot bitmap to reduce its size.

Umesh Deshpande (3):
separate thread for VM migration
fine grained qemu_mutex locking for migration
per memslot dirty bitmap

 arch_init.c     |   14 ++++++++++--
 buffered_file.c |   28 ++++++++++++++++---------
 buffered_file.h |    4 +++
 cpu-all.h       |   40 ++++++++++++++++++++++++++++++------
 exec.c          |   38 +++++++++++++++++++++-------------
 migration.c     |   60 ++++++++++++++++++++++++++++++++++++++++++++----------
 migration.h     |    3 ++
 savevm.c        |   22 +-------------------
 savevm.h        |   29 ++++++++++++++++++++++++++
 xen-all.c       |    6 +---
 10 files changed, 173 insertions(+), 71 deletions(-)
 create mode 100644 savevm.h

Signature · 1 line

1.7.4.1

Umesh Deshpande

author

2011-07-29 20:57 UTC #1 lore raw

RFC PATCH v2 1/3 separate thread for VM migration

This patch creates a separate thread for the guest migration on the source side.

Signed-off-by Umesh Deshpande <[email protected]>

---
 buffered_file.c |   28 ++++++++++++++++---------
 buffered_file.h |    4 +++
 migration.c     |   59 +++++++++++++++++++++++++++++++++++++++++++-----------
 migration.h     |    3 ++
 savevm.c        |   22 +-------------------
 savevm.h        |   29 +++++++++++++++++++++++++++
 6 files changed, 102 insertions(+), 43 deletions(-)
 create mode 100644 savevm.h

buffered_file.c +18 ~~-10~~

index 41b42c3..d4146bf 100644
@@ -16,12 +16,16 @@
 #include "qemu-timer.h"
 #include "qemu-char.h"
 #include "buffered_file.h"
+#include "migration.h"
+#include "savevm.h"
+#include "qemu-thread.h"
 
 //#define DEBUG_BUFFERED_FILE
 
 typedef struct QEMUFileBuffered
 {
     BufferedPutFunc *put_buffer;
+    BufferedBeginFunc *begin;
     BufferedPutReadyFunc *put_ready;
     BufferedWaitForUnfreezeFunc *wait_for_unfreeze;
     BufferedCloseFunc *close;

@@ -35,6 +39,7 @@ typedef struct QEMUFileBuffered
     size_t buffer_size;
     size_t buffer_capacity;
     QEMUTimer *timer;
+    QemuThread thread;
 } QEMUFileBuffered;
 
 #ifdef DEBUG_BUFFERED_FILE

@@ -181,8 +186,6 @@ static int buffered_close(void *opaque)
 
     ret = s->close(s->opaque);
 
-    qemu_del_timer(s->timer);
-    qemu_free_timer(s->timer);
     qemu_free(s->buffer);
     qemu_free(s);

@@ -228,17 +231,15 @@ static int64_t buffered_get_rate_limit(void *opaque)
     return s->xfer_limit;
 }
 
-static void buffered_rate_tick(void *opaque)
+void buffered_rate_tick(QEMUFile *file)
 {
-    QEMUFileBuffered *s = opaque;
+    QEMUFileBuffered *s = file->opaque;
 
     if (s->has_error) {
         buffered_close(s);
         return;
     }
 
-    qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
-
     if (s->freeze_output)
         return;

@@ -250,9 +251,17 @@ static void buffered_rate_tick(void *opaque)
     s->put_ready(s->opaque);
 }
 
+static void *migrate_vm(void *opaque)
+{
+    QEMUFileBuffered *s = opaque;
+    s->begin(s->opaque);
+    return NULL;
+}
+
 QEMUFile *qemu_fopen_ops_buffered(void *opaque,
                                   size_t bytes_per_sec,
                                   BufferedPutFunc *put_buffer,
+                                  BufferedBeginFunc *begin,
                                   BufferedPutReadyFunc *put_ready,
                                   BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
                                   BufferedCloseFunc *close)

@@ -264,6 +273,7 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,
     s->opaque = opaque;
     s->xfer_limit = bytes_per_sec / 10;
     s->put_buffer = put_buffer;
+    s->begin = begin;
     s->put_ready = put_ready;
     s->wait_for_unfreeze = wait_for_unfreeze;
     s->close = close;

@@ -271,11 +281,9 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,
     s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
                              buffered_close, buffered_rate_limit,
                              buffered_set_rate_limit,
-			     buffered_get_rate_limit);
-
-    s->timer = qemu_new_timer_ms(rt_clock, buffered_rate_tick, s);
+                             buffered_get_rate_limit);
 
-    qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
+    qemu_thread_create(&s->thread, migrate_vm, s);
 
     return s->file;
 }

buffered_file.h +4 -0

index 98d358b..cfe2833 100644
@@ -17,12 +17,16 @@
 #include "hw/hw.h"
 
 typedef ssize_t (BufferedPutFunc)(void *opaque, const void *data, size_t size);
+typedef void (BufferedBeginFunc)(void *opaque);
 typedef void (BufferedPutReadyFunc)(void *opaque);
 typedef void (BufferedWaitForUnfreezeFunc)(void *opaque);
 typedef int (BufferedCloseFunc)(void *opaque);
 
+void buffered_rate_tick(QEMUFile *file);
+
 QEMUFile *qemu_fopen_ops_buffered(void *opaque, size_t xfer_limit,
                                   BufferedPutFunc *put_buffer,
+                                  BufferedBeginFunc *begin,
                                   BufferedPutReadyFunc *put_ready,
                                   BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
                                   BufferedCloseFunc *close);

migration.c +47 ~~-12~~

118 lines not on this page · raw message

migration.h +3 -0

index 050c56c..8ed34ab 100644
@@ -48,6 +48,7 @@ struct FdMigrationState
     int (*get_error)(struct FdMigrationState*);
     int (*close)(struct FdMigrationState*);
     int (*write)(struct FdMigrationState*, const void *, size_t);
+    void (*callback)(void *);
     void *opaque;
 };

@@ -118,6 +119,8 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);
 
 void migrate_fd_connect(FdMigrationState *s);
 
+void migrate_fd_begin(void *opaque);
+
 void migrate_fd_put_ready(void *opaque);
 
 int migrate_fd_get_status(MigrationState *mig_state);

savevm.c +1 ~~-21~~

index 8139bc7..4859a34 100644
@@ -82,6 +82,7 @@
 #include "qemu_socket.h"
 #include "qemu-queue.h"
 #include "cpus.h"
+#include "savevm.h"
 
 #define SELF_ANNOUNCE_ROUNDS 5

@@ -155,27 +156,6 @@ void qemu_announce_self(void)
 /***********************************************************/
 /* savevm/loadvm support */
 
-#define IO_BUF_SIZE 32768
-
-struct QEMUFile {
-    QEMUFilePutBufferFunc *put_buffer;
-    QEMUFileGetBufferFunc *get_buffer;
-    QEMUFileCloseFunc *close;
-    QEMUFileRateLimit *rate_limit;
-    QEMUFileSetRateLimit *set_rate_limit;
-    QEMUFileGetRateLimit *get_rate_limit;
-    void *opaque;
-    int is_write;
-
-    int64_t buf_offset; /* start of buffer when writing, end of buffer
-                           when reading */
-    int buf_index;
-    int buf_size; /* 0 when writing */
-    uint8_t buf[IO_BUF_SIZE];
-
-    int has_error;
-};
-
 typedef struct QEMUFileStdio
 {
     FILE *stdio_file;

savevm.h new file +29 -0

new file mode 100644
index 0000000..954eded
@@ -0,0 +1,29 @@
+#ifndef QEMU_SAVEVM_H
+#define QEMU_SAVEVM_H
+
+#include "hw/hw.h"
+
+#define IO_BUF_SIZE 32768
+
+struct QEMUFile {
+    QEMUFilePutBufferFunc *put_buffer;
+    QEMUFileGetBufferFunc *get_buffer;
+    QEMUFileCloseFunc *close;
+    QEMUFileRateLimit *rate_limit;
+    QEMUFileSetRateLimit *set_rate_limit;
+    QEMUFileGetRateLimit *get_rate_limit;
+    void *opaque;
+    int is_write;
+
+    int64_t buf_offset; /* start of buffer when writing, end of buffer
+                           when reading */
+    int buf_index;
+    int buf_size; /* 0 when
+     * writing
+     * */
+    uint8_t buf[IO_BUF_SIZE];
+
+    int has_error;
+};
+
+#endif

Signature · 1 line

1.7.4.1

Paolo Bonzini

maintainer

2011-08-01 09:37 UTC #2 lore raw

Quoted 3 lines · Umesh Deshpande

This patch creates a separate thread for the guest migration on the source side.
Signed-off-by Umesh Deshpande<[email protected]>

Looks pretty good!

One thing that shows, is that the interface separation between buffered_file.c is migration.c is pretty weird. Your patch makes it somewhat worse, but it was like this before so it's not your fault. The good thing is that if buffered_file.c uses threads, you can fix a large part of this and get even simpler code:

1) there is really just one way to implement migrate_fd_put_notify, and with your simplifications it does not belong anymore in migration.c.

2) s->callback is actually not NULL exactly if s->file->frozen_output is true, you can remove it as well;

3) buffered_close is messy because it can be called from both the iothread (monitor->migrate_fd_cancel->migrate_fd_cleanup->qemu_fclose) or the migration thread (after qemu_savevm_state_complete). But buffered_close is actually very similar to your thread function (it does flush+wait_for_unfreeze, basically)! So buffered_close can be simply:

    s->closed = 1;
    ret = qemu_thread_join(s->thread); /* doesn't exist yet :) */
    qemu_free(...);
    return ret;

Another nit is that here:

migration.c ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) · line 396

Quoted 12 lines

migration.c +12 -0

+        if (migrate_fd_check_expire()) {
+            buffered_rate_tick(s->file);
+        }
+
+        if (s->state != MIG_STATE_ACTIVE) {
+            break;
+        }
+
+        if (s->callback) {
+            migrate_fd_wait_for_unfreeze(s);
+            s->callback(s);
+        }

you can still have a busy wait.

Putting it all together, you can move the thread function back to buffered_file.c like:

    while (!s->closed || (!s->has_error && s->buffer_size)) {
        if (s->freeze_output) {
            qemu_mutex_unlock_iothread();
            s->wait_for_unfreeze(s);
            qemu_mutex_lock_iothread();
            /* This comes from qemu_file_put_notify (via
               buffered_put_buffer---can be simplified a lot too?).
            s->freeze_output = 0;
            /* Test again for cancellation.  */
            continue;
        }

        int64_t current_time = qemu_get_clock_ms(rt_clock);
        if (s->expire_time > current_time) {
            struct timeval tv = { .tv_sec = 0, .tv_usec = ... };
            qemu_mutex_unlock_iothread();
            select (0, NULL, NULL, NULL, &tv);
            qemu_mutex_lock_iothread();
            s->expire_time = qemu_get_clock_ms(rt_clock) + 100;
            continue;
        }

        /* This comes from buffered_rate_tick.  */
        s->bytes_xfer = 0;
        buffered_flush(s);
        if (!s->closed) {
            s->put_ready(s->opaque);
        }
    }

    ret = s->close(s->opaque);
    ...

Does it look sane?

Paolo

Umesh Deshpande

author

2011-08-01 21:00 UTC #3 lore raw

Quoted 72 lines · Paolo Bonzini

Quoted 3 lines · Umesh Deshpande
This patch creates a separate thread for the guest migration on the source side.
Signed-off-by Umesh Deshpande<[email protected]>
Looks pretty good!
One thing that shows, is that the interface separation between buffered_file.c is migration.c is pretty weird. Your patch makes it somewhat worse, but it was like this before so it's not your fault. The good thing is that if buffered_file.c uses threads, you can fix a large part of this and get even simpler code:
1) there is really just one way to implement migrate_fd_put_notify, and with your simplifications it does not belong anymore in migration.c.
2) s->callback is actually not NULL exactly if s->file->frozen_output is true, you can remove it as well;
3) buffered_close is messy because it can be called from both the iothread (monitor->migrate_fd_cancel->migrate_fd_cleanup->qemu_fclose) or the migration thread (after qemu_savevm_state_complete). But buffered_close is actually very similar to your thread function (it does flush+wait_for_unfreeze, basically)! So buffered_close can be simply:
    s->closed = 1;
    ret = qemu_thread_join(s->thread); /* doesn't exist yet :) */
    qemu_free(...);
    return ret;
Another nit is that here:
migration.c ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) · line 396
Quoted 12 lines
migration.c +12 -0
+        if (migrate_fd_check_expire()) {
+            buffered_rate_tick(s->file);
+        }
+
+        if (s->state != MIG_STATE_ACTIVE) {
+            break;
+        }
+
+        if (s->callback) {
+            migrate_fd_wait_for_unfreeze(s);
+            s->callback(s);
+        }
you can still have a busy wait.
Putting it all together, you can move the thread function back to buffered_file.c like:
    while (!s->closed || (!s->has_error && s->buffer_size)) {
        if (s->freeze_output) {
            qemu_mutex_unlock_iothread();
            s->wait_for_unfreeze(s);
            qemu_mutex_lock_iothread();
            /* This comes from qemu_file_put_notify (via
               buffered_put_buffer---can be simplified a lot too?).
            s->freeze_output = 0;
            /* Test again for cancellation.  */
            continue;
        }

        int64_t current_time = qemu_get_clock_ms(rt_clock);
        if (s->expire_time > current_time) {
            struct timeval tv = { .tv_sec = 0, .tv_usec = ... };
            qemu_mutex_unlock_iothread();
            select (0, NULL, NULL, NULL, &tv);
            qemu_mutex_lock_iothread();
            s->expire_time = qemu_get_clock_ms(rt_clock) + 100;
            continue;
        }

        /* This comes from buffered_rate_tick.  */
        s->bytes_xfer = 0;
        buffered_flush(s);
        if (!s->closed) {
            s->put_ready(s->opaque);
        }
    }

    ret = s->close(s->opaque);
    ...

Quoted 2 lines

Does it look sane?

I kept this in migration.c to call qemu_savevm_state_begin. (The way it is done currently. i.e. to keep access to FdMigrationState in migration.c) Calling it from buffered_file.c would be inconsistent in that sense. or we will have to call it from the iothread before spawning the migration thread.

Also why is the separation between FdMigrationState and QEMUFileBuffered is required. Is QEMUFileBuffered designed to use also for things other than migration?

Thanks
Umesh

Quoted 2 lines

Paolo

Paolo Bonzini

maintainer

2011-08-02 07:44 UTC #4 lore raw

Quoted 6 lines · Umesh Deshpande

Quoted 1 line
I kept this in migration.c to call qemu_savevm_state_begin. (The way it is done currently. i.e. to keep access to FdMigrationState in migration.c) Calling it from buffered_file.c would be inconsistent in that sense. or we will have to call it from the iothread before spawning the migration thread.

Right, I missed that. Perhaps you can call it the first time put_ready is called.

Quoted 3 lines

Also why is the separation between FdMigrationState and QEMUFileBuffered is required. Is QEMUFileBuffered designed to use also for things other than migration?

No, but let's keep it this way for now. It may be an annoyance, but it also helps making a reusable architecture, and it can probably be cleaned up substantially with thread support.

Paolo

Umesh Deshpande

author

2011-07-29 20:57 UTC #5 lore raw

RFC PATCH v2 2/3 fine grained qemu_mutex locking for migration

In reply to Umesh Deshpande

In the migration thread, qemu_mutex is released during the most time consuming part. i.e. during is_dup_page which identifies the uniform data pages and during the put_buffer. qemu_mutex is also released while blocking on select to wait for the descriptor to become ready for writes.

Signed-off-by Umesh Deshpande <[email protected]>

---
 arch_init.c |   14 +++++++++++---
 migration.c |   11 +++++++----
 2 files changed, 18 insertions(+), 7 deletions(-)

arch_init.c +11 -3

index 484b39d..cd545bc 100644
@@ -110,7 +110,7 @@ static int is_dup_page(uint8_t *page, uint8_t ch)
 static RAMBlock *last_block;
 static ram_addr_t last_offset;
 
-static int ram_save_block(QEMUFile *f)
+static int ram_save_block(QEMUFile *f, int stage)
 {
     RAMBlock *block = last_block;
     ram_addr_t offset = last_offset;

@@ -131,6 +131,10 @@ static int ram_save_block(QEMUFile *f)
                                             current_addr + TARGET_PAGE_SIZE,
                                             MIGRATION_DIRTY_FLAG);
 
+            if (stage != 3) {
+                qemu_mutex_unlock_iothread();
+            }
+
             p = block->host + offset;
 
             if (is_dup_page(p, *p)) {

@@ -153,6 +157,10 @@ static int ram_save_block(QEMUFile *f)
                 bytes_sent = TARGET_PAGE_SIZE;
             }
 
+            if (stage != 3) {
+                qemu_mutex_lock_iothread();
+            }
+
             break;
         }

@@ -301,7 +309,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
     while (!qemu_file_rate_limit(f)) {
         int bytes_sent;
 
-        bytes_sent = ram_save_block(f);
+        bytes_sent = ram_save_block(f, stage);
         bytes_transferred += bytes_sent;
         if (bytes_sent == 0) { /* no more blocks */
             break;

@@ -322,7 +330,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
         int bytes_sent;
 
         /* flush all remaining blocks regardless of rate limiting */
-        while ((bytes_sent = ram_save_block(f)) != 0) {
+        while ((bytes_sent = ram_save_block(f, stage)) != 0) {
             bytes_transferred += bytes_sent;
         }
         cpu_physical_memory_set_dirty_tracking(0);

migration.c +7 -4

index bf86067..992fef5 100644
@@ -375,15 +375,19 @@ void migrate_fd_begin(void *arg)
     if (ret < 0) {
         DPRINTF("failed, %d\n", ret);
         migrate_fd_error(s);
-        goto out;
+        qemu_mutex_unlock_iothread();
+        return;
     }
 
     expire_time = qemu_get_clock_ms(rt_clock) + 100;
     migrate_fd_put_ready(s);
+    qemu_mutex_unlock_iothread();
 
     while (s->state == MIG_STATE_ACTIVE) {
         if (migrate_fd_check_expire()) {
+            qemu_mutex_lock_iothread();
             buffered_rate_tick(s->file);
+            qemu_mutex_unlock_iothread();
         }
 
         if (s->state != MIG_STATE_ACTIVE) {

@@ -392,12 +396,11 @@ void migrate_fd_begin(void *arg)
 
         if (s->callback) {
             migrate_fd_wait_for_unfreeze(s);
+            qemu_mutex_lock_iothread();
             s->callback(s);
+            qemu_mutex_unlock_iothread();
         }
     }
-
-out:
-    qemu_mutex_unlock_iothread();
 }

Signature · 1 line

1.7.4.1

Paolo Bonzini

maintainer

2011-08-01 09:39 UTC #6 lore raw

Re: [RFC PATCH v2 2/3] fine grained qemu_mutex locking for migration

Quoted 12 lines · Umesh Deshpande

migration.c +3 -0

+    qemu_mutex_unlock_iothread();
 
      while (s->state == MIG_STATE_ACTIVE) {
          if (migrate_fd_check_expire()) {
+            qemu_mutex_lock_iothread();
              buffered_rate_tick(s->file);
+            qemu_mutex_unlock_iothread();
          }
 
          if (s->state != MIG_STATE_ACTIVE) {

@@ -392,12 +396,11 @@ void migrate_fd_begin(void *arg)

migration.c void migrate_fd_begin(void *arg) · line 400

Quoted 10 lines

migration.c +2 -3

          if (s->callback) {
              migrate_fd_wait_for_unfreeze(s);
+            qemu_mutex_lock_iothread();
              s->callback(s);
+            qemu_mutex_unlock_iothread();
          }
      }
-
-out:
-    qemu_mutex_unlock_iothread();

I think it's clearer to unlock explicitly around the waiting points (see review of 1/3). In fact, I think you're working around the busy wait by accessing s->state outside the lock, right? I don't think this is provably safe; moving the knowledge of the thread entirely within buffered_file.c also fixes this, because then the lifetimes of the thread and the QEMUFile are much clearer.

Thanks,

Paolo

Marcelo Tosatti

2011-08-02 16:30 UTC #7 lore raw

Re: [RFC PATCH v2 2/3] fine grained qemu_mutex locking for migration

In reply to Umesh Deshpande

Quoted 36 lines · Umesh Deshpande

Signed-off-by Umesh Deshpande <[email protected]>

---
 arch_init.c |   14 +++++++++++---
 migration.c |   11 +++++++----
 2 files changed, 18 insertions(+), 7 deletions(-)

arch_init.c +5 -1

index 484b39d..cd545bc 100644
@@ -110,7 +110,7 @@ static int is_dup_page(uint8_t *page, uint8_t ch)
 static RAMBlock *last_block;
 static ram_addr_t last_offset;
 
-static int ram_save_block(QEMUFile *f)
+static int ram_save_block(QEMUFile *f, int stage)
 {
     RAMBlock *block = last_block;
     ram_addr_t offset = last_offset;

@@ -131,6 +131,10 @@ static int ram_save_block(QEMUFile *f)
                                             current_addr + TARGET_PAGE_SIZE,
                                             MIGRATION_DIRTY_FLAG);
 
+            if (stage != 3) {
+                qemu_mutex_unlock_iothread();
+            }
+
             p = block->host + offset;
 
             if (is_dup_page(p, *p)) {

arch_init.c static int ram_save_block(QEMUFile *f) · line 163

Quoted 7 lines

arch_init.c +4 -0

@@ -153,6 +157,10 @@ static int ram_save_block(QEMUFile *f)
                 bytes_sent = TARGET_PAGE_SIZE;
             }
 
+            if (stage != 3) {
+                qemu_mutex_lock_iothread();
+            }
+

Batching multiple pages (instead of a single page per lock/unlock cycle) is probably worthwhile.

Umesh Deshpande

author

2011-07-29 20:57 UTC #8 lore raw

RFC PATCH v2 3/3 Per memslot dirty bitmap

In reply to Umesh Deshpande

This patch creates a separate dirty bitmap for each slot. Currently dirty bitmap is created for addresses ranging from 0 to the end address of the last memory slot. Since the memslots are not necessarily contiguous, current bitmap might contain empty region or holes that doesn't represent any VM pages. This patch reduces the size of the dirty bitmap by allocating per memslot dirty bitmaps.

Signed-off-by Umesh Deshpande <[email protected]>

---
 cpu-all.h |   40 +++++++++++++++++++++++++++++++++-------
 exec.c    |   38 +++++++++++++++++++++++---------------
 xen-all.c |    6 ++----
 3 files changed, 58 insertions(+), 26 deletions(-)

cpu-all.h +33 -7

index e839100..9517a9b 100644
@@ -920,6 +920,7 @@ extern ram_addr_t ram_size;
 
 typedef struct RAMBlock {
     uint8_t *host;
+    uint8_t *phys_dirty;
     ram_addr_t offset;
     ram_addr_t length;
     uint32_t flags;

@@ -931,7 +932,6 @@ typedef struct RAMBlock {
 } RAMBlock;
 
 typedef struct RAMList {
-    uint8_t *phys_dirty;
     QLIST_HEAD(ram, RAMBlock) blocks;
 } RAMList;
 extern RAMList ram_list;

@@ -961,32 +961,55 @@ extern int mem_prealloc;
 #define CODE_DIRTY_FLAG      0x02
 #define MIGRATION_DIRTY_FLAG 0x08
 
+RAMBlock *qemu_addr_to_ramblock(ram_addr_t);
+
+static inline int get_page_nr(ram_addr_t addr, RAMBlock **block)
+{
+    int page_nr;
+    *block = qemu_addr_to_ramblock(addr);
+
+    page_nr = addr - (*block)->offset;
+    page_nr = page_nr >> TARGET_PAGE_BITS;
+
+    return page_nr;
+}
+
 /* read dirty bit (return 0 or 1) */
 static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
 {
-    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] == 0xff;
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    return block->phys_dirty[page_nr] == 0xff;
 }
 
 static inline int cpu_physical_memory_get_dirty_flags(ram_addr_t addr)
 {
-    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS];
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    return block->phys_dirty[page_nr];
 }
 
 static inline int cpu_physical_memory_get_dirty(ram_addr_t addr,
                                                 int dirty_flags)
 {
-    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & dirty_flags;
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    return block->phys_dirty[page_nr] & dirty_flags;
 }
 
 static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
 {
-    ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] = 0xff;
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    block->phys_dirty[page_nr] = 0xff;
 }
 
 static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
                                                       int dirty_flags)
 {
-    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    return block->phys_dirty[page_nr] |= dirty_flags;
 }
 
 static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,

@@ -995,10 +1018,13 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
 {
     int i, mask, len;
     uint8_t *p;
+    RAMBlock *block;
+    int page_nr = get_page_nr(start, &block);
 
     len = length >> TARGET_PAGE_BITS;
     mask = ~dirty_flags;
-    p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS);
+
+    p = block->phys_dirty + page_nr;
     for (i = 0; i < len; i++) {
         p[i] &= mask;
     }

exec.c +23 ~~-15~~

index 0e2ce57..6312550 100644
@@ -2106,6 +2106,10 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
         abort();
     }
 
+    if (kvm_enabled()) {
+        return;
+    }
+
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
         int mmu_idx;
         for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {

@@ -2894,17 +2898,6 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
     return offset;
 }
 
-static ram_addr_t last_ram_offset(void)
-{
-    RAMBlock *block;
-    ram_addr_t last = 0;
-
-    QLIST_FOREACH(block, &ram_list.blocks, next)
-        last = MAX(last, block->offset + block->length);
-
-    return last;
-}
-
 ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
                                    ram_addr_t size, void *host)
 {

@@ -2974,10 +2967,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
 
     QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
 
-    ram_list.phys_dirty = qemu_realloc(ram_list.phys_dirty,
-                                       last_ram_offset() >> TARGET_PAGE_BITS);
-    memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
-           0xff, size >> TARGET_PAGE_BITS);
+    new_block->phys_dirty = qemu_mallocz(new_block->length >> TARGET_PAGE_BITS);
+    memset(new_block->phys_dirty, 0xff, new_block->length >> TARGET_PAGE_BITS);
 
     if (kvm_enabled())
         kvm_setup_guest_memory(new_block->host, size);

@@ -3141,6 +3132,23 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
     return NULL;
 }
 
+RAMBlock *qemu_addr_to_ramblock(ram_addr_t addr)
+{
+    RAMBlock *block;
+
+    QLIST_FOREACH(block, &ram_list.blocks, next) {
+        if (addr - block->offset < block->length) {
+            return block;
+        }
+    }
+
+    fprintf(stderr, "Bad ram offset %" PRIx64 "\n", (uint64_t)addr);
+    abort();
+
+    return NULL;
+}
+
+
 /* Return a host pointer to ram allocated with qemu_ram_alloc.
  * Same as qemu_get_ram_ptr but avoid reordering ramblocks.
  */

xen-all.c +2 -4

index fcb106f..6782e68 100644
@@ -147,10 +147,8 @@ static void xen_ram_init(ram_addr_t ram_size)
 
     QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
 
-    ram_list.phys_dirty = qemu_realloc(ram_list.phys_dirty,
-                                       new_block->length >> TARGET_PAGE_BITS);
-    memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
-           0xff, new_block->length >> TARGET_PAGE_BITS);
+    new_block->phys_dirty = qemu_mallocz(new_block->length >> TARGET_PAGE_BITS);
+    memset(new_block->phys_dirty, 0xff, new_block->length >> TARGET_PAGE_BITS);
 
     if (ram_size >= 0xe0000000 ) {
         above_4g_mem_size = ram_size - 0xe0000000;

Signature · 1 line

1.7.4.1

Marcelo Tosatti

2011-08-02 16:29 UTC #9 lore raw

Re: [RFC PATCH v2 3/3] Per memslot dirty bitmap

Quoted 113 lines · Umesh Deshpande

Signed-off-by Umesh Deshpande <[email protected]>

---
 cpu-all.h |   40 +++++++++++++++++++++++++++++++++-------
 exec.c    |   38 +++++++++++++++++++++++---------------
 xen-all.c |    6 ++----
 3 files changed, 58 insertions(+), 26 deletions(-)

cpu-all.h +33 -7

index e839100..9517a9b 100644
@@ -920,6 +920,7 @@ extern ram_addr_t ram_size;
 
 typedef struct RAMBlock {
     uint8_t *host;
+    uint8_t *phys_dirty;
     ram_addr_t offset;
     ram_addr_t length;
     uint32_t flags;

@@ -931,7 +932,6 @@ typedef struct RAMBlock {
 } RAMBlock;
 
 typedef struct RAMList {
-    uint8_t *phys_dirty;
     QLIST_HEAD(ram, RAMBlock) blocks;
 } RAMList;
 extern RAMList ram_list;

@@ -961,32 +961,55 @@ extern int mem_prealloc;
 #define CODE_DIRTY_FLAG      0x02
 #define MIGRATION_DIRTY_FLAG 0x08
 
+RAMBlock *qemu_addr_to_ramblock(ram_addr_t);
+
+static inline int get_page_nr(ram_addr_t addr, RAMBlock **block)
+{
+    int page_nr;
+    *block = qemu_addr_to_ramblock(addr);
+
+    page_nr = addr - (*block)->offset;
+    page_nr = page_nr >> TARGET_PAGE_BITS;
+
+    return page_nr;
+}
+
 /* read dirty bit (return 0 or 1) */
 static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
 {
-    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] == 0xff;
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    return block->phys_dirty[page_nr] == 0xff;
 }
 
 static inline int cpu_physical_memory_get_dirty_flags(ram_addr_t addr)
 {
-    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS];
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    return block->phys_dirty[page_nr];
 }
 
 static inline int cpu_physical_memory_get_dirty(ram_addr_t addr,
                                                 int dirty_flags)
 {
-    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & dirty_flags;
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    return block->phys_dirty[page_nr] & dirty_flags;
 }
 
 static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
 {
-    ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] = 0xff;
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    block->phys_dirty[page_nr] = 0xff;
 }
 
 static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
                                                       int dirty_flags)
 {
-    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
+    RAMBlock *block;
+    int page_nr = get_page_nr(addr, &block);
+    return block->phys_dirty[page_nr] |= dirty_flags;
 }
 
 static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,

@@ -995,10 +1018,13 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
 {
     int i, mask, len;
     uint8_t *p;
+    RAMBlock *block;
+    int page_nr = get_page_nr(start, &block);
 
     len = length >> TARGET_PAGE_BITS;
     mask = ~dirty_flags;
-    p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS);
+
+    p = block->phys_dirty + page_nr;
         p[i] &= mask;
     }

exec.c +0 -0

index 0e2ce57..6312550 100644

exec.c void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end, · line 2112

Quoted 7 lines

exec.c +4 -0

@@ -2106,6 +2106,10 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
         abort();
     }
 
+    if (kvm_enabled()) {
+        return;
+    }
+

This belongs to a separate patch.

shawn che

2011-08-01 09:41 UTC #10 lore raw

In reply to Umesh Deshpande

Hi ,

I am studying KVM, but I have a question to ask.I wish someone can spare time to help me.

What does the file "Assigned-dev.c" use for? I add some printk in the funcitons in this file,but it seldom work.
Is it used for VT-d or others? can you give me some suggestions?

Thank you so much for your feedback...