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)
