All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: linux-media@vger.kernel.org
Cc: pawel@osciak.com, s.nawrocki@samsung.com,
	m.szyprowski@samsung.com, Hans Verkuil <hans.verkuil@cisco.com>
Subject: [REVIEWv2 PATCH 10/15] vb2: don't init the list if there are still buffers
Date: Tue, 25 Feb 2014 13:52:50 +0100	[thread overview]
Message-ID: <1393332775-44067-11-git-send-email-hverkuil@xs4all.nl> (raw)
In-Reply-To: <1393332775-44067-1-git-send-email-hverkuil@xs4all.nl>

From: Hans Verkuil <hans.verkuil@cisco.com>

__vb2_queue_free() would init the queued_list at all times, even if
q->num_buffers > 0. This should only happen if num_buffers == 0.

This situation can happen if a CREATE_BUFFERS call couldn't allocate
enough buffers and had to free those it did manage to allocate before
returning an error.

While we're at it: __vb2_queue_alloc() returns the number of buffers
allocated, not an error code. So stick the result in allocated_buffers
instead of ret as that's very confusing.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 2a7815c..90374c0 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -452,9 +452,10 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 	}
 
 	q->num_buffers -= buffers;
-	if (!q->num_buffers)
+	if (!q->num_buffers) {
 		q->memory = 0;
-	INIT_LIST_HEAD(&q->queued_list);
+		INIT_LIST_HEAD(&q->queued_list);
+	}
 	return 0;
 }
 
@@ -820,14 +821,12 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	}
 
 	/* Finally, allocate buffers and video memory */
-	ret = __vb2_queue_alloc(q, req->memory, num_buffers, num_planes);
-	if (ret == 0) {
+	allocated_buffers = __vb2_queue_alloc(q, req->memory, num_buffers, num_planes);
+	if (allocated_buffers == 0) {
 		dprintk(1, "Memory allocation failed\n");
 		return -ENOMEM;
 	}
 
-	allocated_buffers = ret;
-
 	/*
 	 * Check if driver can handle the allocated number of buffers.
 	 */
@@ -851,6 +850,10 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	q->num_buffers = allocated_buffers;
 
 	if (ret < 0) {
+		/*
+		 * Note: __vb2_queue_free() will subtract 'allocated_buffers'
+		 * from q->num_buffers.
+		 */
 		__vb2_queue_free(q, allocated_buffers);
 		return ret;
 	}
@@ -924,20 +927,18 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
 	}
 
 	/* Finally, allocate buffers and video memory */
-	ret = __vb2_queue_alloc(q, create->memory, num_buffers,
+	allocated_buffers = __vb2_queue_alloc(q, create->memory, num_buffers,
 				num_planes);
-	if (ret == 0) {
+	if (allocated_buffers == 0) {
 		dprintk(1, "Memory allocation failed\n");
 		return -ENOMEM;
 	}
 
-	allocated_buffers = ret;
-
 	/*
 	 * Check if driver can handle the so far allocated number of buffers.
 	 */
-	if (ret < num_buffers) {
-		num_buffers = ret;
+	if (allocated_buffers < num_buffers) {
+		num_buffers = allocated_buffers;
 
 		/*
 		 * q->num_buffers contains the total number of buffers, that the
@@ -960,6 +961,10 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
 	q->num_buffers += allocated_buffers;
 
 	if (ret < 0) {
+		/*
+		 * Note: __vb2_queue_free() will subtract 'allocated_buffers'
+		 * from q->num_buffers.
+		 */
 		__vb2_queue_free(q, allocated_buffers);
 		return -ENOMEM;
 	}
-- 
1.9.0


  parent reply	other threads:[~2014-02-25 12:53 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-25 12:52 [REVIEWv2 PATCH 00/15] vb2: fixes, balancing callbacks (PART 1) Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 01/15] vb2: Check if there are buffers before streamon Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 02/15] vb2: fix read/write regression Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 03/15] vb2: fix PREPARE_BUF regression Hans Verkuil
2014-02-27 10:52   ` Laurent Pinchart
2014-02-25 12:52 ` [REVIEWv2 PATCH 04/15] vb2: add debugging code to check for unbalanced ops Hans Verkuil
2014-02-27 11:15   ` Laurent Pinchart
2014-02-27 11:23     ` Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 05/15] vb2: change result code of buf_finish to void Hans Verkuil
2014-02-27 11:23   ` Laurent Pinchart
2014-02-27 11:27     ` Laurent Pinchart
2014-02-27 11:28   ` Laurent Pinchart
2014-02-25 12:52 ` [REVIEWv2 PATCH 06/15] vb2: add note that buf_finish can be called with !vb2_is_streaming() Hans Verkuil
2014-02-27 11:39   ` Laurent Pinchart
2014-02-25 12:52 ` [REVIEWv2 PATCH 07/15] vb2: call buf_finish from __dqbuf Hans Verkuil
2014-02-27 11:51   ` Laurent Pinchart
2014-02-27 12:13     ` Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 08/15] vb2: fix buf_init/buf_cleanup call sequences Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 09/15] vb2: rename queued_count to owned_by_drv_count Hans Verkuil
2014-02-27 12:08   ` Laurent Pinchart
2014-02-25 12:52 ` Hans Verkuil [this message]
2014-02-27 12:08   ` [REVIEWv2 PATCH 10/15] vb2: don't init the list if there are still buffers Laurent Pinchart
2014-02-25 12:52 ` [REVIEWv2 PATCH 11/15] vb2: only call start_streaming if sufficient buffers are queued Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 12/15] vb2: properly clean up PREPARED and QUEUED buffers Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 13/15] vb2: replace BUG by WARN_ON Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 14/15] vb2: fix streamoff handling if streamon wasn't called Hans Verkuil
2014-02-25 12:52 ` [REVIEWv2 PATCH 15/15] vivi: correctly cleanup after a start_streaming failure Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1393332775-44067-11-git-send-email-hverkuil@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-media@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=pawel@osciak.com \
    --cc=s.nawrocki@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.