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 :-(
> so i think it's necessary to clock_gettime each time.
> but clock_gettime only available on linux. maybe clock() is better.
Thanks, Juan.
