Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b18a8ff2 authored by Hans Verkuil's avatar Hans Verkuil Committed by Mauro Carvalho Chehab
Browse files

[media] vb2: push the mmap semaphore down to __buf_prepare() 



Rather than taking the mmap semaphore at a relatively high-level function,
push it down to the place where it is really needed.

It was placed in vb2_queue_or_prepare_buf() to prevent racing with other
vb2 calls. The only way I can see that a race can happen is when two
threads queue the same buffer. The solution for that it to introduce
a PREPARING state.

Moving it down offers opportunities to simplify the code.

Signed-off-by: default avatarHans Verkuil <hans.verkuil@cisco.com>
Acked-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: default avatarMauro Carvalho Chehab <m.chehab@samsung.com>
parent b4fcdaf7

drivers/media/v4l2-core/videobuf2-core.c

+36 −46
Original line number Diff line number Diff line
@@ -481,6 +481,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) 
	case VB2_BUF_STATE_PREPARED:

		b->flags |= V4L2_BUF_FLAG_PREPARED;

		break;

	case VB2_BUF_STATE_PREPARING:

	case VB2_BUF_STATE_DEQUEUED:

		/* nothing */

		break;
@@ -1228,6 +1229,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb) 
static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)

{

	struct vb2_queue *q = vb->vb2_queue;

	struct rw_semaphore *mmap_sem;

	int ret;



	ret = __verify_length(vb, b);
@@ -1237,12 +1239,31 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b) 
		return ret;

	}



	vb->state = VB2_BUF_STATE_PREPARING;

	switch (q->memory) {

	case V4L2_MEMORY_MMAP:

		ret = __qbuf_mmap(vb, b);

		break;

	case V4L2_MEMORY_USERPTR:

		/*

		 * In case of user pointer buffers vb2 allocators need to get direct

		 * access to userspace pages. This requires getting the mmap semaphore

		 * for read access in the current process structure. The same semaphore

		 * is taken before calling mmap operation, while both qbuf/prepare_buf

		 * and mmap are called by the driver or v4l2 core with the driver's lock

		 * held. To avoid an AB-BA deadlock (mmap_sem then driver's lock in mmap

		 * and driver's lock then mmap_sem in qbuf/prepare_buf) the videobuf2

		 * core releases the driver's lock, takes mmap_sem and then takes the

		 * driver's lock again.

		 */

		mmap_sem = &current->mm->mmap_sem;

		call_qop(q, wait_prepare, q);

		down_read(mmap_sem);

		call_qop(q, wait_finish, q);



		ret = __qbuf_userptr(vb, b);



		up_read(mmap_sem);

		break;

	case V4L2_MEMORY_DMABUF:

		ret = __qbuf_dmabuf(vb, b);
@@ -1256,8 +1277,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b) 
		ret = call_qop(q, buf_prepare, vb);

	if (ret)

		dprintk(1, "qbuf: buffer preparation failed: %d\n", ret);

	else

		vb->state = VB2_BUF_STATE_PREPARED;

	vb->state = ret ? VB2_BUF_STATE_DEQUEUED : VB2_BUF_STATE_PREPARED;



	return ret;

}
@@ -1268,80 +1288,47 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b, 
						   struct v4l2_buffer *,

						   struct vb2_buffer *))

{

	struct rw_semaphore *mmap_sem = NULL;

	struct vb2_buffer *vb;

	int ret;



	/*

	 * In case of user pointer buffers vb2 allocators need to get direct

	 * access to userspace pages. This requires getting the mmap semaphore

	 * for read access in the current process structure. The same semaphore

	 * is taken before calling mmap operation, while both qbuf/prepare_buf

	 * and mmap are called by the driver or v4l2 core with the driver's lock

	 * held. To avoid an AB-BA deadlock (mmap_sem then driver's lock in mmap

	 * and driver's lock then mmap_sem in qbuf/prepare_buf) the videobuf2

	 * core releases the driver's lock, takes mmap_sem and then takes the

	 * driver's lock again.

	 *

	 * To avoid racing with other vb2 calls, which might be called after

	 * releasing the driver's lock, this operation is performed at the

	 * beginning of qbuf/prepare_buf processing. This way the queue status

	 * is consistent after getting the driver's lock back.

	 */

	if (q->memory == V4L2_MEMORY_USERPTR) {

		mmap_sem = &current->mm->mmap_sem;

		call_qop(q, wait_prepare, q);

		down_read(mmap_sem);

		call_qop(q, wait_finish, q);

	}



	if (q->fileio) {

		dprintk(1, "%s(): file io in progress\n", opname);

		ret = -EBUSY;

		goto unlock;

		return -EBUSY;

	}



	if (b->type != q->type) {

		dprintk(1, "%s(): invalid buffer type\n", opname);

		ret = -EINVAL;

		goto unlock;

		return -EINVAL;

	}



	if (b->index >= q->num_buffers) {

		dprintk(1, "%s(): buffer index out of range\n", opname);

		ret = -EINVAL;

		goto unlock;

		return -EINVAL;

	}



	vb = q->bufs[b->index];

	if (NULL == vb) {

		/* Should never happen */

		dprintk(1, "%s(): buffer is NULL\n", opname);

		ret = -EINVAL;

		goto unlock;

		return -EINVAL;

	}



	if (b->memory != q->memory) {

		dprintk(1, "%s(): invalid memory type\n", opname);

		ret = -EINVAL;

		goto unlock;

		return -EINVAL;

	}



	ret = __verify_planes_array(vb, b);

	if (ret)

		goto unlock;

		return ret;



	ret = handler(q, b, vb);

	if (ret)

		goto unlock;



	if (!ret) {

		/* Fill buffer information for the userspace */

		__fill_v4l2_buffer(vb, b);



		dprintk(1, "%s() of buffer %d succeeded\n", opname, vb->v4l2_buf.index);

unlock:

	if (mmap_sem)

		up_read(mmap_sem);

	}

	return ret;

}
@@ -1390,6 +1377,9 @@ static int __vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b, 
			return ret;

	case VB2_BUF_STATE_PREPARED:

		break;

	case VB2_BUF_STATE_PREPARING:

		dprintk(1, "qbuf: buffer still being prepared\n");

		return -EINVAL;

	default:

		dprintk(1, "qbuf: buffer already in use\n");

		return -EINVAL;

include/media/videobuf2-core.h

+2 −0
Original line number Diff line number Diff line
@@ -142,6 +142,7 @@ enum vb2_fileio_flags { 
/**

 * enum vb2_buffer_state - current video buffer state

 * @VB2_BUF_STATE_DEQUEUED:	buffer under userspace control

 * @VB2_BUF_STATE_PREPARING:	buffer is being prepared in videobuf

 * @VB2_BUF_STATE_PREPARED:	buffer prepared in videobuf and by the driver

 * @VB2_BUF_STATE_QUEUED:	buffer queued in videobuf, but not in driver

 * @VB2_BUF_STATE_ACTIVE:	buffer queued in driver and possibly used
@@ -154,6 +155,7 @@ enum vb2_fileio_flags { 
 */

enum vb2_buffer_state {

	VB2_BUF_STATE_DEQUEUED,

	VB2_BUF_STATE_PREPARING,

	VB2_BUF_STATE_PREPARED,

	VB2_BUF_STATE_QUEUED,

	VB2_BUF_STATE_ACTIVE,