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

[Christoffer Dall <christoffer.dall@linaro.org>]

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@linaro.org>
> 
> 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);
 
 	/*


Thanks,
-Christoffer