Re: [Qemu-devel] [PATCH v3] migration/block:limit the time used for block migration

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2985 bytes --]

On Fri, Apr 07, 2017 at 09:30:33AM +0800, 858585 jemmy wrote:
> On Thu, Apr 6, 2017 at 10:02 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Wed, Apr 05, 2017 at 05:27:58PM +0800, jemmy858585@gmail.com wrote:
> >> From: Lidong Chen <lidongchen@tencent.com>
> >>
> >> when migration with high speed, mig_save_device_bulk invoke
> >> bdrv_is_allocated too frequently, and cause vnc reponse slowly.
> >> this patch limit the time used for bdrv_is_allocated.
> >
> > bdrv_is_allocated() is supposed to yield back to the event loop if it
> > needs to block.  If your VNC session is experiencing jitter then it's
> > probably because a system call in the bdrv_is_allocated() code path is
> > synchronous when it should be asynchronous.
> >
> > You could try to identify the system call using strace -f -T.  In the
> > output you'll see the duration of each system call.  I guess there is a
> > file I/O system call that is taking noticable amounts of time.
> 
> yes, i find the reason where bdrv_is_allocated needs to block.
> 
> the mainly reason is caused by qemu_co_mutex_lock invoked by
> qcow2_co_get_block_status.
>     qemu_co_mutex_lock(&s->lock);
>     ret = qcow2_get_cluster_offset(bs, sector_num << 9, &bytes,
>                                    &cluster_offset);
>     qemu_co_mutex_unlock(&s->lock);
> 
> other reason is caused by l2_load invoked by
> qcow2_get_cluster_offset.
> 
>     /* load the l2 table in memory */
> 
>     ret = l2_load(bs, l2_offset, &l2_table);
>     if (ret < 0) {
>         return ret;
>     }

The migration thread is holding the QEMU global mutex, the AioContext,
and the qcow2 s->lock while the L2 table is read from disk.

The QEMU global mutex is needed for block layer operations that touch
the global drives list.  bdrv_is_allocated() can be called without the
global mutex.

The VNC server's file descriptor is not in the BDS AioContext.
Therefore it can be processed while the migration thread holds the
AioContext and qcow2 s->lock.

Does the following patch solve the problem?

diff --git a/migration/block.c b/migration/block.c
index 7734ff7..072fc20 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -276,6 +276,7 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds)
     if (bmds->shared_base) {
         qemu_mutex_lock_iothread();
         aio_context_acquire(blk_get_aio_context(bb));
+        qemu_mutex_unlock_iothread();
         /* Skip unallocated sectors; intentionally treats failure as
          * an allocated sector */
         while (cur_sector < total_sectors &&
@@ -283,6 +284,7 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds)
                                   MAX_IS_ALLOCATED_SEARCH, &nr_sectors)) {
             cur_sector += nr_sectors;
         }
+        qemu_mutex_lock_iothread();
         aio_context_release(blk_get_aio_context(bb));
         qemu_mutex_unlock_iothread();
     }


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]