2017-12-04 18:24 GMT+08:00 Cornelia Huck <cohuck@xxxxxxxxxx>:
> On Sat, 2 Dec 2017 01:51:40 +0800
> weiping zhang <zwp10758@xxxxxxxxx> wrote:
>
>> cleanup all resource allocated by virtio_mmio_probe.
>>
>> Signed-off-by: weiping zhang <zhangweiping@xxxxxxxxxxxxxxx>
>> ---
>> drivers/virtio/virtio_mmio.c | 34 ++++++++++++++++++++++++++--------
>> 1 file changed, 26 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>> index 74dc717..3fd0e66 100644
>> --- a/drivers/virtio/virtio_mmio.c
>> +++ b/drivers/virtio/virtio_mmio.c
>> @@ -513,8 +513,10 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>> return -EBUSY;
>>
>> vm_dev = devm_kzalloc(&pdev->dev, sizeof(*vm_dev), GFP_KERNEL);
>> - if (!vm_dev)
>> - return -ENOMEM;
>> + if (!vm_dev) {
>> + rc = -ENOMEM;
>
> Touching this would be a good time to remove the extra space in front
> of the -ENOMEM :)
Thanks, I fix this at V2.
>
>> + goto free_mem;
>> + }
>>
>> vm_dev->vdev.dev.parent = &pdev->dev;
>> vm_dev->vdev.dev.release = virtio_mmio_release_dev_empty;
>
> (...)
>
>> @@ -573,7 +580,18 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, vm_dev);
>>
>> - return register_virtio_device(&vm_dev->vdev);
>> + rc = register_virtio_device(&vm_dev->vdev);
>> + if (rc)
>> + goto unmap;
>> + return 0;
>> +unmap:
>> + iounmap(vm_dev->base);
>> +free_mem:
>> + devm_release_mem_region(&pdev->dev, mem->start,
>> + resource_size(mem));
>> +free_vmdev:
>> + devm_kfree(&pdev->dev, vm_dev);
>
> I think this is problematic as vm_dev embeds a struct device (via
> embedding a struct virtio_device). I think the right way to do this is
> - call this only if register_virtio_device() has not been called
> - put the devm_kfree() into the ->release callback for the
> virtio_device (IOW, replace virtio_mmio_release_dev_empty() with a
> function that calls this)
> - do a put_device() if register_virtio_device() failed
>
> I might be missing some interaction between the usual driver model
> handling and devm resources, though.
>
if fail in virtio_mmio_probe, we need free @mem of @pdev, vdev->base,
so I prefer clean all this resource in virtio_mmio_probe itself, keep
vdev.dev.release do nothing.
also we need release these resources at virtio_mmio_remove.
I'll send V2.
--
thanks a ton
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
