* [PATCH v2 1/8] migration: Drop dead code of ram_debug_dump_bitmap()
2021-12-20 8:53 [PATCH v2 0/8] migration: Postcopy cleanup on ram disgard Peter Xu
@ 2021-12-20 8:53 ` Peter Xu
2021-12-20 8:53 ` [PATCH v2 2/8] migration: Don't return for postcopy_chunk_hostpages() Peter Xu
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2021-12-20 8:53 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr . David Alan Gilbert, peterx,
Leonardo Bras Soares Passos
I planned to add "#ifdef DEBUG_POSTCOPY" around the function too because
otherwise it'll be compiled into qemu binary even if it'll never be used. Then
I found that maybe it's easier to just drop it for good..
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 39 ---------------------------------------
migration/ram.h | 2 --
2 files changed, 41 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 57efa67f20..7d9c8a508b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2408,40 +2408,6 @@ static void ram_state_reset(RAMState *rs)
#define MAX_WAIT 50 /* ms, half buffered_file limit */
-/*
- * 'expected' is the value you expect the bitmap mostly to be full
- * of; it won't bother printing lines that are all this value.
- * If 'todump' is null the migration bitmap is dumped.
- */
-void ram_debug_dump_bitmap(unsigned long *todump, bool expected,
- unsigned long pages)
-{
- int64_t cur;
- int64_t linelen = 128;
- char linebuf[129];
-
- for (cur = 0; cur < pages; cur += linelen) {
- int64_t curb;
- bool found = false;
- /*
- * Last line; catch the case where the line length
- * is longer than remaining ram
- */
- if (cur + linelen > pages) {
- linelen = pages - cur;
- }
- for (curb = 0; curb < linelen; curb++) {
- bool thisbit = test_bit(cur + curb, todump);
- linebuf[curb] = thisbit ? '1' : '.';
- found = found || (thisbit != expected);
- }
- if (found) {
- linebuf[curb] = '\0';
- fprintf(stderr, "0x%08" PRIx64 " : %s\n", cur, linebuf);
- }
- }
-}
-
/* **** functions for postcopy ***** */
void ram_postcopy_migrated_memory_release(MigrationState *ms)
@@ -2669,11 +2635,6 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
if (ret) {
return ret;
}
-
-#ifdef DEBUG_POSTCOPY
- ram_debug_dump_bitmap(block->bmap, true,
- block->used_length >> TARGET_PAGE_BITS);
-#endif
}
trace_ram_postcopy_send_discard_bitmap();
diff --git a/migration/ram.h b/migration/ram.h
index c515396a9a..f543e25765 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -55,8 +55,6 @@ void mig_throttle_counter_reset(void);
uint64_t ram_pagesize_summary(void);
int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len);
void acct_update_position(QEMUFile *f, size_t size, bool zero);
-void ram_debug_dump_bitmap(unsigned long *todump, bool expected,
- unsigned long pages);
void ram_postcopy_migrated_memory_release(MigrationState *ms);
/* For outgoing discard bitmap */
int ram_postcopy_send_discard_bitmap(MigrationState *ms);
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/8] migration: Don't return for postcopy_chunk_hostpages()
2021-12-20 8:53 [PATCH v2 0/8] migration: Postcopy cleanup on ram disgard Peter Xu
2021-12-20 8:53 ` [PATCH v2 1/8] migration: Drop dead code of ram_debug_dump_bitmap() Peter Xu
@ 2021-12-20 8:53 ` Peter Xu
2021-12-20 8:53 ` [PATCH v2 3/8] migration: Drop postcopy_chunk_hostpages() Peter Xu
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2021-12-20 8:53 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr . David Alan Gilbert, peterx,
Leonardo Bras Soares Passos
It always return zero, because it just can't go wrong so far. Simplify the
code with no functional change.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 7d9c8a508b..0ed0f51a09 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2580,12 +2580,10 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block)
* dirty host-page size chunks as all dirty. In this case the host-page
* is the host-page for the particular RAMBlock, i.e. it might be a huge page
*
- * Returns zero on success
- *
* @ms: current migration state
* @block: block we want to work with
*/
-static int postcopy_chunk_hostpages(MigrationState *ms, RAMBlock *block)
+static void postcopy_chunk_hostpages(MigrationState *ms, RAMBlock *block)
{
postcopy_discard_send_init(ms, block->idstr);
@@ -2595,7 +2593,6 @@ static int postcopy_chunk_hostpages(MigrationState *ms, RAMBlock *block)
postcopy_chunk_hostpages_pass(ms, block);
postcopy_discard_send_finish(ms);
- return 0;
}
/**
@@ -2617,7 +2614,6 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
{
RAMState *rs = ram_state;
RAMBlock *block;
- int ret;
RCU_READ_LOCK_GUARD();
@@ -2631,10 +2627,7 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
RAMBLOCK_FOREACH_NOT_IGNORED(block) {
/* Deal with TPS != HPS and huge pages */
- ret = postcopy_chunk_hostpages(ms, block);
- if (ret) {
- return ret;
- }
+ postcopy_chunk_hostpages(ms, block);
}
trace_ram_postcopy_send_discard_bitmap();
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/8] migration: Drop postcopy_chunk_hostpages()
2021-12-20 8:53 [PATCH v2 0/8] migration: Postcopy cleanup on ram disgard Peter Xu
2021-12-20 8:53 ` [PATCH v2 1/8] migration: Drop dead code of ram_debug_dump_bitmap() Peter Xu
2021-12-20 8:53 ` [PATCH v2 2/8] migration: Don't return for postcopy_chunk_hostpages() Peter Xu
@ 2021-12-20 8:53 ` Peter Xu
2021-12-20 8:53 ` [PATCH v2 4/8] migration: Do chunk page in postcopy_each_ram_send_discard() Peter Xu
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2021-12-20 8:53 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr . David Alan Gilbert, peterx,
Leonardo Bras Soares Passos
This function calls three functions:
- postcopy_discard_send_init(ms, block->idstr);
- postcopy_chunk_hostpages_pass(ms, block);
- postcopy_discard_send_finish(ms);
However only the 2nd function call is meaningful. It's major role is to make
sure dirty bits are applied in host-page-size granule, so there will be no
partial dirty bits set for a whole host page if huge pages are used.
The 1st/3rd call are for latter when we want to send the disgard ranges.
They're mostly no-op here besides some tracepoints (which are misleading!).
Drop them, then we can directly drop postcopy_chunk_hostpages() as a whole
because we can call postcopy_chunk_hostpages_pass() directly.
There're still some nice comments above postcopy_chunk_hostpages() that explain
what it does. Copy it over to the caller's site.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 33 +++++++--------------------------
1 file changed, 7 insertions(+), 26 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 0ed0f51a09..b22c9e7432 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2571,30 +2571,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block)
}
}
-/**
- * postcopy_chunk_hostpages: discard any partially sent host page
- *
- * Utility for the outgoing postcopy code.
- *
- * Discard any partially sent host-page size chunks, mark any partially
- * dirty host-page size chunks as all dirty. In this case the host-page
- * is the host-page for the particular RAMBlock, i.e. it might be a huge page
- *
- * @ms: current migration state
- * @block: block we want to work with
- */
-static void postcopy_chunk_hostpages(MigrationState *ms, RAMBlock *block)
-{
- postcopy_discard_send_init(ms, block->idstr);
-
- /*
- * Ensure that all partially dirty host pages are made fully dirty.
- */
- postcopy_chunk_hostpages_pass(ms, block);
-
- postcopy_discard_send_finish(ms);
-}
-
/**
* ram_postcopy_send_discard_bitmap: transmit the discard bitmap
*
@@ -2626,8 +2602,13 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
rs->last_page = 0;
RAMBLOCK_FOREACH_NOT_IGNORED(block) {
- /* Deal with TPS != HPS and huge pages */
- postcopy_chunk_hostpages(ms, block);
+ /*
+ * Deal with TPS != HPS and huge pages. It discard any partially sent
+ * host-page size chunks, mark any partially dirty host-page size
+ * chunks as all dirty. In this case the host-page is the host-page
+ * for the particular RAMBlock, i.e. it might be a huge page.
+ */
+ postcopy_chunk_hostpages_pass(ms, block);
}
trace_ram_postcopy_send_discard_bitmap();
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/8] migration: Do chunk page in postcopy_each_ram_send_discard()
2021-12-20 8:53 [PATCH v2 0/8] migration: Postcopy cleanup on ram disgard Peter Xu
` (2 preceding siblings ...)
2021-12-20 8:53 ` [PATCH v2 3/8] migration: Drop postcopy_chunk_hostpages() Peter Xu
@ 2021-12-20 8:53 ` Peter Xu
2021-12-20 8:53 ` [PATCH v2 5/8] migration: Drop return code for disgard ram process Peter Xu
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2021-12-20 8:53 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr . David Alan Gilbert, peterx,
Leonardo Bras Soares Passos
Right now we loop ramblocks for twice, the 1st time chunk the dirty bits with
huge page information; the 2nd time we send the discard ranges. That's not
necessary - we can do them in a single loop.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index b22c9e7432..e7107b9790 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2468,6 +2468,8 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block)
return 0;
}
+static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block);
+
/**
* postcopy_each_ram_send_discard: discard all RAMBlocks
*
@@ -2489,6 +2491,14 @@ static int postcopy_each_ram_send_discard(MigrationState *ms)
RAMBLOCK_FOREACH_NOT_IGNORED(block) {
postcopy_discard_send_init(ms, block->idstr);
+ /*
+ * Deal with TPS != HPS and huge pages. It discard any partially sent
+ * host-page size chunks, mark any partially dirty host-page size
+ * chunks as all dirty. In this case the host-page is the host-page
+ * for the particular RAMBlock, i.e. it might be a huge page.
+ */
+ postcopy_chunk_hostpages_pass(ms, block);
+
/*
* Postcopy sends chunks of bitmap over the wire, but it
* just needs indexes at this point, avoids it having
@@ -2589,7 +2599,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block)
int ram_postcopy_send_discard_bitmap(MigrationState *ms)
{
RAMState *rs = ram_state;
- RAMBlock *block;
RCU_READ_LOCK_GUARD();
@@ -2601,15 +2610,6 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
rs->last_sent_block = NULL;
rs->last_page = 0;
- RAMBLOCK_FOREACH_NOT_IGNORED(block) {
- /*
- * Deal with TPS != HPS and huge pages. It discard any partially sent
- * host-page size chunks, mark any partially dirty host-page size
- * chunks as all dirty. In this case the host-page is the host-page
- * for the particular RAMBlock, i.e. it might be a huge page.
- */
- postcopy_chunk_hostpages_pass(ms, block);
- }
trace_ram_postcopy_send_discard_bitmap();
return postcopy_each_ram_send_discard(ms);
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/8] migration: Drop return code for disgard ram process
2021-12-20 8:53 [PATCH v2 0/8] migration: Postcopy cleanup on ram disgard Peter Xu
` (3 preceding siblings ...)
2021-12-20 8:53 ` [PATCH v2 4/8] migration: Do chunk page in postcopy_each_ram_send_discard() Peter Xu
@ 2021-12-20 8:53 ` Peter Xu
2021-12-20 8:53 ` [PATCH v2 6/8] migration: Dump sub-cmd name in loadvm_process_command tp Peter Xu
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2021-12-20 8:53 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr . David Alan Gilbert, peterx,
Leonardo Bras Soares Passos
It will just never fail. Drop those return values where they're constantly
zeros.
A tiny touch-up on the tracepoint so trace_ram_postcopy_send_discard_bitmap()
is called after the logic itself (which sounds more reasonable).
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 5 +----
migration/ram.c | 20 +++++---------------
migration/ram.h | 2 +-
3 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 3de11ae921..beeb162a1a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2990,10 +2990,7 @@ static int postcopy_start(MigrationState *ms)
* that are dirty
*/
if (migrate_postcopy_ram()) {
- if (ram_postcopy_send_discard_bitmap(ms)) {
- error_report("postcopy send discard bitmap failed");
- goto fail;
- }
+ ram_postcopy_send_discard_bitmap(ms);
}
/*
diff --git a/migration/ram.c b/migration/ram.c
index e7107b9790..5234d1ece1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2473,8 +2473,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block);
/**
* postcopy_each_ram_send_discard: discard all RAMBlocks
*
- * Returns 0 for success or negative for error
- *
* Utility for the outgoing postcopy code.
* Calls postcopy_send_discard_bm_ram for each RAMBlock
* passing it bitmap indexes and name.
@@ -2483,10 +2481,9 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block);
*
* @ms: current migration state
*/
-static int postcopy_each_ram_send_discard(MigrationState *ms)
+static void postcopy_each_ram_send_discard(MigrationState *ms)
{
struct RAMBlock *block;
- int ret;
RAMBLOCK_FOREACH_NOT_IGNORED(block) {
postcopy_discard_send_init(ms, block->idstr);
@@ -2504,14 +2501,9 @@ static int postcopy_each_ram_send_discard(MigrationState *ms)
* just needs indexes at this point, avoids it having
* target page specific code.
*/
- ret = postcopy_send_discard_bm_ram(ms, block);
+ postcopy_send_discard_bm_ram(ms, block);
postcopy_discard_send_finish(ms);
- if (ret) {
- return ret;
- }
}
-
- return 0;
}
/**
@@ -2584,8 +2576,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block)
/**
* ram_postcopy_send_discard_bitmap: transmit the discard bitmap
*
- * Returns zero on success
- *
* Transmit the set of pages to be discarded after precopy to the target
* these are pages that:
* a) Have been previously transmitted but are now dirty again
@@ -2596,7 +2586,7 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block)
*
* @ms: current migration state
*/
-int ram_postcopy_send_discard_bitmap(MigrationState *ms)
+void ram_postcopy_send_discard_bitmap(MigrationState *ms)
{
RAMState *rs = ram_state;
@@ -2610,9 +2600,9 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
rs->last_sent_block = NULL;
rs->last_page = 0;
- trace_ram_postcopy_send_discard_bitmap();
+ postcopy_each_ram_send_discard(ms);
- return postcopy_each_ram_send_discard(ms);
+ trace_ram_postcopy_send_discard_bitmap();
}
/**
diff --git a/migration/ram.h b/migration/ram.h
index f543e25765..2c6dc3675d 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -57,7 +57,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len);
void acct_update_position(QEMUFile *f, size_t size, bool zero);
void ram_postcopy_migrated_memory_release(MigrationState *ms);
/* For outgoing discard bitmap */
-int ram_postcopy_send_discard_bitmap(MigrationState *ms);
+void ram_postcopy_send_discard_bitmap(MigrationState *ms);
/* For incoming postcopy discard */
int ram_discard_range(const char *block_name, uint64_t start, size_t length);
int ram_postcopy_incoming_init(MigrationIncomingState *mis);
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 6/8] migration: Dump sub-cmd name in loadvm_process_command tp
2021-12-20 8:53 [PATCH v2 0/8] migration: Postcopy cleanup on ram disgard Peter Xu
` (4 preceding siblings ...)
2021-12-20 8:53 ` [PATCH v2 5/8] migration: Drop return code for disgard ram process Peter Xu
@ 2021-12-20 8:53 ` Peter Xu
2021-12-21 10:08 ` David Edmondson
2021-12-20 8:53 ` [PATCH v2 7/8] migration: Finer grained tracepoints for POSTCOPY_LISTEN Peter Xu
2021-12-20 8:53 ` [PATCH v2 8/8] migration: Tracepoint change in postcopy-run bottom half Peter Xu
7 siblings, 1 reply; 13+ messages in thread
From: Peter Xu @ 2021-12-20 8:53 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr . David Alan Gilbert, peterx,
Leonardo Bras Soares Passos
It'll be easier to read the name rather than index of sub-cmd when debugging.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/savevm.c | 3 ++-
migration/trace-events | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 0bef031acb..7f7af6f750 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2272,12 +2272,13 @@ static int loadvm_process_command(QEMUFile *f)
return qemu_file_get_error(f);
}
- trace_loadvm_process_command(cmd, len);
if (cmd >= MIG_CMD_MAX || cmd == MIG_CMD_INVALID) {
error_report("MIG_CMD 0x%x unknown (len 0x%x)", cmd, len);
return -EINVAL;
}
+ trace_loadvm_process_command(mig_cmd_args[cmd].name, len);
+
if (mig_cmd_args[cmd].len != -1 && mig_cmd_args[cmd].len != len) {
error_report("%s received with bad length - expecting %zu, got %d",
mig_cmd_args[cmd].name,
diff --git a/migration/trace-events b/migration/trace-events
index b48d873b8a..d63a5915f5 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -22,7 +22,7 @@ loadvm_postcopy_handle_resume(void) ""
loadvm_postcopy_ram_handle_discard(void) ""
loadvm_postcopy_ram_handle_discard_end(void) ""
loadvm_postcopy_ram_handle_discard_header(const char *ramid, uint16_t len) "%s: %ud"
-loadvm_process_command(uint16_t com, uint16_t len) "com=0x%x len=%d"
+loadvm_process_command(const char *s, uint16_t len) "com=%s len=%d"
loadvm_process_command_ping(uint32_t val) "0x%x"
postcopy_ram_listen_thread_exit(void) ""
postcopy_ram_listen_thread_start(void) ""
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/8] migration: Dump sub-cmd name in loadvm_process_command tp
2021-12-20 8:53 ` [PATCH v2 6/8] migration: Dump sub-cmd name in loadvm_process_command tp Peter Xu
@ 2021-12-21 10:08 ` David Edmondson
2021-12-21 12:59 ` Peter Xu
0 siblings, 1 reply; 13+ messages in thread
From: David Edmondson @ 2021-12-21 10:08 UTC (permalink / raw)
To: qemu-devel
On Monday, 2021-12-20 at 16:53:53 +08, Peter Xu wrote:
> It'll be easier to read the name rather than index of sub-cmd when debugging.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/savevm.c | 3 ++-
> migration/trace-events | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 0bef031acb..7f7af6f750 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2272,12 +2272,13 @@ static int loadvm_process_command(QEMUFile *f)
> return qemu_file_get_error(f);
> }
>
> - trace_loadvm_process_command(cmd, len);
> if (cmd >= MIG_CMD_MAX || cmd == MIG_CMD_INVALID) {
> error_report("MIG_CMD 0x%x unknown (len 0x%x)", cmd, len);
> return -EINVAL;
> }
>
> + trace_loadvm_process_command(mig_cmd_args[cmd].name, len);
> +
> if (mig_cmd_args[cmd].len != -1 && mig_cmd_args[cmd].len != len) {
> error_report("%s received with bad length - expecting %zu, got %d",
> mig_cmd_args[cmd].name,
> diff --git a/migration/trace-events b/migration/trace-events
> index b48d873b8a..d63a5915f5 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -22,7 +22,7 @@ loadvm_postcopy_handle_resume(void) ""
> loadvm_postcopy_ram_handle_discard(void) ""
> loadvm_postcopy_ram_handle_discard_end(void) ""
> loadvm_postcopy_ram_handle_discard_header(const char *ramid, uint16_t len) "%s: %ud"
> -loadvm_process_command(uint16_t com, uint16_t len) "com=0x%x len=%d"
> +loadvm_process_command(const char *s, uint16_t len) "com=%s len=%d"
"cmd" rather than "com", to match the code.
> loadvm_process_command_ping(uint32_t val) "0x%x"
> postcopy_ram_listen_thread_exit(void) ""
> postcopy_ram_listen_thread_start(void) ""
dme.
--
I went starin' out of my window, been caught doin' it once or twice.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/8] migration: Dump sub-cmd name in loadvm_process_command tp
2021-12-21 10:08 ` David Edmondson
@ 2021-12-21 12:59 ` Peter Xu
0 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2021-12-21 12:59 UTC (permalink / raw)
To: David Edmondson; +Cc: qemu-devel
On Tue, Dec 21, 2021 at 10:08:24AM +0000, David Edmondson wrote:
> > @@ -22,7 +22,7 @@ loadvm_postcopy_handle_resume(void) ""
> > loadvm_postcopy_ram_handle_discard(void) ""
> > loadvm_postcopy_ram_handle_discard_end(void) ""
> > loadvm_postcopy_ram_handle_discard_header(const char *ramid, uint16_t len) "%s: %ud"
> > -loadvm_process_command(uint16_t com, uint16_t len) "com=0x%x len=%d"
> > +loadvm_process_command(const char *s, uint16_t len) "com=%s len=%d"
>
> "cmd" rather than "com", to match the code.
"com" is what it was used before (perhaps "com"mand?). Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 7/8] migration: Finer grained tracepoints for POSTCOPY_LISTEN
2021-12-20 8:53 [PATCH v2 0/8] migration: Postcopy cleanup on ram disgard Peter Xu
` (5 preceding siblings ...)
2021-12-20 8:53 ` [PATCH v2 6/8] migration: Dump sub-cmd name in loadvm_process_command tp Peter Xu
@ 2021-12-20 8:53 ` Peter Xu
2021-12-21 10:12 ` David Edmondson
2021-12-20 8:53 ` [PATCH v2 8/8] migration: Tracepoint change in postcopy-run bottom half Peter Xu
7 siblings, 1 reply; 13+ messages in thread
From: Peter Xu @ 2021-12-20 8:53 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr . David Alan Gilbert, peterx,
Leonardo Bras Soares Passos
The enablement of postcopy listening has a few steps, add a few tracepoints to
be there ready for some basic measurements for them.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/savevm.c | 9 ++++++++-
migration/trace-events | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 7f7af6f750..25face6de0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1947,9 +1947,10 @@ static void *postcopy_ram_listen_thread(void *opaque)
static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
{
PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_LISTENING);
- trace_loadvm_postcopy_handle_listen();
Error *local_err = NULL;
+ trace_loadvm_postcopy_handle_listen("enter");
+
if (ps != POSTCOPY_INCOMING_ADVISE && ps != POSTCOPY_INCOMING_DISCARD) {
error_report("CMD_POSTCOPY_LISTEN in wrong postcopy state (%d)", ps);
return -1;
@@ -1964,6 +1965,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
}
}
+ trace_loadvm_postcopy_handle_listen("after disgard");
+
/*
* Sensitise RAM - can now generate requests for blocks that don't exist
* However, at this point the CPU shouldn't be running, and the IO
@@ -1976,6 +1979,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
}
}
+ trace_loadvm_postcopy_handle_listen("after uffd");
+
if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_LISTEN, &local_err)) {
error_report_err(local_err);
return -1;
@@ -1990,6 +1995,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
qemu_sem_wait(&mis->listen_thread_sem);
qemu_sem_destroy(&mis->listen_thread_sem);
+ trace_loadvm_postcopy_handle_listen("exit");
+
return 0;
}
diff --git a/migration/trace-events b/migration/trace-events
index d63a5915f5..77d1237d89 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -14,7 +14,7 @@ loadvm_handle_cmd_packaged_main(int ret) "%d"
loadvm_handle_cmd_packaged_received(int ret) "%d"
loadvm_handle_recv_bitmap(char *s) "%s"
loadvm_postcopy_handle_advise(void) ""
-loadvm_postcopy_handle_listen(void) ""
+loadvm_postcopy_handle_listen(const char *str) "%s"
loadvm_postcopy_handle_run(void) ""
loadvm_postcopy_handle_run_cpu_sync(void) ""
loadvm_postcopy_handle_run_vmstart(void) ""
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 7/8] migration: Finer grained tracepoints for POSTCOPY_LISTEN
2021-12-20 8:53 ` [PATCH v2 7/8] migration: Finer grained tracepoints for POSTCOPY_LISTEN Peter Xu
@ 2021-12-21 10:12 ` David Edmondson
2021-12-21 13:08 ` Peter Xu
0 siblings, 1 reply; 13+ messages in thread
From: David Edmondson @ 2021-12-21 10:12 UTC (permalink / raw)
To: qemu-devel
On Monday, 2021-12-20 at 16:53:54 +08, Peter Xu wrote:
> The enablement of postcopy listening has a few steps, add a few tracepoints to
> be there ready for some basic measurements for them.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/savevm.c | 9 ++++++++-
> migration/trace-events | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 7f7af6f750..25face6de0 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1947,9 +1947,10 @@ static void *postcopy_ram_listen_thread(void *opaque)
> static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> {
> PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_LISTENING);
> - trace_loadvm_postcopy_handle_listen();
> Error *local_err = NULL;
>
> + trace_loadvm_postcopy_handle_listen("enter");
> +
> if (ps != POSTCOPY_INCOMING_ADVISE && ps != POSTCOPY_INCOMING_DISCARD) {
> error_report("CMD_POSTCOPY_LISTEN in wrong postcopy state (%d)", ps);
> return -1;
> @@ -1964,6 +1965,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> }
> }
>
> + trace_loadvm_postcopy_handle_listen("after disgard");
s/disgard/discard/
> +
> /*
> * Sensitise RAM - can now generate requests for blocks that don't exist
> * However, at this point the CPU shouldn't be running, and the IO
> @@ -1976,6 +1979,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> }
> }
>
> + trace_loadvm_postcopy_handle_listen("after uffd");
> +
> if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_LISTEN, &local_err)) {
> error_report_err(local_err);
> return -1;
> @@ -1990,6 +1995,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> qemu_sem_wait(&mis->listen_thread_sem);
> qemu_sem_destroy(&mis->listen_thread_sem);
>
> + trace_loadvm_postcopy_handle_listen("exit");
> +
"return" rather than "exit"?
> return 0;
> }
>
> diff --git a/migration/trace-events b/migration/trace-events
> index d63a5915f5..77d1237d89 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -14,7 +14,7 @@ loadvm_handle_cmd_packaged_main(int ret) "%d"
> loadvm_handle_cmd_packaged_received(int ret) "%d"
> loadvm_handle_recv_bitmap(char *s) "%s"
> loadvm_postcopy_handle_advise(void) ""
> -loadvm_postcopy_handle_listen(void) ""
> +loadvm_postcopy_handle_listen(const char *str) "%s"
> loadvm_postcopy_handle_run(void) ""
> loadvm_postcopy_handle_run_cpu_sync(void) ""
> loadvm_postcopy_handle_run_vmstart(void) ""
dme.
--
When you were the brightest star, who were the shadows?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 7/8] migration: Finer grained tracepoints for POSTCOPY_LISTEN
2021-12-21 10:12 ` David Edmondson
@ 2021-12-21 13:08 ` Peter Xu
0 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2021-12-21 13:08 UTC (permalink / raw)
To: David Edmondson; +Cc: qemu-devel
On Tue, Dec 21, 2021 at 10:12:34AM +0000, David Edmondson wrote:
> On Monday, 2021-12-20 at 16:53:54 +08, Peter Xu wrote:
>
> > The enablement of postcopy listening has a few steps, add a few tracepoints to
> > be there ready for some basic measurements for them.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > migration/savevm.c | 9 ++++++++-
> > migration/trace-events | 2 +-
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 7f7af6f750..25face6de0 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -1947,9 +1947,10 @@ static void *postcopy_ram_listen_thread(void *opaque)
> > static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > {
> > PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_LISTENING);
> > - trace_loadvm_postcopy_handle_listen();
> > Error *local_err = NULL;
> >
> > + trace_loadvm_postcopy_handle_listen("enter");
> > +
> > if (ps != POSTCOPY_INCOMING_ADVISE && ps != POSTCOPY_INCOMING_DISCARD) {
> > error_report("CMD_POSTCOPY_LISTEN in wrong postcopy state (%d)", ps);
> > return -1;
> > @@ -1964,6 +1965,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > }
> > }
> >
> > + trace_loadvm_postcopy_handle_listen("after disgard");
>
> s/disgard/discard/
Will fix.
>
> > +
> > /*
> > * Sensitise RAM - can now generate requests for blocks that don't exist
> > * However, at this point the CPU shouldn't be running, and the IO
> > @@ -1976,6 +1979,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > }
> > }
> >
> > + trace_loadvm_postcopy_handle_listen("after uffd");
> > +
> > if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_LISTEN, &local_err)) {
> > error_report_err(local_err);
> > return -1;
> > @@ -1990,6 +1995,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > qemu_sem_wait(&mis->listen_thread_sem);
> > qemu_sem_destroy(&mis->listen_thread_sem);
> >
> > + trace_loadvm_postcopy_handle_listen("exit");
> > +
>
> "return" rather than "exit"?
I don't think it matters a lot, but sure.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 8/8] migration: Tracepoint change in postcopy-run bottom half
2021-12-20 8:53 [PATCH v2 0/8] migration: Postcopy cleanup on ram disgard Peter Xu
` (6 preceding siblings ...)
2021-12-20 8:53 ` [PATCH v2 7/8] migration: Finer grained tracepoints for POSTCOPY_LISTEN Peter Xu
@ 2021-12-20 8:53 ` Peter Xu
7 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2021-12-20 8:53 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr . David Alan Gilbert, peterx,
Leonardo Bras Soares Passos
Remove the old two tracepoints and they're even near each other:
trace_loadvm_postcopy_handle_run_cpu_sync()
trace_loadvm_postcopy_handle_run_vmstart()
Add trace_loadvm_postcopy_handle_run_bh() with a finer granule trace.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/savevm.c | 12 +++++++++---
migration/trace-events | 3 +--
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 25face6de0..de2577c251 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2005,13 +2005,19 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
Error *local_err = NULL;
MigrationIncomingState *mis = opaque;
+ trace_loadvm_postcopy_handle_run_bh("enter");
+
/* TODO we should move all of this lot into postcopy_ram.c or a shared code
* in migration.c
*/
cpu_synchronize_all_post_init();
+ trace_loadvm_postcopy_handle_run_bh("after cpu sync");
+
qemu_announce_self(&mis->announce_timer, migrate_announce_params());
+ trace_loadvm_postcopy_handle_run_bh("after announce");
+
/* Make sure all file formats flush their mutable metadata.
* If we get an error here, just don't restart the VM yet. */
bdrv_invalidate_cache_all(&local_err);
@@ -2021,9 +2027,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
autostart = false;
}
- trace_loadvm_postcopy_handle_run_cpu_sync();
-
- trace_loadvm_postcopy_handle_run_vmstart();
+ trace_loadvm_postcopy_handle_run_bh("after invalidate cache");
dirty_bitmap_mig_before_vm_start();
@@ -2036,6 +2040,8 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
}
qemu_bh_delete(mis->bh);
+
+ trace_loadvm_postcopy_handle_run_bh("exit");
}
/* After all discards we can start running and asking for pages */
diff --git a/migration/trace-events b/migration/trace-events
index 77d1237d89..e165687af2 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -16,8 +16,7 @@ loadvm_handle_recv_bitmap(char *s) "%s"
loadvm_postcopy_handle_advise(void) ""
loadvm_postcopy_handle_listen(const char *str) "%s"
loadvm_postcopy_handle_run(void) ""
-loadvm_postcopy_handle_run_cpu_sync(void) ""
-loadvm_postcopy_handle_run_vmstart(void) ""
+loadvm_postcopy_handle_run_bh(const char *str) "%s"
loadvm_postcopy_handle_resume(void) ""
loadvm_postcopy_ram_handle_discard(void) ""
loadvm_postcopy_ram_handle_discard_end(void) ""
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread