guangrong.xiao@xxxxxxxxx wrote: > From: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxx> > > 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 <xiaoguangrong@xxxxxxxxxxx> 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.
