[PATCH] vb2: revert: vb2: allow requeuing buffers while streaming

[PATCH] vb2: revert: vb2: allow requeuing buffers while streaming

[Antti Palosaari]

commit ce0eff016f7272faa6dc6eec722b1ca1970ff9aa
[media] vb2: allow requeuing buffers while streaming

That commit causes buf_queue() called on infinity loop when
start_streaming() returns error. On that case resources are eaten
quickly and machine crashes.

Cc: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 93b3154..e7b4f6a 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -182,7 +182,6 @@ module_param(debug, int, 0644);
 				 V4L2_BUF_FLAG_KEYFRAME | V4L2_BUF_FLAG_TIMECODE)
 
 static void __vb2_queue_cancel(struct vb2_queue *q);
-static void __enqueue_in_driver(struct vb2_buffer *vb);
 
 /**
  * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
@@ -1154,9 +1153,8 @@ EXPORT_SYMBOL_GPL(vb2_plane_cookie);
 /**
  * vb2_buffer_done() - inform videobuf that an operation on a buffer is finished
  * @vb:		vb2_buffer returned from the driver
- * @state:	either VB2_BUF_STATE_DONE if the operation finished successfully,
- *		VB2_BUF_STATE_ERROR if the operation finished with an error or
- *		VB2_BUF_STATE_QUEUED if the driver wants to requeue buffers.
+ * @state:	either VB2_BUF_STATE_DONE if the operation finished successfully
+ *		or VB2_BUF_STATE_ERROR if the operation finished with an error.
  *		If start_streaming fails then it should return buffers with state
  *		VB2_BUF_STATE_QUEUED to put them back into the queue.
  *
@@ -1207,11 +1205,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	atomic_dec(&q->owned_by_drv_count);
 	spin_unlock_irqrestore(&q->done_lock, flags);
 
-	if (state == VB2_BUF_STATE_QUEUED) {
-		if (q->start_streaming_called)
-			__enqueue_in_driver(vb);
+	if (state == VB2_BUF_STATE_QUEUED)
 		return;
-	}
 
 	/* Inform any processes that may be waiting for buffers */
 	wake_up(&q->done_wq);
-- 
http://palosaari.fi/

Re: [PATCH] vb2: revert: vb2: allow requeuing buffers while streaming

[Sakari Ailus]

Terve,

On Wed, Jul 29, 2015 at 06:29:05PM +0300, Antti Palosaari wrote:
> commit ce0eff016f7272faa6dc6eec722b1ca1970ff9aa
> [media] vb2: allow requeuing buffers while streaming
> 
> That commit causes buf_queue() called on infinity loop when
> start_streaming() returns error. On that case resources are eaten
> quickly and machine crashes.
> 
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Antti Palosaari <crope@iki.fi>

Fixed by this patch in media-fixes:

commit 6d058c5643e16779ae4c001d2e893c140940e48f
Author: Sakari Ailus <sakari.ailus@linux.intel.com>
Date:   Fri Jul 3 04:37:07 2015 -0300

    [media] vb2: Only requeue buffers immediately once streaming is started
    
    Buffers can be returned back to videobuf2 in driver's streamon handler. In
    this case vb2_buffer_done() with buffer state VB2_BUF_STATE_QUEUED will
    cause the driver's buf_queue vb2 operation to be called, queueing the same
    buffer again only to be returned to videobuf2 using vb2_buffer_done() and so
    on.
    
    Add a new buffer state VB2_BUF_STATE_REQUEUEING which, when used as the
    state argument to vb2_buffer_done(), will result in buffers queued to the
    driver. Using VB2_BUF_STATE_QUEUED will leave the buffer to videobuf2, as it
    was before "[media] vb2: allow requeuing buffers while streaming".
    
    Fixes: ce0eff016f72 ("[media] vb2: allow requeuing buffers while streaming")
    
    [mchehab@osg.samsung.com: fix warning: enumeration value 'VB2_BUF_STATE_REQUEUEING' not handled in switch]
    
    Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
    Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
    Cc: stable@vger.kernel.org # for v4.1
    Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

-- 
Terveisin,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH] vb2: revert: vb2: allow requeuing buffers while streaming

[Antti Palosaari]

Moikka!

On 07/31/2015 05:43 PM, Sakari Ailus wrote:
> Terve,
>
> On Wed, Jul 29, 2015 at 06:29:05PM +0300, Antti Palosaari wrote:
>> commit ce0eff016f7272faa6dc6eec722b1ca1970ff9aa
>> [media] vb2: allow requeuing buffers while streaming
>>
>> That commit causes buf_queue() called on infinity loop when
>> start_streaming() returns error. On that case resources are eaten
>> quickly and machine crashes.
>>
>> Cc: Hans Verkuil <hverkuil@xs4all.nl>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>
> Fixed by this patch in media-fixes:
>
> commit 6d058c5643e16779ae4c001d2e893c140940e48f
> Author: Sakari Ailus <sakari.ailus@linux.intel.com>
> Date:   Fri Jul 3 04:37:07 2015 -0300
>
>      [media] vb2: Only requeue buffers immediately once streaming is started
>
>      Buffers can be returned back to videobuf2 in driver's streamon handler. In
>      this case vb2_buffer_done() with buffer state VB2_BUF_STATE_QUEUED will
>      cause the driver's buf_queue vb2 operation to be called, queueing the same
>      buffer again only to be returned to videobuf2 using vb2_buffer_done() and so
>      on.
>
>      Add a new buffer state VB2_BUF_STATE_REQUEUEING which, when used as the
>      state argument to vb2_buffer_done(), will result in buffers queued to the
>      driver. Using VB2_BUF_STATE_QUEUED will leave the buffer to videobuf2, as it
>      was before "[media] vb2: allow requeuing buffers while streaming".
>
>      Fixes: ce0eff016f72 ("[media] vb2: allow requeuing buffers while streaming")
>
>      [mchehab@osg.samsung.com: fix warning: enumeration value 'VB2_BUF_STATE_REQUEUEING' not handled in switch]
>
>      Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>      Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
>      Cc: stable@vger.kernel.org # for v4.1
>      Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>

I wonder why this fix is not included to media master yet, but only 
fixes... It is not first time such happens. I wasted a lot of time when 
I tried implement receiver / transmitter blocking to hackrf driver 
start_streaming and it crashes machine always, lets say 15 times, until 
I started suspect issue might be somewhere else than driver.

Anyhow, you could crash machine easily with that bug using vivid driver 
error injection option
v4l2-ctl -d /dev/video0 -c inject_vidioc_streamon_error=1

regards
Antti

-- 
http://palosaari.fi/