[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v4 07/13] migration: Split the function ram_save_page
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [v4 07/13] migration: Split the function ram_save_page |
Date: |
Fri, 6 Feb 2015 11:01:20 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Liang Li (address@hidden) wrote:
> Split the function ram_save_page for code reuse purpose.
That's better, but I still think there is an XBZRLE problem; see below.
> Signed-off-by: Liang Li <address@hidden>
> Signed-off-by: Yang Zhang <address@hidden>
> ---
> arch_init.c | 102
> +++++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 56 insertions(+), 46 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 500f299..eae082b 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -595,6 +595,58 @@ static void migration_bitmap_sync_range(ram_addr_t
> start, ram_addr_t length)
> }
> }
>
> +static int save_zero_and_xbzrle_page(QEMUFile *f, uint8_t **current_data,
> + RAMBlock *block, ram_addr_t offset,
> + bool last_stage, bool *send_async)
> +{
> + int bytes_sent = -1;
> + int cont, ret;
> + ram_addr_t current_addr;
> +
> + cont = (block == last_sent_block) ? RAM_SAVE_FLAG_CONTINUE : 0;
> +
> + /* In doubt sent page as normal */
> + ret = ram_control_save_page(f, block->offset,
> + offset, TARGET_PAGE_SIZE, &bytes_sent);
> +
> + XBZRLE_cache_lock();
> +
> + current_addr = block->offset + offset;
> + if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> + if (ret != RAM_SAVE_CONTROL_DELAYED) {
> + if (bytes_sent > 0) {
> + acct_info.norm_pages++;
> + } else if (bytes_sent == 0) {
> + acct_info.dup_pages++;
> + }
> + }
> + } else if (is_zero_range(*current_data, TARGET_PAGE_SIZE)) {
> + acct_info.dup_pages++;
> + bytes_sent = save_block_hdr(f, block, offset, cont,
> + RAM_SAVE_FLAG_COMPRESS);
> + qemu_put_byte(f, 0);
> + bytes_sent++;
> + /* Must let xbzrle know, otherwise a previous (now 0'd) cached
> + * page would be stale
> + */
> + xbzrle_cache_zero_page(current_addr);
> + } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
> + bytes_sent = save_xbzrle_page(f, current_data, current_addr, block,
> + offset, cont, last_stage);
> + if (!last_stage) {
> + /* Can't send this cached data async, since the cache page
> + * might get updated before it gets to the wire
> + */
> + if (send_async != NULL) {
> + *send_async = false;
> + }
> + }
> + }
> +
> + XBZRLE_cache_unlock();
I think this is too soon; when save_xbzrle_page updates current_data to point
to a page from the cache, the cache data is still in use by this point, so
we must be careful that the cache couldn't get resized until after the
qemu_put_buffer
below. Thus this lock must be held until after that.
Dave
> + return bytes_sent;
> +}
>
> /* Needs iothread lock! */
> /* Fix me: there are too many global variables used in migration process. */
> @@ -685,60 +737,20 @@ static void migration_bitmap_sync(void)
> *
> * Returns: Number of bytes written.
> */
> -static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
> +static int ram_save_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
> bool last_stage)
> {
> int bytes_sent;
> int cont;
> - ram_addr_t current_addr;
> MemoryRegion *mr = block->mr;
> uint8_t *p;
> - int ret;
> bool send_async = true;
>
> - cont = (block == last_sent_block) ? RAM_SAVE_FLAG_CONTINUE : 0;
> -
> p = memory_region_get_ram_ptr(mr) + offset;
> -
> - /* In doubt sent page as normal */
> - bytes_sent = -1;
> - ret = ram_control_save_page(f, block->offset,
> - offset, TARGET_PAGE_SIZE, &bytes_sent);
> -
> - XBZRLE_cache_lock();
> -
> - current_addr = block->offset + offset;
> - if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> - if (ret != RAM_SAVE_CONTROL_DELAYED) {
> - if (bytes_sent > 0) {
> - acct_info.norm_pages++;
> - } else if (bytes_sent == 0) {
> - acct_info.dup_pages++;
> - }
> - }
> - } else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
> - acct_info.dup_pages++;
> - bytes_sent = save_block_hdr(f, block, offset, cont,
> - RAM_SAVE_FLAG_COMPRESS);
> - qemu_put_byte(f, 0);
> - bytes_sent++;
> - /* Must let xbzrle know, otherwise a previous (now 0'd) cached
> - * page would be stale
> - */
> - xbzrle_cache_zero_page(current_addr);
> - } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
> - bytes_sent = save_xbzrle_page(f, &p, current_addr, block,
> - offset, cont, last_stage);
> - if (!last_stage) {
> - /* Can't send this cached data async, since the cache page
> - * might get updated before it gets to the wire
> - */
> - send_async = false;
> - }
> - }
> -
> - /* XBZRLE overflow or normal page */
> + bytes_sent = save_zero_and_xbzrle_page(f, &p, block, offset,
> + last_stage, &send_async);
> if (bytes_sent == -1) {
> + cont = (block == last_sent_block) ? RAM_SAVE_FLAG_CONTINUE : 0;
> bytes_sent = save_block_hdr(f, block, offset, cont,
> RAM_SAVE_FLAG_PAGE);
> if (send_async) {
> qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE);
> @@ -749,8 +761,6 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block,
> ram_addr_t offset,
> acct_info.norm_pages++;
> }
>
> - XBZRLE_cache_unlock();
> -
> return bytes_sent;
> }
>
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v4 0/13] migration: Add a new feature to do live migration, Liang Li, 2015/02/02
- [Qemu-devel] [v4 01/13] docs: Add a doc about multiple thread compression, Liang Li, 2015/02/02
- [Qemu-devel] [v4 03/13] migration: Add the framework of multi-thread decompression, Liang Li, 2015/02/02
- [Qemu-devel] [v4 04/13] qemu-file: Add compression functions to QEMUFile, Liang Li, 2015/02/02
- [Qemu-devel] [v4 05/13] arch_init: Alloc and free data struct for compression, Liang Li, 2015/02/02
- [Qemu-devel] [v4 07/13] migration: Split the function ram_save_page, Liang Li, 2015/02/02
- Re: [Qemu-devel] [v4 07/13] migration: Split the function ram_save_page,
Dr. David Alan Gilbert <=
- [Qemu-devel] [v4 08/13] migration: Add the core code of multi-thread compression, Liang Li, 2015/02/02
- [Qemu-devel] [v4 09/13] migration: Make compression co-work with xbzrle, Liang Li, 2015/02/02
- [Qemu-devel] [v4 10/13] migration: Add the core code for decompression, Liang Li, 2015/02/02
- [Qemu-devel] [v4 11/13] migration: Add interface to control compression, Liang Li, 2015/02/02
- [Qemu-devel] [v4 06/13] arch_init: Add and free data struct for decompression, Liang Li, 2015/02/02