Re: [PATCH] vfio: fix deadlock between group lock and kvm lock

[Matthew Rosato]

On 2/1/23 7:43 AM, Liu, Yi L wrote:
>> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
>> Sent: Wednesday, February 1, 2023 4:26 AM
>>
>> On Tue, Jan 31, 2023 at 03:06:35PM -0500, Matthew Rosato wrote:
>>> @@ -799,13 +794,14 @@
>> EXPORT_SYMBOL_GPL(vfio_file_enforced_coherent);
>>>  void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
>>>  {
>>>  	struct vfio_group *group = file->private_data;
>>> +	unsigned long flags;
>>>
>>>  	if (!vfio_file_is_group(file))
>>>  		return;
>>>
>>> -	mutex_lock(&group->group_lock);
>>> +	spin_lock_irqsave(&group->kvm_ref_lock, flags);
>>>  	group->kvm = kvm;
>>> -	mutex_unlock(&group->group_lock);
>>> +	spin_unlock_irqrestore(&group->kvm_ref_lock, flags);
>>
>> We know we are in a sleeping context here so these are just
>> 'spin_lock()', same with the other one
> 
> a dumb question. Why spinlock is required here? 😊
> 

You mean as opposed to another mutex?  I don't think it's required per se (we are replacing a mutex so we could have again used another mutex here), but all current users of this new lock hold it over a very short window (e.g. set a pointer as above, or refcount++ and copy the pointer as in the first device_open)