Re: [REVIEWv2 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often

2014-04-09 Thread Pawel Osciak
On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 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.verk...@cisco.com

Acked-by: Pawel Osciak pa...@osciak.com

 ---
  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 = 

[REVIEWv2 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often

2014-04-07 Thread Hans Verkuil
From: Hans Verkuil hans.verk...@cisco.com

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.verk...@cisco.com
---
 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