On 11/09/2016 10:00 PM, Xiao Guangrong wrote:
> On 11/09/2016 09:31 PM, Paolo Bonzini wrote:
>> On 09/11/2016 14:06, Xiao Guangrong wrote:
>>> On 11/09/2016 08:49 PM, Jike Song wrote:
>>>
>>>> +void vfio_group_attach_kvm(struct vfio_group *group, struct kvm *kvm,
>>>> + void (*fn)(struct kvm *))
>>>> +{
>>>> + mutex_lock(&group->udata.lock);
>>>
>>> This lock is needed, please see below.
>>
>> *not* needed I guess.
>
> Yes, indeed. Sorry for the typo. :(
>
>>
>>>> +
>>>> + fn(kvm);
>>>> + blocking_notifier_call_chain(&group->udata.notifier,
>>>> + VFIO_GROUP_NOTIFY_ATTACH_KVM, kvm);
>>>
>>> As this is a callback before KVM releases its last refcount, i do not
>>> think vendor driver need to get additional KVM refcount.
>>
>> The *group* driver doesn't need it indeed. The mdev vendor driver
>> however does, so it will use kvm_get_kvm under its own mutex. That is:
>
> Yes, own mutex is definitely can work.:) It is vendor driver internal
> operation and it depends on the internal implementation.
>
> My idea is that we can make sure the KVM instance is valid before calling
> DETACH callback. So if we can properly release all the resource associated
> with the kvm instance in this callback, it is okay without additional kvm
> ref.
Yes, the mutex in vfio_group is actually pointless, now I understand and agree.
Thanks Guangrong and Polao :)
--
Thanks,
Jike
--
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
