On Tue, May 09, 2017 at 08:59:30PM +0800, 858585 jemmy wrote: > On Tue, May 9, 2017 at 4:54 AM, Stefan Hajnoczi <stefa...@gmail.com> wrote: > > On Fri, May 05, 2017 at 04:03:49PM +0800, jemmy858...@gmail.com wrote: > >> From: Lidong Chen <lidongc...@tencent.com> > >> > >> when block migration with high-speed, mig_save_device_bulk hold the > >> BQL and invoke bdrv_is_allocated frequently. This patch moves > >> bdrv_is_allocated() into bb's AioContext. It will execute without > >> blocking other I/O activity. > >> > >> Signed-off-by: Lidong Chen <lidongc...@tencent.com> > >> --- > >> v4 changelog: > >> Use the prototype code written by Stefan and fix some bug. > >> moves bdrv_is_allocated() into bb's AioContext. > >> --- > >> migration/block.c | 48 +++++++++++++++++++++++++++++++++++++++--------- > >> 1 file changed, 39 insertions(+), 9 deletions(-) > > > > Added Paolo because he's been reworking AioContext and locking. > > > > The goal of this patch is to avoid waiting for bdrv_is_allocated() to > > complete while holding locks. Do bdrv_is_allocated() in the AioContext > > so event processing continues after yield. > > Hi Paolo: > Some information about the problem.
Lidong, please see my comment below: > >> @@ -274,17 +298,23 @@ static int mig_save_device_bulk(QEMUFile *f, > >> BlkMigDevState *bmds) > >> int nr_sectors; > >> > >> if (bmds->shared_base) { > >> + AioContext *bb_ctx; > >> + Coroutine *co; > >> + MigNextAllocatedClusterData data = { > >> + .cur_sector = &cur_sector, > >> + .total_sectors = &total_sectors, > >> + .bb = bb, > >> + }; > >> + qemu_event_init(&data.event, false); > >> + > >> qemu_mutex_lock_iothread(); > >> - aio_context_acquire(blk_get_aio_context(bb)); > >> - /* Skip unallocated sectors; intentionally treats failure as > >> - * an allocated sector */ > >> - while (cur_sector < total_sectors && > >> - !bdrv_is_allocated(blk_bs(bb), cur_sector, > >> - MAX_IS_ALLOCATED_SEARCH, &nr_sectors)) { > >> - cur_sector += nr_sectors; > >> - } > >> - aio_context_release(blk_get_aio_context(bb)); > >> + bdrv_inc_in_flight(blk_bs(bb)); > > > > Please add a comment explaining why bdrv_inc_in_flight() is invoked.
signature.asc
Description: PGP signature