Re: [PATCH v6 2/2] migration/dirtyrate: implement dirty-bitmap dirtyrate calculation

[Peter Xu]

On Fri, Jul 16, 2021 at 07:13:47PM +0800, huangy81@chinatelecom.cn wrote:
> +static void calculate_dirtyrate_dirty_bitmap(struct DirtyRateConfig config)
> +{
> +    int64_t msec = 0;
> +    int64_t start_time;
> +    DirtyPageRecord dirty_pages;

[1]

> +
> +    dirtyrate_global_dirty_log_start();
> +
> +    /*
> +     * 1'round of log sync may return all 1 bits with
> +     * KVM_DIRTY_LOG_INITIALLY_SET enable
> +     * skip it unconditionally and start dirty tracking
> +     * from 2'round of log sync
> +     */
> +    dirtyrate_global_dirty_log_sync();
> +
> +    /*
> +     * reset page protect manually and unconditionally.
> +     * this make sure kvm dirty log be cleared if
> +     * KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE cap is enabled.
> +     */
> +    dirtyrate_manual_reset_protect();
> +

[2]

> +    record_dirtypages_bitmap(&dirty_pages, true);

[3]

> +
> +    start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +    DirtyStat.start_time = start_time / 1000;
> +
> +    msec = config.sample_period_seconds * 1000;
> +    msec = set_sample_page_period(msec, start_time);
> +    DirtyStat.calc_time = msec / 1000;
> +
> +    /* fetch dirty bitmap from kvm and stop dirty tracking */

I don't think it really fetched anything..  So I think we need:

       dirtyrate_global_dirty_log_sync();

It seems to be there in older versions but not in the latest two versions.

Please still remember to smoke the patches before posting, because without the
log sync we'll read nothing.

> +    dirtyrate_global_dirty_log_stop();
> +
> +    record_dirtypages_bitmap(&dirty_pages, false);

[4]

I think it's easier we take bql at [1]/[3] and release at [2]/[4], rather than
taking it for every function.  Then we can move the bql operations out of
dirtyrate_global_dirty_log_stop() in this patch.

Thanks,

> +
> +    do_calculate_dirtyrate_bitmap(dirty_pages);
> +}

-- 
Peter Xu