Re: [PATCH RFC v2 9/9] KVM: Dirty ring support

[Peter Xu]

On Tue, Apr 28, 2020 at 04:05:09PM -0400, Peter Xu wrote:
> +/*
> + * Flush all the existing dirty pages to the KVM slot buffers.  When
> + * this call returns, we guarantee that all the touched dirty pages
> + * before calling this function have been put into the per-kvmslot
> + * dirty bitmap.
> + *
> + * To achieve this, we need to:
> + *
> + * (1) Kick all vcpus out, this will make sure that we flush all the
> + *     dirty buffers that potentially in the hardware (PML) into the
> + *     dirty rings, after that,
> + *
> + * (2) Kick the reaper thread and make sure it reaps all the dirty
> + *     page that is in the dirty rings.

Please note that some of the comments might be outdated, like this one...

(I think I'll remove these two paragraph in the next post)

> + *
> + * This function must be called with BQL held.
> + */
> +static void kvm_dirty_ring_flush(struct KVMDirtyRingReaper *r)
> +{
> +    trace_kvm_dirty_ring_flush(0);
> +
> +    /*
> +     * The function needs to be serialized.  Since this function
> +     * should always be with BQL held, serialization is guaranteed.
> +     * However, let's be sure of it.
> +     */
> +    assert(qemu_mutex_iothread_locked());
> +
> +    /*
> +     * First make sure to flush the hardware buffers by kicking all
> +     * vcpus out in a synchronous way.
> +     */
> +    kvm_cpu_synchronize_kick_all();
> +
> +    /*
> +     * Recycle the dirty bits outside the reaper thread.  We're safe because
> +     * kvm_dirty_ring_reap() is internally protected by a mutex.

Same here; the comment is obsolete.  There used to be a mutex after v1 and
before v2, but I removed the mutex because now we simply always take the BQL so
that mutex is not needed any more.

I'm not sure whether there's still obsolete comments here and there (since the
code does changed quite a bit).  Anyway please stick to the code if there's
conflicts, and I'll try to fix the comments up.

> +     */
> +    kvm_dirty_ring_reap(kvm_state);
> +
> +    trace_kvm_dirty_ring_flush(1);
> +}

-- 
Peter Xu