On Wed, Jul 18, 2018 at 06:41:53PM +0300, Denis Plotnikov wrote:
> The background snapshot uses memeory page copying to seal the page
> memory content. The patch adapts the migration infrastructure to save
> copies of the pages.
Again, since previous page only defined some fields that are firstly
used in this patch, I think you can squash that into this one.
>
> Signed-off-by: Denis Plotnikov <dplotni...@virtuozzo.com>
> ---
> migration/migration.c | 2 +-
> migration/ram.c | 59 ++++++++++++++++++++++++++++++++-----------
> migration/ram.h | 3 ++-
> 3 files changed, 47 insertions(+), 17 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 87096d23ef..131d0904e4 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1716,7 +1716,7 @@ static void migrate_handle_rp_req_pages(MigrationState
> *ms, const char* rbname,
> return;
> }
>
> - if (ram_save_queue_pages(rbname, start, len)) {
> + if (ram_save_queue_pages(NULL, rbname, start, len, NULL)) {
> mark_source_rp_bad(ms);
> }
> }
> diff --git a/migration/ram.c b/migration/ram.c
> index dc7dfe0726..3c4ccd85b4 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -976,7 +976,12 @@ static int ram_save_page(RAMState *rs, PageSearchStatus
> *pss, bool last_stage)
> RAMBlock *block = pss->block;
> ram_addr_t offset = pss->page << TARGET_PAGE_BITS;
>
> - p = block->host + offset;
> + if (pss->page_copy) {
> + p = pss->page_copy;
> + } else {
> + p = block->host + offset;
> + }
> +
> trace_ram_save_page(block->idstr, (uint64_t)offset, p);
>
> /* In doubt sent page as normal */
> @@ -1003,13 +1008,18 @@ static int ram_save_page(RAMState *rs,
> PageSearchStatus *pss, bool last_stage)
> } else {
> pages = save_zero_page(rs, block, offset, p);
> if (pages > 0) {
> - /* Must let xbzrle know, otherwise a previous (now 0'd) cached
> - * page would be stale
> - */
> - xbzrle_cache_zero_page(rs, current_addr);
> - ram_release_pages(block->idstr, offset, pages);
> + if (pss->page_copy) {
> + qemu_madvise(p, TARGET_PAGE_SIZE, MADV_DONTNEED);
> + } else {
> + /* Must let xbzrle know, otherwise a previous (now 0'd)
> cached
> + * page would be stale
> + */
> + xbzrle_cache_zero_page(rs, current_addr);
> + ram_release_pages(block->idstr, offset, pages);
> + }
> } else if (!rs->ram_bulk_stage &&
> - !migration_in_postcopy() && migrate_use_xbzrle()) {
> + !migration_in_postcopy() && migrate_use_xbzrle() &&
> + !migrate_background_snapshot()) {
> pages = save_xbzrle_page(rs, &p, current_addr, block,
> offset, last_stage);
> if (!last_stage) {
> @@ -1026,9 +1036,10 @@ static int ram_save_page(RAMState *rs,
> PageSearchStatus *pss, bool last_stage)
> ram_counters.transferred +=
> save_page_header(rs, rs->f, block, offset | RAM_SAVE_FLAG_PAGE);
> if (send_async) {
> - qemu_put_buffer_async(rs->f, p, TARGET_PAGE_SIZE,
> - migrate_release_ram() &
> - migration_in_postcopy());
> + bool may_free = migrate_background_snapshot() ||
> + (migrate_release_ram() &&
> + migration_in_postcopy());
> + qemu_put_buffer_async(rs->f, p, TARGET_PAGE_SIZE, may_free);
> } else {
> qemu_put_buffer(rs->f, p, TARGET_PAGE_SIZE);
> }
> @@ -1269,7 +1280,8 @@ static bool find_dirty_block(RAMState *rs,
> PageSearchStatus *pss, bool *again)
> * @rs: current RAM state
> * @offset: used to return the offset within the RAMBlock
> */
> -static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t *offset)
> +static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t *offset,
> + void **page_copy)
> {
> RAMBlock *block = NULL;
>
> @@ -1279,10 +1291,14 @@ static RAMBlock *unqueue_page(RAMState *rs,
> ram_addr_t *offset)
> QSIMPLEQ_FIRST(&rs->src_page_requests);
> block = entry->rb;
> *offset = entry->offset;
> + *page_copy = entry->page_copy;
>
> if (entry->len > TARGET_PAGE_SIZE) {
> entry->len -= TARGET_PAGE_SIZE;
> entry->offset += TARGET_PAGE_SIZE;
> + if (entry->page_copy) {
> + entry->page_copy += TARGET_PAGE_SIZE / sizeof(void *);
> + }
> } else {
> memory_region_unref(block->mr);
> QSIMPLEQ_REMOVE_HEAD(&rs->src_page_requests, next_req);
> @@ -1309,9 +1325,10 @@ static bool get_queued_page(RAMState *rs,
> PageSearchStatus *pss)
> RAMBlock *block;
> ram_addr_t offset;
> bool dirty;
> + void *page_copy;
>
> do {
> - block = unqueue_page(rs, &offset);
> + block = unqueue_page(rs, &offset, &page_copy);
> /*
> * We're sending this page, and since it's postcopy nothing else
> * will dirty it, and we must make sure it doesn't get sent again
> @@ -1349,6 +1366,7 @@ static bool get_queued_page(RAMState *rs,
> PageSearchStatus *pss)
> */
> pss->block = block;
> pss->page = offset >> TARGET_PAGE_BITS;
> + pss->page_copy = page_copy;
> }
>
> return !!block;
> @@ -1386,17 +1404,25 @@ static void migration_page_queue_free(RAMState *rs)
> *
> * @rbname: Name of the RAMBLock of the request. NULL means the
> * same that last one.
> + * @block: RAMBlock to use. block and rbname have mutualy exclusive
> + * semantic with higher priority of the block.
If "mutual exclusive" then no priority at all? Maybe simply:
@block: RAMBlock to use. When @block is provided, then @rbname is ignored.
> * @start: starting address from the start of the RAMBlock
> * @len: length (in bytes) to send
> + * @page_copy: the address the page should be written from
Maybe:
@page_copy: the page to copy to destination. If not specified, will
use the page data specified by @start and @len.
> */
> -int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t
> len)
> +int ram_save_queue_pages(RAMBlock *block, const char *rbname,
> + ram_addr_t start, ram_addr_t len, void *page_copy)
> {
> RAMBlock *ramblock;
> RAMState *rs = ram_state;
>
> ram_counters.postcopy_requests++;
> +
> rcu_read_lock();
> - if (!rbname) {
> +
> + if (block) {
> + ramblock = block;
> + } else if (!rbname) {
> /* Reuse last RAMBlock */
> ramblock = rs->last_req_rb;
>
> @@ -1431,6 +1457,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t
> start, ram_addr_t len)
> new_entry->rb = ramblock;
> new_entry->offset = start;
> new_entry->len = len;
> + new_entry->page_copy = page_copy;
>
> memory_region_ref(ramblock->mr);
> qemu_mutex_lock(&rs->src_page_req_mutex);
> @@ -1468,7 +1495,8 @@ static int ram_save_target_page(RAMState *rs,
> PageSearchStatus *pss,
> * xbzrle can do better than compression.
> */
> if (migrate_use_compression() &&
> - (rs->ram_bulk_stage || !migrate_use_xbzrle())) {
> + (rs->ram_bulk_stage || !migrate_use_xbzrle()) &&
> + !migrate_background_snapshot()) {
This seems unecessary - in the first patch you have already made sure
that compression is not enabled during a live snapshot, so we should
be fine since we check migrate_use_compression() first.
> res = ram_save_compressed_page(rs, pss, last_stage);
> } else {
> res = ram_save_page(rs, pss, last_stage);
> @@ -1706,6 +1734,7 @@ static int ram_find_and_save_block(RAMState *rs, bool
> last_stage)
> pss.block = rs->last_seen_block;
> pss.page = rs->last_page;
> pss.complete_round = false;
> + pss.page_copy = NULL;
>
> if (!pss.block) {
> pss.block = QLIST_FIRST_RCU(&ram_list.blocks);
> diff --git a/migration/ram.h b/migration/ram.h
> index 4c463597a5..c3679ba65e 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -46,7 +46,8 @@ int multifd_load_setup(void);
> int multifd_load_cleanup(Error **errp);
>
> uint64_t ram_pagesize_summary(void);
> -int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t
> len);
> +int ram_save_queue_pages(RAMBlock *block, const char *rbname,
> + ram_addr_t start, ram_addr_t len, void *page_copy);
> void acct_update_position(QEMUFile *f, size_t size, bool zero);
> void ram_debug_dump_bitmap(unsigned long *todump, bool expected,
> unsigned long pages);
> --
> 2.17.0
>
>
Regards,
--
Peter Xu
