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

[Auger Eric <eric.auger@redhat.com>]

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
>