Re: [PATCH v6 11/18] media: videobuf2: Be more flexible on the number of queue stored buffers

[Hans Verkuil <hverkuil-cisco@xxxxxxxxx>]

Hi Benjamin,

This patch can be folded into 11/18 to make it work properly.

vb2_core_queue_release() is also called when the file handle of the video device is
closed and it is also the owner of the currently allocated buffers. This will free
q->bufs, but queue_init isn't called a second time for non-m2m devices. So move
the allocation of q->bufs from queue_init to reqbufs/create_buffers.

And when releasing the file handle we also check if there is no owner at all:
in that case vb2_queue_release() must still be called to free q->bufs.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 15 +++++++++++++--
 drivers/media/common/videobuf2/videobuf2-v4l2.c |  4 ++--
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index dc7f6b59d237..202d7c80ffd2 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -859,6 +859,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 			return 0;
 	}

+	if (!q->bufs) {
+		q->bufs = kcalloc(q->max_allowed_buffers, sizeof(*q->bufs), GFP_KERNEL);
+		if (!q->bufs)
+			return -ENOMEM;
+	}
+
 	/*
 	 * Make sure the requested values and current defaults are sane.
 	 */
@@ -985,6 +991,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
 		return -ENOBUFS;
 	}

+	if (!q->bufs) {
+		q->bufs = kcalloc(q->max_allowed_buffers, sizeof(*q->bufs), GFP_KERNEL);
+		if (!q->bufs)
+			return -ENOMEM;
+	}
+
 	if (no_previous_buffers) {
 		if (q->waiting_in_dqbuf && *count) {
 			dprintk(q, 1, "another dup()ped fd is waiting for a buffer\n");
@@ -2525,8 +2537,6 @@ int vb2_core_queue_init(struct vb2_queue *q)
 	/* The maximum is limited by offset cookie encoding pattern */
 	q->max_allowed_buffers = min_t(unsigned int, q->max_allowed_buffers, BUFFER_INDEX_MASK);

-	q->bufs = kcalloc(q->max_allowed_buffers, sizeof(*q->bufs), GFP_KERNEL);
-
 	if (q->buf_struct_size == 0)
 		q->buf_struct_size = sizeof(struct vb2_buffer);

@@ -2552,6 +2562,7 @@ void vb2_core_queue_release(struct vb2_queue *q)
 	mutex_lock(&q->mmap_lock);
 	__vb2_queue_free(q, q->num_buffers);
 	kfree(q->bufs);
+	q->bufs = NULL;
 	mutex_unlock(&q->mmap_lock);
 }
 EXPORT_SYMBOL_GPL(vb2_core_queue_release);
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 8ba658ad9891..104fc5c4f574 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -1148,7 +1148,7 @@ int _vb2_fop_release(struct file *file, struct mutex *lock)

 	if (lock)
 		mutex_lock(lock);
-	if (file->private_data == vdev->queue->owner) {
+	if (!vdev->queue->owner || file->private_data == vdev->queue->owner) {
 		vb2_queue_release(vdev->queue);
 		vdev->queue->owner = NULL;
 	}
@@ -1276,7 +1276,7 @@ void vb2_video_unregister_device(struct video_device *vdev)
 	 */
 	get_device(&vdev->dev);
 	video_unregister_device(vdev);
-	if (vdev->queue && vdev->queue->owner) {
+	if (vdev->queue) {
 		struct mutex *lock = vdev->queue->lock ?
 			vdev->queue->lock : vdev->lock;

-- 
2.40.1