Re: [Qemu-devel] [PATCH v3 00/29] Dirty bitmap atomic access and optimizations

[Aurelien Jarno <aurelien@aurel32.net>]

On 2015-05-26 18:54, Paolo Bonzini wrote:
> QEMU is currently accessing the dirty bitmaps very liberally,
> which is understandable since the accesses are cheap.  This is
> however not good for squeezing maximum performance out of dataplane,
> and is also not good if the accesses become more expensive---as is
> the case when they use atomic primitives.
> 
> This patch series does the following optimizations and cleanups:
> 
> 1) it lets KVM code treat migration as "just another dirty bitmap
> client" instead of needing the special global_log_start/stop callbacks.
> These remain in use in Xen and vhost.  This removes code and avoids
> bugs such as the one fixed in commit 4cc856f (kvm-all: Sync dirty-bitmap
> from kvm before kvm destroy the corresponding dirty_bitmap, 2015-04-02).
> 
> 2) it avoids modifications to unused dirty bitmaps: code if TCG
> is disabled, migration if no migration is in progress, VGA for
> regions other than VRAM.
> 
> and on top of this makes dirty bitmap access atomic.  I'm not including
> the patch to make the migration thread synchronize the bitmap outside
> the big QEMU lock (thus removing the last source of jitter during the
> RAM copy phase of migration) but it is also enabled by these patches.
> 
> Patches 1-4 are cleanups to DIRTY_MEMORY_VGA users.
> 
> Patches 5-12 are the first cleanup (KVM treats migration as just
> another client).  Patches 13-14 are a simple optimization that is enabled
> by these patches.
> 
> Patches 15-18 are bonus cleanups to translate-all.c's dirty memory
> tracking for TCG.
> 
> Patches 19-22 are the second cleanup (avoid modifications to unused
> dirty bitmaps).
> 
> Patches 23-28 are Stefan's patches for atomic access to the dirty
> bitmap, which has no performance impact in the common case thanks to
> the previous work.
> 
> Patch 29 is an unrelated strengthening of assertions, that mst spotted
> while reviewing v1.

I don't feel qualified to review this kind of patch set. That said, I
have tested the SH4 machine (and hence the sm501 display) and the MIPS
magnum machine up to the ARCsystem (and hence the g364fb display) and
they both seems to work correctly with this patchset applied.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net