Re: [PATCH 7/7] vimc: free vimc_cap_device when the last user disappears

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hans,

Thank you for the patch.

On Thu, Feb 21, 2019 at 03:21:48PM +0100, Hans Verkuil wrote:
> Don't free vimc_cap_device immediately, instead do this
> in the video_device release function which is called when the
> last user closes the video device. Only then is it safe to
> free the memory.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/platform/vimc/vimc-capture.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
> index ea869631a3f6..3d433361d297 100644
> --- a/drivers/media/platform/vimc/vimc-capture.c
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -338,6 +338,15 @@ static const struct media_entity_operations vimc_cap_mops = {
>  	.link_validate		= vimc_link_validate,
>  };
>  
> +static void vimc_cap_release(struct video_device *vdev)
> +{
> +	struct vimc_cap_device *vcap = container_of(vdev, struct vimc_cap_device,
> +						    vdev);
> +
> +	vimc_pads_cleanup(vcap->ved.pads);
> +	kfree(vcap);
> +}
> +
>  static void vimc_cap_comp_unbind(struct device *comp, struct device *master,
>  				 void *master_data)
>  {
> @@ -348,8 +357,6 @@ static void vimc_cap_comp_unbind(struct device *comp, struct device *master,
>  	vb2_queue_release(&vcap->queue);
>  	media_entity_cleanup(ved->ent);
>  	video_unregister_device(&vcap->vdev);
> -	vimc_pads_cleanup(vcap->ved.pads);
> -	kfree(vcap);
>  }
>  
>  static void *vimc_cap_process_frame(struct vimc_ent_device *ved,
> @@ -467,7 +474,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
>  	vdev = &vcap->vdev;
>  	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
>  	vdev->entity.ops = &vimc_cap_mops;
> -	vdev->release = video_device_release_empty;
> +	vdev->release = vimc_cap_release;

Another use of video_device_release_empty gone, only 80+ to go ;-)

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	vdev->fops = &vimc_cap_fops;
>  	vdev->ioctl_ops = &vimc_cap_ioctl_ops;
>  	vdev->lock = &vcap->lock;

-- 
Regards,

Laurent Pinchart