On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> Added a vb2_fileio_is_active inline function that returns true if fileio
> is in progress. Check for this too in mmap() (you don't want apps mmap()ing
> buffers used by fileio) and expbuf() (same reason).
>
> In addition drivers should be able to check for this in queue_setup() to
> return an error if an attempt is made to read() or write() with
> V4L2_FIELD_ALTERNATE being configured. This is illegal (there is no way
> to pass the TOP/BOTTOM information around using file I/O).
>
> However, in order to be able to check for this the init_fileio function
> needs to set q->fileio early on, before the buffers are allocated. So switch
> to using internal functions (__reqbufs, vb2_internal_qbuf and
> vb2_internal_streamon) to skip the fileio check. Well, that's why the internal
> functions were created...
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
Acked-by: Pawel Osciak <pawel@xxxxxxxxxx>
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 39 ++++++++++++++++++++------------
> include/media/videobuf2-core.h | 17 ++++++++++++++
> 2 files changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 89147d2..08152dd 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
> * create_bufs is called with count == 0, but count == 0 should still
> * do the memory and type validation.
> */
> - if (q->fileio) {
> + if (vb2_fileio_is_active(q)) {
> dprintk(1, "file io in progress\n");
> return -EBUSY;
> }
> @@ -1617,7 +1617,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
> struct vb2_buffer *vb;
> int ret;
>
> - if (q->fileio) {
> + if (vb2_fileio_is_active(q)) {
> dprintk(1, "file io in progress\n");
> return -EBUSY;
> }
> @@ -1786,7 +1786,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
> */
> int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
> {
> - if (q->fileio) {
> + if (vb2_fileio_is_active(q)) {
> dprintk(1, "file io in progress\n");
> return -EBUSY;
> }
> @@ -2006,7 +2006,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
> */
> int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
> {
> - if (q->fileio) {
> + if (vb2_fileio_is_active(q)) {
> dprintk(1, "file io in progress\n");
> return -EBUSY;
> }
> @@ -2136,7 +2136,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
> */
> int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
> {
> - if (q->fileio) {
> + if (vb2_fileio_is_active(q)) {
> dprintk(1, "file io in progress\n");
> return -EBUSY;
> }
> @@ -2183,7 +2183,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
> */
> int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
> {
> - if (q->fileio) {
> + if (vb2_fileio_is_active(q)) {
> dprintk(1, "file io in progress\n");
> return -EBUSY;
> }
> @@ -2268,6 +2268,11 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
> return -EINVAL;
> }
>
> + if (vb2_fileio_is_active(q)) {
> + dprintk(1, "expbuf: file io in progress\n");
> + return -EBUSY;
> + }
> +
> vb_plane = &vb->planes[eb->plane];
>
> dbuf = call_memop(vb, get_dmabuf, vb_plane->mem_priv, eb->flags & O_ACCMODE);
> @@ -2344,6 +2349,10 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
> return -EINVAL;
> }
> }
> + if (vb2_fileio_is_active(q)) {
> + dprintk(1, "mmap: file io in progress\n");
> + return -EBUSY;
> + }
>
> /*
> * Find the plane corresponding to the offset passed by userspace.
> @@ -2455,7 +2464,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
> /*
> * Start file I/O emulator only if streaming API has not been used yet.
> */
> - if (q->num_buffers == 0 && q->fileio == NULL) {
> + if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) {
> if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
> (req_events & (POLLIN | POLLRDNORM))) {
> if (__vb2_init_fileio(q, 1))
> @@ -2660,7 +2669,8 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
> fileio->req.count = count;
> fileio->req.memory = V4L2_MEMORY_MMAP;
> fileio->req.type = q->type;
> - ret = vb2_reqbufs(q, &fileio->req);
> + q->fileio = fileio;
> + ret = __reqbufs(q, &fileio->req);
> if (ret)
> goto err_kfree;
>
> @@ -2698,7 +2708,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
> b->type = q->type;
> b->memory = q->memory;
> b->index = i;
> - ret = vb2_qbuf(q, b);
> + ret = vb2_internal_qbuf(q, b);
> if (ret)
> goto err_reqbufs;
> fileio->bufs[i].queued = 1;
> @@ -2714,19 +2724,18 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
> /*
> * Start streaming.
> */
> - ret = vb2_streamon(q, q->type);
> + ret = vb2_internal_streamon(q, q->type);
> if (ret)
> goto err_reqbufs;
>
> - q->fileio = fileio;
> -
> return ret;
>
> err_reqbufs:
> fileio->req.count = 0;
> - vb2_reqbufs(q, &fileio->req);
> + __reqbufs(q, &fileio->req);
>
> err_kfree:
> + q->fileio = NULL;
> kfree(fileio);
> return ret;
> }
> @@ -2779,7 +2788,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
> /*
> * Initialize emulator on first call.
> */
> - if (!q->fileio) {
> + if (!vb2_fileio_is_active(q)) {
> ret = __vb2_init_fileio(q, read);
> dprintk(3, "vb2_init_fileio result: %d\n", ret);
> if (ret)
> @@ -3147,7 +3156,7 @@ unsigned int vb2_fop_poll(struct file *file, poll_table *wait)
>
> /* Try to be smart: only lock if polling might start fileio,
> otherwise locking will only introduce unwanted delays. */
> - if (q->num_buffers == 0 && q->fileio == NULL) {
> + if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) {
> if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
> (req_events & (POLLIN | POLLRDNORM)))
> must_lock = true;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 3b57851..af34ae0 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -472,6 +472,23 @@ static inline bool vb2_is_streaming(struct vb2_queue *q)
> }
>
> /**
> + * vb2_fileio_is_active() - return true if fileio is active.
> + * @q: videobuf queue
> + *
> + * This returns true if read() or write() is used to stream the data
> + * as opposed to stream I/O. This is almost never an important distinction,
> + * except in rare cases. One such case is that using read() or write() to
> + * stream a format using V4L2_FIELD_ALTERNATE is not allowed since there
> + * is no way you can pass the field information of each buffer to/from
> + * userspace. A driver that supports this field format should check for
> + * this in the queue_setup op and reject it if this function returns true.
> + */
> +static inline bool vb2_fileio_is_active(struct vb2_queue *q)
> +{
> + return q->fileio;
> +}
> +
> +/**
> * vb2_is_busy() - return busy status of the queue
> * @q: videobuf queue
> *
> --
> 1.9.1
>
--
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
