On 21/05/18 10:54, Sakari Ailus wrote:
> Lock the media request for updating on QBUF IOCTL using
> media_request_lock_for_update().
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/common/videobuf2/videobuf2-v4l2.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 0a68b19b40da7..8b390960ca671 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -398,12 +398,13 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
> if (IS_ERR(req)) {
> dprintk(1, "%s: invalid request_fd\n", opname);
> return PTR_ERR(req);
> - }
> -
> - if (atomic_read(&req->state) != MEDIA_REQUEST_STATE_IDLE) {
> - dprintk(1, "%s: request is not idle\n", opname);
> - media_request_put(req);
> - return -EBUSY;
> + } else {
> + ret = media_request_lock_for_update(req);
> + if (ret) {
> + media_request_put(req);
> + dprintk(1, "%s: request %d busy\n", opname, b->request_fd);
> + return PTR_ERR(req);
> + }
> }
>
> *p_req = req;
> @@ -683,8 +684,10 @@ int vb2_qbuf(struct vb2_queue *q, struct media_device *mdev,
> if (ret)
> return ret;
> ret = vb2_core_qbuf(q, b->index, b, req);
> - if (req)
> + if (req) {
> + media_request_unlock_for_update(req);
> media_request_put(req);
> + }
> return ret;
> }
> EXPORT_SYMBOL_GPL(vb2_qbuf);
>
The media_request_(un)lock_for_update calls shouldn't be done here, instead they
should happen in videobuf2-core.c in vb2_core_qbuf, right before and after the
call to media_request_object_bind().
The atomic_read in the original patch in vb2_queue_or_prepare_buf() was there as
an early sanity check. The call to media_request_object_bind() is where the real
check for the request state takes place.
Regards,
Hans
