Re: [PATCHv9 21/25] v4l: vb2-dma-contig: add reference counting for a device from allocator context

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

Hi Tomasz,

Thanks for the patch.

On Tuesday 02 October 2012 16:27:32 Tomasz Stanislawski wrote:
> This patch adds taking reference to the device for MMAP buffers.
> 
> Such buffers, may be exported using DMABUF mechanism. If the driver that
> created a queue is unloaded then the queue is released, the device might be
> released too.  However, buffers cannot be released if they are referenced by
> DMABUF descriptor(s). The device pointer kept in a buffer must be valid for
> the whole buffer's lifetime. Therefore MMAP buffers should take a reference
> to the device to avoid risk of dangling pointers.
> 
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>
> ---
>  drivers/media/video/videobuf2-dma-contig.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/video/videobuf2-dma-contig.c
> b/drivers/media/video/videobuf2-dma-contig.c index b138b5c..b4d287a 100644
> --- a/drivers/media/video/videobuf2-dma-contig.c
> +++ b/drivers/media/video/videobuf2-dma-contig.c
> @@ -148,6 +148,7 @@ static void vb2_dc_put(void *buf_priv)
>  		kfree(buf->sgt_base);
>  	}
>  	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
> +	put_device(buf->dev);
>  	kfree(buf);
>  }
> 
> @@ -161,9 +162,13 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
> long size) if (!buf)
>  		return ERR_PTR(-ENOMEM);
> 
> +	/* prevent the device from release while the buffer is exported */
> +	get_device(dev);
> +

What about moving this below the dma_alloc_coherent() call ? You could then 
avoid the put_device() call in the error path.

>  	buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr, GFP_KERNEL);
>  	if (!buf->vaddr) {
>  		dev_err(dev, "dma_alloc_coherent of size %ld failed\n", size);
> +		put_device(dev);
>  		kfree(buf);
>  		return ERR_PTR(-ENOMEM);
>  	}

Something like

-	buf->dev = dev;
+	buf->dev = get_device(dev);
	buf->size = size

-- 
Regards,

Laurent Pinchart