| Message ID | 1424883128-9841-34-git-send-email-dgilbert@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Feb 25, 2015 at 04:51:56PM +0000, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > On receiving MIG_RPCOMM_REQ_PAGES look up the address and > queue the page. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > arch_init.c | 55 +++++++++++++++++++++++++++++++++++++++++++ > include/exec/cpu-all.h | 2 -- > include/migration/migration.h | 21 +++++++++++++++++ > include/qemu/typedefs.h | 1 + > migration/migration.c | 33 +++++++++++++++++++++++++- > trace-events | 3 ++- > 6 files changed, 111 insertions(+), 4 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index d2c4457..9d8fc6b 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -669,6 +669,61 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset, > } > > /* > + * Queue the pages for transmission, e.g. a request from postcopy destination > + * ms: MigrationStatus in which the queue is held > + * rbname: The RAMBlock the request is for - may be NULL (to mean reuse last) > + * start: Offset from the start of the RAMBlock > + * len: Length (in bytes) to send > + * Return: 0 on success > + */ > +int ram_save_queue_pages(MigrationState *ms, const char *rbname, > + ram_addr_t start, ram_addr_t len) > +{ > + RAMBlock *ramblock; > + > + if (!rbname) { > + /* Reuse last RAMBlock */ > + ramblock = ms->last_req_rb; > + > + if (!ramblock) { > + /* > + * Shouldn't happen, we can't reuse the last RAMBlock if > + * it's the 1st request. > + */ > + error_report("ram_save_queue_pages no previous block"); > + return -1; > + } > + } else { > + ramblock = ram_find_block(rbname); > + > + if (!ramblock) { > + /* We shouldn't be asked for a non-existent RAMBlock */ > + error_report("ram_save_queue_pages no block '%s'", rbname); > + return -1; > + } > + } > + trace_ram_save_queue_pages(ramblock->idstr, start, len); > + if (start+len > ramblock->used_length) { > + error_report("%s request overrun start=%zx len=%zx blocklen=%zx", > + __func__, start, len, ramblock->used_length); > + return -1; > + } > + > + struct MigrationSrcPageRequest *new_entry = > + g_malloc0(sizeof(struct MigrationSrcPageRequest)); > + new_entry->rb = ramblock; > + new_entry->offset = start; > + new_entry->len = len; > + ms->last_req_rb = ramblock; > + > + qemu_mutex_lock(&ms->src_page_req_mutex); > + QSIMPLEQ_INSERT_TAIL(&ms->src_page_requests, new_entry, next_req); > + qemu_mutex_unlock(&ms->src_page_req_mutex); > + > + return 0; > +} > + > +/* > * ram_find_and_save_block: Finds a page to send and sends it to f > * > * Returns: The number of bytes written. > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h > index 2c48286..3088000 100644 > --- a/include/exec/cpu-all.h > +++ b/include/exec/cpu-all.h > @@ -265,8 +265,6 @@ CPUArchState *cpu_copy(CPUArchState *env); > > /* memory API */ > > -typedef struct RAMBlock RAMBlock; > - > struct RAMBlock { > struct MemoryRegion *mr; > uint8_t *host; > diff --git a/include/migration/migration.h b/include/migration/migration.h > index 2c15d63..b1c7cad 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -100,6 +100,18 @@ MigrationIncomingState *migration_incoming_get_current(void); > MigrationIncomingState *migration_incoming_state_new(QEMUFile *f); > void migration_incoming_state_destroy(void); > > +/* > + * An outstanding page request, on the source, having been received > + * and queued > + */ > +struct MigrationSrcPageRequest { > + RAMBlock *rb; > + hwaddr offset; > + hwaddr len; > + > + QSIMPLEQ_ENTRY(MigrationSrcPageRequest) next_req; > +}; > + > struct MigrationState > { > int64_t bandwidth_limit; > @@ -142,6 +154,12 @@ struct MigrationState > * of the postcopy phase > */ > unsigned long *sentmap; > + > + /* Queue of outstanding page requests from the destination */ > + QemuMutex src_page_req_mutex; > + QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_requests; > + /* The RAMBlock used in the last src_page_request */ > + RAMBlock *last_req_rb; > }; > > void process_incoming_migration(QEMUFile *f); > @@ -276,6 +294,9 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, > ram_addr_t offset, size_t size, > int *bytes_sent); > > +int ram_save_queue_pages(MigrationState *ms, const char *rbname, > + ram_addr_t start, ram_addr_t len); > + > PostcopyState postcopy_state_get(MigrationIncomingState *mis); > > /* Set the state and return the old state */ > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h > index 0651275..396044d 100644 > --- a/include/qemu/typedefs.h > +++ b/include/qemu/typedefs.h > @@ -73,6 +73,7 @@ typedef struct QEMUSGList QEMUSGList; > typedef struct QEMUSizedBuffer QEMUSizedBuffer; > typedef struct QEMUTimerListGroup QEMUTimerListGroup; > typedef struct QEMUTimer QEMUTimer; > +typedef struct RAMBlock RAMBlock; > typedef struct Range Range; > typedef struct SerialState SerialState; > typedef struct SHPCDevice SHPCDevice; > diff --git a/migration/migration.c b/migration/migration.c > index 2e9d0dd..939f426 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -26,6 +26,8 @@ > #include "qemu/thread.h" > #include "qmp-commands.h" > #include "trace.h" > +#include "exec/memory.h" > +#include "exec/address-spaces.h" > > enum MigrationPhase { > MIG_STATE_ERROR = -1, > @@ -495,6 +497,15 @@ static void migrate_fd_cleanup(void *opaque) > > migrate_fd_cleanup_src_rp(s); > > + /* This queue generally should be empty - but in the case of a failed > + * migration might have some droppings in. > + */ > + struct MigrationSrcPageRequest *mspr, *next_mspr; > + QSIMPLEQ_FOREACH_SAFE(mspr, &s->src_page_requests, next_req, next_mspr) { > + QSIMPLEQ_REMOVE_HEAD(&s->src_page_requests, next_req); > + g_free(mspr); > + } > + > if (s->file) { > trace_migrate_fd_cleanup(); > qemu_mutex_unlock_iothread(); > @@ -613,6 +624,9 @@ MigrationState *migrate_init(const MigrationParams *params) > s->state = MIG_STATE_SETUP; > trace_migrate_set_state(MIG_STATE_SETUP); > > + qemu_mutex_init(&s->src_page_req_mutex); > + QSIMPLEQ_INIT(&s->src_page_requests); > + > s->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); > return s; > } > @@ -826,7 +840,24 @@ static void source_return_path_bad(MigrationState *s) > static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname, > ram_addr_t start, ram_addr_t len) > { > - trace_migrate_handle_rp_req_pages(start, len); > + trace_migrate_handle_rp_req_pages(rbname, start, len); > + > + /* Round everything up to our host page size */ > + long our_host_ps = getpagesize(); > + if (start & (our_host_ps-1)) { > + long roundings = start & (our_host_ps-1); > + start -= roundings; > + len += roundings; > + } > + if (len & (our_host_ps-1)) { > + long roundings = len & (our_host_ps-1); > + len -= roundings; > + len += our_host_ps; > + } Why is it necessary to round out to host page size on the source? I understand why the host page size is relevant on the destination, due to the userfaultfd and atomic populate constraints, but not on the source. > + if (ram_save_queue_pages(ms, rbname, start, len)) { > + source_return_path_bad(ms); > + } > } > > /* > diff --git a/trace-events b/trace-events > index 9bedee4..8a0d70d 100644 > --- a/trace-events > +++ b/trace-events > @@ -1218,6 +1218,7 @@ migration_bitmap_sync_start(void) "" > migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64"" > migration_throttle(void) "" > ram_postcopy_send_discard_bitmap(void) "" > +ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: %zx len: %zx" > > # hw/display/qxl.c > disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d" > @@ -1404,7 +1405,7 @@ migrate_fd_error(void) "" > migrate_fd_cancel(void) "" > migrate_pending(uint64_t size, uint64_t max, uint64_t post, uint64_t nonpost) "pending size %" PRIu64 " max %" PRIu64 " (post=%" PRIu64 " nonpost=%" PRIu64 ")" > migrate_send_rp_message(int cmd, uint16_t len) "cmd=%d, len=%d" > -migrate_handle_rp_req_pages(size_t start, size_t len) "at %zx for len %zx" > +migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len) "in %s at %zx len %zx" > migration_thread_after_loop(void) "" > migration_thread_file_err(void) "" > migration_thread_setup_complete(void) ""
* David Gibson (david@gibson.dropbear.id.au) wrote: > On Wed, Feb 25, 2015 at 04:51:56PM +0000, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > On receiving MIG_RPCOMM_REQ_PAGES look up the address and > > queue the page. > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > arch_init.c | 55 +++++++++++++++++++++++++++++++++++++++++++ > > include/exec/cpu-all.h | 2 -- > > include/migration/migration.h | 21 +++++++++++++++++ > > include/qemu/typedefs.h | 1 + > > migration/migration.c | 33 +++++++++++++++++++++++++- > > trace-events | 3 ++- > > 6 files changed, 111 insertions(+), 4 deletions(-) > > > > diff --git a/arch_init.c b/arch_init.c > > index d2c4457..9d8fc6b 100644 > > --- a/arch_init.c > > +++ b/arch_init.c <snip> > > static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname, > > ram_addr_t start, ram_addr_t len) > > { > > - trace_migrate_handle_rp_req_pages(start, len); > > + trace_migrate_handle_rp_req_pages(rbname, start, len); > > + > > + /* Round everything up to our host page size */ > > + long our_host_ps = getpagesize(); > > + if (start & (our_host_ps-1)) { > > + long roundings = start & (our_host_ps-1); > > + start -= roundings; > > + len += roundings; > > + } > > + if (len & (our_host_ps-1)) { > > + long roundings = len & (our_host_ps-1); > > + len -= roundings; > > + len += our_host_ps; > > + } > > Why is it necessary to round out to host page size on the source? I > understand why the host page size is relevant on the destination, due > to the userfaultfd and atomic populate constraints, but not on the source. In principal the request you get from the destination should already be nicely aligned; but of course you can't actually trust it, so you have to at least test for alignment. Since the code has to send whole host pages to keep the destination happy, it expects the requests that come out of the queue to be host page aligned. At the moment we're only supporting matching page sizes, if we wanted to support mismatches then it probably needs to round to the size of destination host page sizes. Dave > > + if (ram_save_queue_pages(ms, rbname, start, len)) { > > + source_return_path_bad(ms); > > + } > > } > > > > /* > > diff --git a/trace-events b/trace-events > > index 9bedee4..8a0d70d 100644 > > --- a/trace-events > > +++ b/trace-events > > @@ -1218,6 +1218,7 @@ migration_bitmap_sync_start(void) "" > > migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64"" > > migration_throttle(void) "" > > ram_postcopy_send_discard_bitmap(void) "" > > +ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: %zx len: %zx" > > > > # hw/display/qxl.c > > disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d" > > @@ -1404,7 +1405,7 @@ migrate_fd_error(void) "" > > migrate_fd_cancel(void) "" > > migrate_pending(uint64_t size, uint64_t max, uint64_t post, uint64_t nonpost) "pending size %" PRIu64 " max %" PRIu64 " (post=%" PRIu64 " nonpost=%" PRIu64 ")" > > migrate_send_rp_message(int cmd, uint16_t len) "cmd=%d, len=%d" > > -migrate_handle_rp_req_pages(size_t start, size_t len) "at %zx for len %zx" > > +migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len) "in %s at %zx len %zx" > > migration_thread_after_loop(void) "" > > migration_thread_file_err(void) "" > > migration_thread_setup_complete(void) "" > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Wed, Mar 25, 2015 at 05:37:34PM +0000, Dr. David Alan Gilbert wrote: > * David Gibson (david@gibson.dropbear.id.au) wrote: > > On Wed, Feb 25, 2015 at 04:51:56PM +0000, Dr. David Alan Gilbert (git) wrote: > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > > > On receiving MIG_RPCOMM_REQ_PAGES look up the address and > > > queue the page. > > > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > --- > > > arch_init.c | 55 +++++++++++++++++++++++++++++++++++++++++++ > > > include/exec/cpu-all.h | 2 -- > > > include/migration/migration.h | 21 +++++++++++++++++ > > > include/qemu/typedefs.h | 1 + > > > migration/migration.c | 33 +++++++++++++++++++++++++- > > > trace-events | 3 ++- > > > 6 files changed, 111 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch_init.c b/arch_init.c > > > index d2c4457..9d8fc6b 100644 > > > --- a/arch_init.c > > > +++ b/arch_init.c > > <snip> > > > > static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname, > > > ram_addr_t start, ram_addr_t len) > > > { > > > - trace_migrate_handle_rp_req_pages(start, len); > > > + trace_migrate_handle_rp_req_pages(rbname, start, len); > > > + > > > + /* Round everything up to our host page size */ > > > + long our_host_ps = getpagesize(); > > > + if (start & (our_host_ps-1)) { > > > + long roundings = start & (our_host_ps-1); > > > + start -= roundings; > > > + len += roundings; > > > + } > > > + if (len & (our_host_ps-1)) { > > > + long roundings = len & (our_host_ps-1); > > > + len -= roundings; > > > + len += our_host_ps; > > > + } > > > > Why is it necessary to round out to host page size on the source? I > > understand why the host page size is relevant on the destination, due > > to the userfaultfd and atomic populate constraints, but not on the source. > > In principal the request you get from the destination should already > be nicely aligned; but of course you can't actually trust it, so you > have to at least test for alignment. > > Since the code has to send whole host pages to keep the > destination happy, it expects the requests that come out of the queue > to be host page aligned. I don't follow. It sounds like you'll only send non-aligned things if the destination (incorrectly) requests them. But in that case the only thing that the destination will mess up is itself, so where's the requirement to do anything on the source side? > At the moment we're only supporting matching page sizes, if we wanted > to support mismatches then it probably needs to round to the size of > destination host page sizes. And can't that effectively be done by just answering the requests exactly as the destination makes them?
diff --git a/arch_init.c b/arch_init.c index d2c4457..9d8fc6b 100644 --- a/arch_init.c +++ b/arch_init.c @@ -669,6 +669,61 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset, } /* + * Queue the pages for transmission, e.g. a request from postcopy destination + * ms: MigrationStatus in which the queue is held + * rbname: The RAMBlock the request is for - may be NULL (to mean reuse last) + * start: Offset from the start of the RAMBlock + * len: Length (in bytes) to send + * Return: 0 on success + */ +int ram_save_queue_pages(MigrationState *ms, const char *rbname, + ram_addr_t start, ram_addr_t len) +{ + RAMBlock *ramblock; + + if (!rbname) { + /* Reuse last RAMBlock */ + ramblock = ms->last_req_rb; + + if (!ramblock) { + /* + * Shouldn't happen, we can't reuse the last RAMBlock if + * it's the 1st request. + */ + error_report("ram_save_queue_pages no previous block"); + return -1; + } + } else { + ramblock = ram_find_block(rbname); + + if (!ramblock) { + /* We shouldn't be asked for a non-existent RAMBlock */ + error_report("ram_save_queue_pages no block '%s'", rbname); + return -1; + } + } + trace_ram_save_queue_pages(ramblock->idstr, start, len); + if (start+len > ramblock->used_length) { + error_report("%s request overrun start=%zx len=%zx blocklen=%zx", + __func__, start, len, ramblock->used_length); + return -1; + } + + struct MigrationSrcPageRequest *new_entry = + g_malloc0(sizeof(struct MigrationSrcPageRequest)); + new_entry->rb = ramblock; + new_entry->offset = start; + new_entry->len = len; + ms->last_req_rb = ramblock; + + qemu_mutex_lock(&ms->src_page_req_mutex); + QSIMPLEQ_INSERT_TAIL(&ms->src_page_requests, new_entry, next_req); + qemu_mutex_unlock(&ms->src_page_req_mutex); + + return 0; +} + +/* * ram_find_and_save_block: Finds a page to send and sends it to f * * Returns: The number of bytes written. diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index 2c48286..3088000 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -265,8 +265,6 @@ CPUArchState *cpu_copy(CPUArchState *env); /* memory API */ -typedef struct RAMBlock RAMBlock; - struct RAMBlock { struct MemoryRegion *mr; uint8_t *host; diff --git a/include/migration/migration.h b/include/migration/migration.h index 2c15d63..b1c7cad 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -100,6 +100,18 @@ MigrationIncomingState *migration_incoming_get_current(void); MigrationIncomingState *migration_incoming_state_new(QEMUFile *f); void migration_incoming_state_destroy(void); +/* + * An outstanding page request, on the source, having been received + * and queued + */ +struct MigrationSrcPageRequest { + RAMBlock *rb; + hwaddr offset; + hwaddr len; + + QSIMPLEQ_ENTRY(MigrationSrcPageRequest) next_req; +}; + struct MigrationState { int64_t bandwidth_limit; @@ -142,6 +154,12 @@ struct MigrationState * of the postcopy phase */ unsigned long *sentmap; + + /* Queue of outstanding page requests from the destination */ + QemuMutex src_page_req_mutex; + QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_requests; + /* The RAMBlock used in the last src_page_request */ + RAMBlock *last_req_rb; }; void process_incoming_migration(QEMUFile *f); @@ -276,6 +294,9 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, ram_addr_t offset, size_t size, int *bytes_sent); +int ram_save_queue_pages(MigrationState *ms, const char *rbname, + ram_addr_t start, ram_addr_t len); + PostcopyState postcopy_state_get(MigrationIncomingState *mis); /* Set the state and return the old state */ diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index 0651275..396044d 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -73,6 +73,7 @@ typedef struct QEMUSGList QEMUSGList; typedef struct QEMUSizedBuffer QEMUSizedBuffer; typedef struct QEMUTimerListGroup QEMUTimerListGroup; typedef struct QEMUTimer QEMUTimer; +typedef struct RAMBlock RAMBlock; typedef struct Range Range; typedef struct SerialState SerialState; typedef struct SHPCDevice SHPCDevice; diff --git a/migration/migration.c b/migration/migration.c index 2e9d0dd..939f426 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -26,6 +26,8 @@ #include "qemu/thread.h" #include "qmp-commands.h" #include "trace.h" +#include "exec/memory.h" +#include "exec/address-spaces.h" enum MigrationPhase { MIG_STATE_ERROR = -1, @@ -495,6 +497,15 @@ static void migrate_fd_cleanup(void *opaque) migrate_fd_cleanup_src_rp(s); + /* This queue generally should be empty - but in the case of a failed + * migration might have some droppings in. + */ + struct MigrationSrcPageRequest *mspr, *next_mspr; + QSIMPLEQ_FOREACH_SAFE(mspr, &s->src_page_requests, next_req, next_mspr) { + QSIMPLEQ_REMOVE_HEAD(&s->src_page_requests, next_req); + g_free(mspr); + } + if (s->file) { trace_migrate_fd_cleanup(); qemu_mutex_unlock_iothread(); @@ -613,6 +624,9 @@ MigrationState *migrate_init(const MigrationParams *params) s->state = MIG_STATE_SETUP; trace_migrate_set_state(MIG_STATE_SETUP); + qemu_mutex_init(&s->src_page_req_mutex); + QSIMPLEQ_INIT(&s->src_page_requests); + s->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); return s; } @@ -826,7 +840,24 @@ static void source_return_path_bad(MigrationState *s) static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname, ram_addr_t start, ram_addr_t len) { - trace_migrate_handle_rp_req_pages(start, len); + trace_migrate_handle_rp_req_pages(rbname, start, len); + + /* Round everything up to our host page size */ + long our_host_ps = getpagesize(); + if (start & (our_host_ps-1)) { + long roundings = start & (our_host_ps-1); + start -= roundings; + len += roundings; + } + if (len & (our_host_ps-1)) { + long roundings = len & (our_host_ps-1); + len -= roundings; + len += our_host_ps; + } + + if (ram_save_queue_pages(ms, rbname, start, len)) { + source_return_path_bad(ms); + } } /* diff --git a/trace-events b/trace-events index 9bedee4..8a0d70d 100644 --- a/trace-events +++ b/trace-events @@ -1218,6 +1218,7 @@ migration_bitmap_sync_start(void) "" migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64"" migration_throttle(void) "" ram_postcopy_send_discard_bitmap(void) "" +ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: %zx len: %zx" # hw/display/qxl.c disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d" @@ -1404,7 +1405,7 @@ migrate_fd_error(void) "" migrate_fd_cancel(void) "" migrate_pending(uint64_t size, uint64_t max, uint64_t post, uint64_t nonpost) "pending size %" PRIu64 " max %" PRIu64 " (post=%" PRIu64 " nonpost=%" PRIu64 ")" migrate_send_rp_message(int cmd, uint16_t len) "cmd=%d, len=%d" -migrate_handle_rp_req_pages(size_t start, size_t len) "at %zx for len %zx" +migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len) "in %s at %zx len %zx" migration_thread_after_loop(void) "" migration_thread_file_err(void) "" migration_thread_setup_complete(void) ""