Skip to contentSkip to conversation

davinci: vpif: missing unlocks on error

PATCH Superseded Dan Carpenter 3 participants 6 messages Jun 11, 2014 – Jun 16, 2014

Subsystems (2)
Media (V4L/DVB) Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 davinci: vpif: missing unlocks on error

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

Signed-off-by Dan Carpenter <[email protected]>

---
Please review this one carefully. I don't know if the unlock should go before or after the list_for_each_entry_safe() loop.

drivers/media/platform/davinci/vpif_capture.c +2 -0
index a7ed164..2c08fbd 100644
@@ -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);
drivers/media/platform/davinci/vpif_display.c +2 -0
index 5bb085b..b7b2bdf 100644
@@ -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);
#1 lore raw

Hi Dan,

Thanks for the patch.

Quoted 8 lines · Dan Carpenter

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

Signed-off-by Dan Carpenter <[email protected]>

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

Quoted 26 lines
drivers/media/platform/davinci/vpif_capture.c +2 -0
index a7ed164..2c08fbd 100644
@@ -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);
drivers/media/platform/davinci/vpif_display.c +2 -0
index 5bb085b..b7b2bdf 100644
@@ -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);
author
#2 lore raw

PATCH v2 1/1 davinci: vpif: missing unlocks on error

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

Signed-off-by Dan Carpenter <[email protected]>

---
v2: move the unlock so the list_for_each_entry_safe() loop is protected

drivers/media/platform/davinci/vpif_capture.c +1 -0
index a7ed164..1e4ec69 100644
@@ -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;
 }
drivers/media/platform/davinci/vpif_display.c +1 -0
index 5bb085b..b431b58 100644
@@ -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;
 }
Applied
#3 lore raw
Quoted 4 lines · Dan Carpenter

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

Signed-off-by Dan Carpenter <[email protected]>

Applied!

Thanks,
--Prabhakar Lad

Quoted 27 lines

---
v2: move the unlock so the list_for_each_entry_safe() loop is protected

drivers/media/platform/davinci/vpif_capture.c +1 -0
index a7ed164..1e4ec69 100644
@@ -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;
 }
drivers/media/platform/davinci/vpif_display.c +1 -0
index 5bb085b..b431b58 100644
@@ -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;
 }
#4 lore raw

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

Quoted 43 lines · Prabhakar Lad
Quoted 4 lines · Dan Carpenter

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

Signed-off-by Dan Carpenter <[email protected]>

Applied!

Thanks,
--Prabhakar Lad

Quoted 20 lines

---
v2: move the unlock so the list_for_each_entry_safe() loop is protected

drivers/media/platform/davinci/vpif_capture.c +1 -0
index a7ed164..1e4ec69 100644
@@ -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;
 }
drivers/media/platform/davinci/vpif_display.c +0 -0
index 5bb085b..b431b58 100644

drivers/media/platform/davinci/vpif_display.c err: · line 239

Quoted 7 lines
drivers/media/platform/davinci/vpif_display.c +1 -0
@@ -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;
 }

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

#5 lore raw

Hi Hans,

Quoted 5 lines · 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

Quoted 49 lines

Regards,

Hans

Quoted 43 lines · Prabhakar Lad
Quoted 4 lines · Dan Carpenter

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

Signed-off-by Dan Carpenter <[email protected]>

Applied!

Thanks,
--Prabhakar Lad

Quoted 20 lines

---
v2: move the unlock so the list_for_each_entry_safe() loop is protected

drivers/media/platform/davinci/vpif_capture.c +1 -0
index a7ed164..1e4ec69 100644
@@ -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;
 }
drivers/media/platform/davinci/vpif_display.c +0 -0
index 5bb085b..b431b58 100644

drivers/media/platform/davinci/vpif_display.c err: · line 239

Quoted 7 lines
drivers/media/platform/davinci/vpif_display.c +1 -0
@@ -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;
 }

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