Re: [PATCH 2/2] KVM: Protect device ops->create and list_add with kvm->lock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 09, 2016 at 03:16:26PM +0200, Paolo Bonzini wrote:
> 
> 
> On 09/08/2016 14:55, Christoffer Dall wrote:
> > On Tue, Aug 09, 2016 at 02:37:43PM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 09/08/2016 14:20, Christoffer Dall wrote:
> >>> KVM devices were manipulating list data structures without any form of
> >>> synchronization, and some implementations of the create operations also
> >>> suffered from a lack of synchronization.
> >>>
> >>> Now when we've split the xics create operation into create and init, we
> >>> can hold the kvm->lock mutex while calling the create operation and when
> >>> manipulating the devices list.
> >>>
> >>> The error path in the generic code gets slightly ugly because we have to
> >>> take the mutex again and delete the device from the list, but holding
> >>> the mutex during anon_inode_getfd or releasing/locking the mutex in the
> >>> common non-error path seemed wrong.
> >>>
> >>> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> >>
> >> Very nice (and small), but please add a comment to the create member in
> >> kvm_device_ops.
> > 
> > Like this?:
> > 
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index d3c9b82..9c28b4d 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -1113,6 +1113,12 @@ struct kvm_device {
> >  /* create, destroy, and name are mandatory */
> >  struct kvm_device_ops {
> >  	const char *name;
> > +
> > +	/*
> > +	 * create is called holding kvm->lock and any operations not suitable
> > +	 * to do while holding the lock should be deferred to init (see
> > +	 * below).
> > +	 */
> >  	int (*create)(struct kvm_device *dev, u32 type);
> >  
> >  	/*
> > 
> 
> That's okay, series
> 
> Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>

Thanks, I'll send a v2.  Will you just apply the patches to kvm/master
or would you like me to include it in my pull request for -rc2 ?

Also, do you want to wait for a tested-by from the other arch
maintainers?

-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux