On 09/18/2014 11:57 PM, Sakari Ailus wrote: > Patch "[media] v4l: vb2: Fix stream start and buffer completion race" has a > sets q->start_streaming_called before calling queue op start_streaming() in > order to fix a bug. This has the side effect that buffers returned to > videobuf2 in VB2_BUF_STATE_QUEUED will cause a WARN_ON() to be called. > > Add a new field called done_buffers_queued_state to struct vb2_queue, which > must be set if the new state of buffers returned to videobuf2 must be > VB2_BUF_STATE_QUEUED, i.e. buffers returned in start_streaming op.
I posted a fix for this over a month ago: https://www.mail-archive.com/linux-media@vger.kernel.org/msg77871.html Unfortunately, the pull request with that patch (https://patchwork.linuxtv.org/patch/25162/) fell through the cracks as I discovered yesterday. Hopefully Mauro will pick up that pull request quickly. I prefer my patch since that avoids introducing yet another state variable. Regards, Hans > > Signed-off-by: Sakari Ailus <sakari.ai...@iki.fi> > Cc: Hans Verkuil <hverk...@xs4all.nl> > --- > drivers/media/v4l2-core/videobuf2-core.c | 5 +++-- > include/media/videobuf2-core.h | 4 ++++ > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > b/drivers/media/v4l2-core/videobuf2-core.c > index 7e6aff6..202e2a5 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -1174,7 +1174,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum > vb2_buffer_state state) > if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE)) > return; > > - if (!q->start_streaming_called) { > + if (q->done_buffers_queued_state) { > if (WARN_ON(state != VB2_BUF_STATE_QUEUED)) > state = VB2_BUF_STATE_QUEUED; > } else if (WARN_ON(state != VB2_BUF_STATE_DONE && > @@ -1742,9 +1742,10 @@ static int vb2_start_streaming(struct vb2_queue *q) > __enqueue_in_driver(vb); > > /* Tell the driver to start streaming */ > - q->start_streaming_called = 1; > + q->done_buffers_queued_state = q->start_streaming_called = 1; > ret = call_qop(q, start_streaming, q, > atomic_read(&q->owned_by_drv_count)); > + q->done_buffers_queued_state = 0; > if (!ret) > return 0; > > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 5a10d8d..7c0dac6 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -380,6 +380,9 @@ struct v4l2_fh; > * @streaming: current streaming state > * @start_streaming_called: start_streaming() was called successfully and we > * started streaming. > + * @done_buffers_queued_state: buffers returned to videobuf2 must go > + * to VB2_BUF_STATE_QUEUED state. This is the case whilst > + * the driver's start_streaming op is called. > * @error: a fatal error occurred on the queue > * @fileio: file io emulator internal data, used only if emulator is active > * @threadio: thread io internal data, used only if thread is active > @@ -418,6 +421,7 @@ struct vb2_queue { > > unsigned int streaming:1; > unsigned int start_streaming_called:1; > + unsigned int done_buffers_queued_state:1; > unsigned int error:1; > > struct vb2_fileio_data *fileio; > -- 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
