Re: [RFC PATCH 1/3] kvm: arm/arm64: vgic-vits: separate vgic_its_free_list() function

Auger Eric
2017-09-12T10:06:00Z

Hi Wanghaibin,

On 12/09/2017 10:50, wanghaibin wrote:
> On 2017/9/6 21:05, wanghaibin wrote:
>
>> We slightly refactor vgic_its_destroy, separate vgic_its_free_list()
>> function for later patch invoke.
>>
>> The patch also take a functional change. If the its->device_list.next
>> is NULL, we still should free the its.
>
>
> Hi, Eric
>
> Does this its->device_list.next is NULL can happened ?

I don't get why we have this check.

The kvm device is removed by kvm_destroy_devices which loops on all
devices added to kvm->devices. kvm_ioctl_create_device only adds the
device to kvm_devices once the lists have been initialized (in
vgic_create_its). So it looks safe to me without the check.

André, do we miss something?

Thanks

Eric

>
> Thanks
>
>> Honestly, I can't understand How does the its->device_list.next is NULL
>> happened at this moment.
>>
>> Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com>
>> ---
>> virt/kvm/arm/vgic/vgic-its.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>> index aa6b68d..25d614f 100644
>> --- a/virt/kvm/arm/vgic/vgic-its.c
>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>> @@ -1624,10 +1624,8 @@ static void vgic_its_free_device(struct kvm *kvm, struct its_device *dev)
>> kfree(dev);
>> }
>>
>> -static void vgic_its_destroy(struct kvm_device *kvm_dev)
>> +static void vgic_its_free_list(struct kvm *kvm, struct vgic_its *its)
>> {
>> - struct kvm *kvm = kvm_dev->kvm;
>> - struct vgic_its *its = kvm_dev->private;
>> struct list_head *cur, *temp;
>>
>> /*
>> @@ -1653,7 +1651,14 @@ static void vgic_its_destroy(struct kvm_device *kvm_dev)
>> kfree(coll);
>> }
>> mutex_unlock(&its->its_lock);
>> +}
>> +
>> +static void vgic_its_destroy(struct kvm_device *kvm_dev)
>> +{
>> + struct kvm *kvm = kvm_dev->kvm;
>> + struct vgic_its *its = kvm_dev->private;
>>
>> + vgic_its_free_list(kvm, its);
>> kfree(its);
>> }
>>
>
>
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>