[PATCH] [media] s2255drv: fix spinlock issue

2014-11-04 Thread Dean Anderson
qlock spinlock controls access to buf_list and sequence.
qlock spinlock should not be locked during a copy to video buffers, an
operation that may sleep.

Signed-off-by: Dean Anderson linux-...@sensoray.com
---
 drivers/media/usb/s2255/s2255drv.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index ccc0009..24c4413 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -558,27 +558,31 @@ static void s2255_fwchunk_complete(struct urb *urb)
 
 }
 
-static int s2255_got_frame(struct s2255_vc *vc, int jpgsize)
+static void s2255_got_frame(struct s2255_vc *vc, int jpgsize)
 {
struct s2255_buffer *buf;
struct s2255_dev *dev = to_s2255_dev(vc-vdev.v4l2_dev);
unsigned long flags = 0;
-   int rc = 0;
+
spin_lock_irqsave(vc-qlock, flags);
if (list_empty(vc-buf_list)) {
dprintk(dev, 1, No active queue to serve\n);
-   rc = -1;
-   goto unlock;
+   spin_unlock_irqrestore(vc-qlock, flags);
+   return;
}
buf = list_entry(vc-buf_list.next,
 struct s2255_buffer, list);
list_del(buf-list);
v4l2_get_timestamp(buf-vb.v4l2_buf.timestamp);
+   buf-vb.v4l2_buf.field = vc-field;
+   buf-vb.v4l2_buf.sequence = vc-frame_count;
+   spin_unlock_irqrestore(vc-qlock, flags);
+
s2255_fillbuff(vc, buf, jpgsize);
+   /* tell v4l buffer was filled */
+   vb2_buffer_done(buf-vb, VB2_BUF_STATE_DONE);
dprintk(dev, 2, %s: [buf] [%p]\n, __func__, buf);
-unlock:
-   spin_unlock_irqrestore(vc-qlock, flags);
-   return rc;
+   return;
 }
 
 static const struct s2255_fmt *format_by_fourcc(int fourcc)
@@ -649,11 +653,6 @@ static void s2255_fillbuff(struct s2255_vc *vc,
}
dprintk(dev, 2, s2255fill at : Buffer 0x%08lx size= %d\n,
(unsigned long)vbuf, pos);
-   /* tell v4l buffer was filled */
-   buf-vb.v4l2_buf.field = vc-field;
-   buf-vb.v4l2_buf.sequence = vc-frame_count;
-   v4l2_get_timestamp(buf-vb.v4l2_buf.timestamp);
-   vb2_buffer_done(buf-vb, VB2_BUF_STATE_DONE);
 }
 
 
-- 
1.9.1

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


Re: [PATCH] [media] s2255drv: fix spinlock issue

2014-11-04 Thread Dean Anderson
This patch is not so urgent, but should still be considered.  Please 
disregard the second comment below, which is not correct.


The original code, however, holds the spinlock for a long time.  From 
the structure of videobuf2, we could just hold qlock during buf_list and 
sequence accesses.



On 2014-11-04 14:34, Dean Anderson wrote:

qlock spinlock controls access to buf_list and sequence.
qlock spinlock should not be locked during a copy to video buffers, an
operation that may sleep.

Signed-off-by: Dean Anderson linux-...@sensoray.com
---
 drivers/media/usb/s2255/s2255drv.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c
b/drivers/media/usb/s2255/s2255drv.c
index ccc0009..24c4413 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -558,27 +558,31 @@ static void s2255_fwchunk_complete(struct urb 
*urb)


 }

-static int s2255_got_frame(struct s2255_vc *vc, int jpgsize)
+static void s2255_got_frame(struct s2255_vc *vc, int jpgsize)
 {
struct s2255_buffer *buf;
struct s2255_dev *dev = to_s2255_dev(vc-vdev.v4l2_dev);
unsigned long flags = 0;
-   int rc = 0;
+
spin_lock_irqsave(vc-qlock, flags);
if (list_empty(vc-buf_list)) {
dprintk(dev, 1, No active queue to serve\n);
-   rc = -1;
-   goto unlock;
+   spin_unlock_irqrestore(vc-qlock, flags);
+   return;
}
buf = list_entry(vc-buf_list.next,
 struct s2255_buffer, list);
list_del(buf-list);
v4l2_get_timestamp(buf-vb.v4l2_buf.timestamp);
+   buf-vb.v4l2_buf.field = vc-field;
+   buf-vb.v4l2_buf.sequence = vc-frame_count;
+   spin_unlock_irqrestore(vc-qlock, flags);
+
s2255_fillbuff(vc, buf, jpgsize);
+   /* tell v4l buffer was filled */
+   vb2_buffer_done(buf-vb, VB2_BUF_STATE_DONE);
dprintk(dev, 2, %s: [buf] [%p]\n, __func__, buf);
-unlock:
-   spin_unlock_irqrestore(vc-qlock, flags);
-   return rc;
+   return;
 }

 static const struct s2255_fmt *format_by_fourcc(int fourcc)
@@ -649,11 +653,6 @@ static void s2255_fillbuff(struct s2255_vc *vc,
}
dprintk(dev, 2, s2255fill at : Buffer 0x%08lx size= %d\n,
(unsigned long)vbuf, pos);
-   /* tell v4l buffer was filled */
-   buf-vb.v4l2_buf.field = vc-field;
-   buf-vb.v4l2_buf.sequence = vc-frame_count;
-   v4l2_get_timestamp(buf-vb.v4l2_buf.timestamp);
-   vb2_buffer_done(buf-vb, VB2_BUF_STATE_DONE);
 }


--
1.9.1

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