[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] migration: keep bytes_xfer_prev init'd to z
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] migration: keep bytes_xfer_prev init'd to zero |
Date: |
Thu, 25 May 2017 08:50:20 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, May 24, 2017 at 05:10:00PM +0100, Felipe Franciosi wrote:
> The first time migration_bitmap_sync() is called, bytes_xfer_prev is set
> to ram_state.bytes_transferred which is, at this point, zero. The next
> time migration_bitmap_sync() is called, an iteration has happened and
> bytes_xfer_prev is set to 'x' bytes. Most likely, more than one second
> has passed, so the auto converge logic will be triggered and
> bytes_xfer_now will also be set to 'x' bytes.
>
> This condition is currently masked by dirty_rate_high_cnt, which will
> wait for a few iterations before throttling. It would otherwise always
> assume zero bytes have been copied and therefore throttle the guest
> (possibly) prematurely.
>
> Given bytes_xfer_prev is only used by the auto convergence logic, it
> makes sense to only set its value after a check has been made against
> bytes_xfer_now.
>
> Signed-off-by: Felipe Franciosi <address@hidden>
Yes, with patch 3, I think it should be accurate.
Reviewed-by: Peter Xu <address@hidden>
> ---
> migration/ram.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index c07a9c0..36bf720 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -673,10 +673,6 @@ static void migration_bitmap_sync(RAMState *rs)
>
> rs->bitmap_sync_count++;
>
> - if (!rs->bytes_xfer_prev) {
> - rs->bytes_xfer_prev = ram_bytes_transferred();
> - }
> -
> if (!rs->time_last_bitmap_sync) {
> rs->time_last_bitmap_sync = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> }
> --
> 1.9.5
>
--
Peter Xu