From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 3/4] KVM: Count the number of dirty pages for dirty logging Date: Mon, 14 Nov 2011 12:07:27 +0200 Message-ID: <4EC0E85F.80004@redhat.com> References: <20111114182041.43570cdf.yoshikawa.takuya@oss.ntt.co.jp> <20111114182334.f57fbeae.yoshikawa.takuya@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org, takuya.yoshikawa@gmail.com To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:52335 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754016Ab1KNKHd (ORCPT ); Mon, 14 Nov 2011 05:07:33 -0500 In-Reply-To: <20111114182334.f57fbeae.yoshikawa.takuya@oss.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: On 11/14/2011 11:23 AM, Takuya Yoshikawa wrote: > Needed for the next patch which uses this number to decide how to write > protect a slot. > > /* If nothing is dirty, don't bother messing with page tables. */ > - if (is_dirty) { > + if (memslot->nr_dirty_pages) { > struct kvm_memslots *slots, *old_slots; > unsigned long *dirty_bitmap; > > @@ -3504,6 +3500,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, > goto out; > memcpy(slots, kvm->memslots, sizeof(struct kvm_memslots)); > slots->memslots[log->slot].dirty_bitmap = dirty_bitmap; > + slots->memslots[log->slot].nr_dirty_pages = 0; > slots->generation++; > > #endif /* !CONFIG_S390 */ > @@ -1491,7 +1492,8 @@ void mark_page_dirty_in_slot(struct kvm *kvm, struct kvm_memory_slot *memslot, > if (memslot && memslot->dirty_bitmap) { > unsigned long rel_gfn = gfn - memslot->base_gfn; > > - __set_bit_le(rel_gfn, memslot->dirty_bitmap); > + if (!__test_and_set_bit_le(rel_gfn, memslot->dirty_bitmap)) > + memslot->nr_dirty_pages++; > } > } > The two assignments to ->nr_dirty_pages can race, no? Nothing protects it. btw mark_page_dirty() itself seems to assume mmu_lock protection that doesn't exist. Marcelo? -- error compiling committee.c: too many arguments to function