[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 1/4] migration: do not flush_compressed_data
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH v5 1/4] migration: do not flush_compressed_data at the end of each iteration |
Date: |
Mon, 03 Sep 2018 18:38:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
address@hidden wrote:
> From: Xiao Guangrong <address@hidden>
>
> flush_compressed_data() needs to wait all compression threads to
> finish their work, after that all threads are free until the
> migration feeds new request to them, reducing its call can improve
> the throughput and use CPU resource more effectively
> We do not need to flush all threads at the end of iteration, the
> data can be kept locally until the memory block is changed or
> memory migration starts over in that case we will meet a dirtied
> page which may still exists in compression threads's ring
>
> Signed-off-by: Xiao Guangrong <address@hidden>
I am not so sure about this patch.
Right now, we warantee that after each iteration, all data is written
before we start a new round.
This patch changes to only "flush" the compression threads if we have
"synched" with the kvm migration bitmap. Idea is good but as far as I
can see:
- we already call flush_compressed_data() inside firnd_dirty_block if we
synchronize the bitmap. So, at least, we need to update
dirty_sync_count there.
- queued pages are "interesting", but I am not sure if compression and
postcopy work well together.
So, if we don't need to call flush_compressed_data() every round, then
the one inside find_dirty_block() should be enough. Otherwise, I can't
see why we need this other.
Independent of this patch:
- We always send data for every compression thread without testing if
there is any there.
> @@ -3212,7 +3225,6 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> }
> i++;
> }
> - flush_compressed_data(rs);
> rcu_read_unlock();
>
> /*
Why is not enough just to remove this call to flush_compressed_data?
Later, Juan.