[PATCHv10 21/26] v4l: vb2-dma-contig: add reference counting for a device from allocator context
laurent.pinchart at ideasonboard.com
Fri Oct 12 02:32:02 PDT 2012
On Friday 12 October 2012 08:28:23 Tomasz Stanislawski wrote:
> On 10/11/2012 11:49 PM, Laurent Pinchart wrote:
> > On Wednesday 10 October 2012 16:46:40 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 at samsung.com>
> >> Acked-by: Hans Verkuil <hans.verkuil at cisco.com>
> > Acked-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > But two small comments below.
> >> ---
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 ++++
> >> 1 file changed, 4 insertions(+)
> >> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> >> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index b138b5c..2d661fd
> >> 100644
> >> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> >> +++ b/drivers/media/v4l2-core/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);
> >> }
> >> @@ -168,6 +169,9 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
> >> long size)
> >> return ERR_PTR(-ENOMEM);
> >> }
> >> + /* prevent the device from release while the buffer is exported */
> > s/prevent/Prevent/ ?
> s/release/being released/ ?
Oops. Of course :-)
> >> + get_device(dev);
> >> +
> >> buf->dev = dev;
> > What about just
> > buf->dev = get_device(dev);
> Right, sorry I missed that from your previous review :).
More information about the dri-devel