Re: [PATCH 3/4] KVM: Count the number of dirty pages for dirty logging

[Avi Kivity <avi@redhat.com>]

On 12/27/2011 03:50 PM, Marcelo Tosatti wrote:
> On Sat, Dec 24, 2011 at 11:52:51AM +0900, Takuya Yoshikawa wrote:
> > On Fri, 23 Dec 2011 09:14:51 -0200
> > Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > 
> > > > >btw mark_page_dirty() itself seems to assume mmu_lock protection that
> > > > >doesn't exist.  Marcelo?
> > > > >
> > > 
> > > Not mmu_lock protection, kvm->srcu protection.
> > 
> > But it just protects slot readers against updates and two, or more, threads
> > can call mark_page_dirty() concurrently?
>
> Yes, two or more threads can call mark_page_dirty() concurrently.
>
> > What I am worring about here is the atomicity of bitmap updates.
> > 
> > 	commit c8240bd6f0b4b1b21ffd36dd44114d05c7afe0c0
> > 	Author: Alexander Graf <agraf@suse.de>
> > 	Date:   Fri Oct 30 05:47:26 2009 +0000
> > 
> > 		Use Little Endian for Dirty Bitmap
> > 
> > has changed set_bit() to the non-atomic version and nothing protects
> > dirty bits if mmu_lock is not held.
> > 
> > 	The changelog has no explanation why using non-atomic version is safe.
> > 	Some comment in the code may be worthwhile if it is really safe.
>
> It should not be necessary, atomicity of updates to
> memslot->dirty_bitmap, because of RCU updates to the memslot pointer
> (note memslot = gfn_to_memslot(kvm, gfn); in mark_page_dirty).
>
> The order is:
>
> - update page data.
> - grab memslot pointer.
> - set page bit in memslot.
>

What if both grab the same memslot pointer, but set different bits?

-- 
error compiling committee.c: too many arguments to function