Memory freeing when dmabuf fds are exported with VIDIOC_EXPBUF
Hi,
I have a question about memory freeing by calling REQBUF(0) before all the
dmabuf fds exported with VIDIOC_EXPBUF are closed.
In calling REQBUF(0), videobuf2-core returns -EBUSY when the reference count
of a vb2 buffer is more than 1. When dmabuf fds are not exported (usual
V4L2_MEMORY_MMAP case),
the check is no problem, but when dmabuf fds are exported and some of them are
not closed (in other words the references to that memory are left),
we cannot succeed in calling REQBUF(0) despite being able to free the memory
after all the references are dropped.
Actually REQBUF(0) does not force a vb2 buffer to be freed but decreases
the refcount of it. Also all the vb2 memory allocators that support dmabuf
exporting
(dma-contig, dma-sg, vmalloc) implements memory freeing by release() of
dma_buf_ops,
so I think there is no need to return -EBUSY when exporting dmabuf fds.
Could you please tell me what you think?
The code that I am talking about is in drivers/media/v4l2-core/videobuf2-core.c:
if (*count == 0 || q->num_buffers != 0 || q->memory != memory) {
/*
* We already have buffers allocated, so first check if they
* are not in use and can be freed.
*/
mutex_lock(>mmap_lock);
if (q->memory == VB2_MEMORY_MMAP && __buffers_in_use(q)) {
mutex_unlock(>mmap_lock);
dprintk(1, "memory in use, cannot free\n");
return -EBUSY;
}
Regards,
Kobayashi
--
-----
IGEL Co.,Ltd. Kazunori Kobayashi
kkoba...@igel.co.jp
http://www.igel.co.jp/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] soc_camera: Support VIDIOC_EXPBUF ioctl
Hi Guennadi,
Thank you for your review.
(2014/09/20 16:50), Guennadi Liakhovetski wrote:
Hi Kobayashi-san,
Thanks for the patch. In principle it looks good, just one question below:
On Thu, 11 Sep 2014, Kazunori Kobayashi wrote:
This patch allows for exporting a dmabuf descriptor from soc_camera drivers.
Signed-off-by: Kazunori Kobayashi kkoba...@igel.co.jp
---
drivers/media/platform/soc_camera/soc_camera.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4308fe..9d7b8ea 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -437,6 +437,19 @@ static int soc_camera_prepare_buf(struct file *file,
void *priv,
return vb2_prepare_buf(icd-vb2_vidq, b);
}
+static int soc_camera_expbuf(struct file *file, void *priv,
+ struct v4l2_exportbuffer *p)
+{
+struct soc_camera_device *icd = file-private_data;
+struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
+
+/* videobuf2 only */
+if (ici-ops-init_videobuf)
+return -EINVAL;
+else
+return vb2_expbuf(icd-vb2_vidq, p);
Many soc-camera queue management functions, like queuing, dequeuing a
buffer check, that the caller is indeed the process, that is controlling
the device. This is done for the case, when two (or more) processes open a
video device node, some of them only want to monitor it, or control
parameters like exposure etc. Whereas only one process is allowed to
perform critical operations, like setting a video format, starting and
stopping streaming, queuing and dequeuing buffers. Should this function
too use such a check
if (icd-streamer != file)
return -EBUSY;
If you agree, I can just add it myself, no need to resubmit.
I think that's appropriate suggestion.
The dmabuf exporting should be restricted to the process that
operates the video streaming.
Please add the check.
Thanks
Guennadi
+}
+
/* Always entered with .host_lock held */
static int soc_camera_init_user_formats(struct soc_camera_device *icd)
{
@@ -2085,6 +2098,7 @@ static const struct v4l2_ioctl_ops
soc_camera_ioctl_ops = {
.vidioc_dqbuf= soc_camera_dqbuf,
.vidioc_create_bufs = soc_camera_create_bufs,
.vidioc_prepare_buf = soc_camera_prepare_buf,
+.vidioc_expbuf = soc_camera_expbuf,
.vidioc_streamon = soc_camera_streamon,
.vidioc_streamoff= soc_camera_streamoff,
.vidioc_cropcap = soc_camera_cropcap,
--
1.8.1.2
--
-
IGEL Co.,Ltd. Kazunori Kobayashi
kkoba...@igel.co.jp
http://www.igel.co.jp/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] soc_camera: Support VIDIOC_EXPBUF ioctl
This patch allows for exporting a dmabuf descriptor from soc_camera drivers.
Signed-off-by: Kazunori Kobayashi kkoba...@igel.co.jp
---
drivers/media/platform/soc_camera/soc_camera.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4308fe..9d7b8ea 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -437,6 +437,19 @@ static int soc_camera_prepare_buf(struct file *file, void
*priv,
return vb2_prepare_buf(icd-vb2_vidq, b);
}
+static int soc_camera_expbuf(struct file *file, void *priv,
+struct v4l2_exportbuffer *p)
+{
+ struct soc_camera_device *icd = file-private_data;
+ struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
+
+ /* videobuf2 only */
+ if (ici-ops-init_videobuf)
+ return -EINVAL;
+ else
+ return vb2_expbuf(icd-vb2_vidq, p);
+}
+
/* Always entered with .host_lock held */
static int soc_camera_init_user_formats(struct soc_camera_device *icd)
{
@@ -2085,6 +2098,7 @@ static const struct v4l2_ioctl_ops soc_camera_ioctl_ops =
{
.vidioc_dqbuf= soc_camera_dqbuf,
.vidioc_create_bufs = soc_camera_create_bufs,
.vidioc_prepare_buf = soc_camera_prepare_buf,
+ .vidioc_expbuf = soc_camera_expbuf,
.vidioc_streamon = soc_camera_streamon,
.vidioc_streamoff= soc_camera_streamoff,
.vidioc_cropcap = soc_camera_cropcap,
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
