From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takuya Yoshikawa Subject: Re: [PATCH v2 3/4] KVM: cleanup: remove kvm_get_dirty_log() Date: Wed, 23 Jun 2010 18:01:13 +0900 Message-ID: <4C21CD59.9020000@oss.ntt.co.jp> References: <20100623145807.07a07c4c.yoshikawa.takuya@oss.ntt.co.jp> <20100623150134.9fb5915b.yoshikawa.takuya@oss.ntt.co.jp> <4C21CA5D.4060800@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, agraf-l3A5Bk7waGM@public.gmane.org, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, takuya.yoshikawa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org To: Avi Kivity Return-path: In-Reply-To: <4C21CA5D.4060800-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Sender: kvm-ppc-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: kvm.vger.kernel.org (2010/06/23 17:48), Avi Kivity wrote: >> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c >> index 801d9f3..bea6f7c 100644 >> --- a/arch/powerpc/kvm/book3s.c >> +++ b/arch/powerpc/kvm/book3s.c >> @@ -1185,28 +1185,43 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, >> struct kvm_memory_slot *memslot; >> struct kvm_vcpu *vcpu; >> ulong ga, ga_end; >> - int is_dirty = 0; >> - int r; >> + unsigned long is_dirty = 0; >> + int r, i; >> unsigned long n; >> >> mutex_lock(&kvm->slots_lock); >> >> - r = kvm_get_dirty_log(kvm, log,&is_dirty); >> - if (r) >> + r = -EINVAL; >> + if (log->slot>= KVM_MEMORY_SLOTS) >> + goto out; >> + >> + memslot =&kvm->memslots->memslots[log->slot]; > > Not introduced by this patch, but shouldn't this use rcu_dereference()? > > I was thinking like that, but sorry I don't know well about ppc. My final goal is to make everything except /* If nothing is dirty, don't bother messing with page tables. */ part arch independent, so that we can concentrate on the truely dirty logging things in the future. So, making ppc code same as x86, rcu_dereference() for example , really helps me. Do you like this approach? Takuya