| Message ID | 1438183745-2652-1-git-send-email-crope@iki.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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
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);
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(-)