On Wed, Jun 23, 2010 at 12:00:47PM +0300, Avi Kivity wrote: > On 06/23/2010 12:01 PM, Takuya Yoshikawa wrote: > >(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? > > Well yes, I expect that not using rcu_dereference() (and > srcu_read_lock()) is a bug here. > > Marcelo? No because slots_lock is held which serializes with memslot updates. > On a related note, I'd like to consolidate rcu locking: > > - all vcpu ioctls take srcu_read_lock() when they start and drop it > at the end. > - KVM_VCPU_RUN drops the lock when sleeping and entering the guest > (also on context switch?) Yep, makes sense. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
