[patch] [media] davinci: vpif: missing unlocks on error

Discussion:

Dan Carpenter

2014-06-11 07:31:08 UTC

We recently changed some locking around so we need some new unlocks on
the error paths.

Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
---
Please review this one carefully. I don't know if the unlock should go
before or after the list_for_each_entry_safe() loop.

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index a7ed164..2c08fbd 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -265,6 +265,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
return 0;

err:
+ spin_unlock_irqrestore(&common->irqlock, flags);
+
list_for_each_entry_safe(buf, tmp, &common->dma_queue, list) {
list_del(&buf->list);
vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 5bb085b..b7b2bdf 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -229,6 +229,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
return 0;

err:
+ spin_unlock_irqrestore(&common->irqlock, flags);
+
list_for_each_entry_safe(buf, tmp, &common->dma_queue, list) {
list_del(&buf->list);
vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);

Prabhakar Lad

2014-06-11 09:44:20 UTC

Permalink

Hi Dan,

Thanks for the patch.

Post by Dan Carpenter
We recently changed some locking around so we need some new unlocks on
the error paths.
Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
---
Please review this one carefully. I don't know if the unlock should go
before or after the list_for_each_entry_safe() loop.

Yes the unlock should go after the list_for_each_entry_safe() loop
please respin another version fixing it.

Thanks,
--Prabhakar Lad

Post by Dan Carpenter
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index a7ed164..2c08fbd 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -265,6 +265,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
return 0;
+ spin_unlock_irqrestore(&common->irqlock, flags);
+
list_for_each_entry_safe(buf, tmp, &common->dma_queue, list) {
list_del(&buf->list);
vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 5bb085b..b7b2bdf 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -229,6 +229,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
return 0;
+ spin_unlock_irqrestore(&common->irqlock, flags);
+
list_for_each_entry_safe(buf, tmp, &common->dma_queue, list) {
list_del(&buf->list);
vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);

Dan Carpenter

2014-06-12 07:01:45 UTC

Permalink

We recently changed some locking around so we need some new unlocks on
the error paths.

Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
---
v2: move the unlock so the list_for_each_entry_safe() loop is protected

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index a7ed164..1e4ec69 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -269,6 +269,7 @@ err:
list_del(&buf->list);
vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);
}
+ spin_unlock_irqrestore(&common->irqlock, flags);

return ret;
}
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 5bb085b..b431b58 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -233,6 +233,7 @@ err:
list_del(&buf->list);
vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);
}
+ spin_unlock_irqrestore(&common->irqlock, flags);

return ret;
}

Prabhakar Lad

2014-06-13 18:13:44 UTC

Permalink

Post by Dan Carpenter
We recently changed some locking around so we need some new unlocks on
the error paths.
Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>

Applied!

Thanks,
--Prabhakar Lad

Post by Dan Carpenter
---
v2: move the unlock so the list_for_each_entry_safe() loop is protected
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index a7ed164..1e4ec69 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
list_del(&buf->list);
vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);
}
+ spin_unlock_irqrestore(&common->irqlock, flags);
return ret;
}
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 5bb085b..b431b58 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
list_del(&buf->list);
vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);
}
+ spin_unlock_irqrestore(&common->irqlock, flags);
return ret;
}

Hans Verkuil

2014-06-16 09:48:31 UTC

Permalink

Prabhakar,

Are you going to make a pull request for this, or shall I take it? Should it be applied
to 3.16?

Regards,

Hans

Post by Prabhakar Lad

Post by Dan Carpenter
We recently changed some locking around so we need some new unlocks on
the error paths.
Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>

Applied!
Thanks,
--Prabhakar Lad

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Prabhakar Lad

2014-06-16 11:07:44 UTC

Permalink

Hi Hans,

Post by Hans Verkuil
Prabhakar,
Are you going to make a pull request for this, or shall I take it? Should it be applied
to 3.16?

As this is not a critical bug, I was planning to wait for v3.17 as
v3.16 is almost closed.

Regards,
--Prabhakar Lad

Post by Hans Verkuil
Regards,
Hans

Post by Prabhakar Lad

Post by Dan Carpenter
We recently changed some locking around so we need some new unlocks on
the error paths.
Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>

Applied!
Thanks,
--Prabhakar Lad

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html