On Wed, Mar 29, 2017 at 11:57 PM, Juan Quintela <quint...@redhat.com> wrote:
>
> 858585 jemmy <jemmy858...@gmail.com> wrote:
> > On Tue, Mar 28, 2017 at 5:47 PM, Juan Quintela <quint...@redhat.com> wrote:
> >> Lidong Chen <jemmy858...@gmail.com> wrote:
> >>> when migration with quick 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.
> >>>
> >>> Signed-off-by: Lidong Chen <lidongc...@tencent.com>
> >>> ---
> >>> migration/block.c | 39 +++++++++++++++++++++++++++++++--------
> >>> 1 file changed, 31 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/migration/block.c b/migration/block.c
> >>> index 7734ff7..d3e81ca 100644
> >>> --- a/migration/block.c
> >>> +++ b/migration/block.c
> >>> @@ -110,6 +110,7 @@ typedef struct BlkMigState {
> >>> int transferred;
> >>> int prev_progress;
> >>> int bulk_completed;
> >>> + int time_ns_used;
> >>
> >> An int that can only take values 0/1 is called a bool O:-)
> > time_ns_used is used to store how many ns used by bdrv_is_allocated.
>
> Oops, I really mean timeout_flag, sorry :-(
>
> >> Do we really want to call clock_gettime each time that
> >> bdrv_is_allocated() is called? My understanding is that clock_gettime
> >> is expensive, but I don't know how expensive is brdrv_is_allocated()
> >
> > i write this code to measure the time used by brdrv_is_allocated()
> >
> > 279 static int max_time = 0;
> > 280 int tmp;
> >
> > 288 clock_gettime(CLOCK_MONOTONIC_RAW, &ts1);
> > 289 ret = bdrv_is_allocated(blk_bs(bb), cur_sector,
> > 290 MAX_IS_ALLOCATED_SEARCH,
> > &nr_sectors);
> > 291 clock_gettime(CLOCK_MONOTONIC_RAW, &ts2);
> > 292
> > 293
> > 294 tmp = (ts2.tv_sec - ts1.tv_sec)*1000000000L
> > 295 + (ts2.tv_nsec - ts1.tv_nsec);
> > 296 if (tmp > max_time) {
> > 297 max_time=tmp;
> > 298 fprintf(stderr, "max_time is %d\n", max_time);
> > 299 }
> >
> > the test result is below:
> >
> > max_time is 37014
> > max_time is 1075534
> > max_time is 17180913
> > max_time is 28586762
> > max_time is 49563584
> > max_time is 103085447
> > max_time is 110836833
> > max_time is 120331438
>
> this is around 120ms, no? It is quite a lot, really :-(
i find the reason is mainly 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);
>
>
> > so i think it's necessary to clock_gettime each time.
> > but clock_gettime only available on linux. maybe clock() is better.
>
qemu_clock_get_ms(QEMU_CLOCK_REALTIME) is a better option.
> Thanks, Juan.
