* [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
@ 2013-12-16 15:06 Dan Carpenter
2013-12-17 17:28 ` Laurent Pinchart
2013-12-19 1:03 ` [patch v2] " Laurent Pinchart
0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2013-12-16 15:06 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman, Laurent Pinchart, Hans Verkuil,
Sergio Aguirre, linux-media, devel, kernel-janitors
The spin_lock_irqsave() macro is not nestable. The second call will
overwrite the first record of "flags" so the IRQs will not be enabled
correctly at the end of the function.
I haven't looked at all the callers but it could be that this function
is always called with interrupts disabled and the bug doesn't cause
problems in real life.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 766491e6a8d0..c9b71c750b15 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
}
if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
- spin_lock_irqsave(&pipe->lock, flags);
+ spin_lock(&pipe->lock);
pipe->state &= ~ISS_PIPELINE_STREAM;
- spin_unlock_irqrestore(&pipe->lock, flags);
+ spin_unlock(&pipe->lock);
}
buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
2013-12-16 15:06 [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next() Dan Carpenter
@ 2013-12-17 17:28 ` Laurent Pinchart
2013-12-19 1:03 ` [patch v2] " Laurent Pinchart
1 sibling, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2013-12-17 17:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Sergio Aguirre, linux-media, devel, kernel-janitors
Hi Dan,
Thank you for the patch.
On Monday 16 December 2013 18:06:12 Dan Carpenter wrote:
> The spin_lock_irqsave() macro is not nestable. The second call will
> overwrite the first record of "flags" so the IRQs will not be enabled
> correctly at the end of the function.
>
> I haven't looked at all the callers but it could be that this function
> is always called with interrupts disabled and the bug doesn't cause
> problems in real life.
The function is called from interrupt handlers only. Would you like to
rephrase the commit message accordingly ?
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index
> 766491e6a8d0..c9b71c750b15 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct
> iss_video *video) }
>
> if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
> - spin_lock_irqsave(&pipe->lock, flags);
> + spin_lock(&pipe->lock);
> pipe->state &= ~ISS_PIPELINE_STREAM;
> - spin_unlock_irqrestore(&pipe->lock, flags);
> + spin_unlock(&pipe->lock);
> }
>
> buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch v2] [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
2013-12-16 15:06 [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next() Dan Carpenter
2013-12-17 17:28 ` Laurent Pinchart
@ 2013-12-19 1:03 ` Laurent Pinchart
1 sibling, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2013-12-19 1:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, devel, Greg Kroah-Hartman, kernel-janitors,
Sergio Aguirre, Hans Verkuil, linux-media
Hi Dan,
Thank you for the patch.
On Wednesday 18 December 2013 10:29:24 Dan Carpenter wrote:
> The spin_lock_irqsave() macro is not nestable. The second call will
> overwrite the first record of "flags" so the IRQs will not be enabled
> correctly at the end of the function.
>
> In the current code, this function is always called from the IRQ handler
> so everything works fine and this fix doesn't change anything.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
and applied to my tree. I'll send a pull request shortly.
> ---
> v2: Updated the change log
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index
> 766491e6a8d0..c9b71c750b15 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct
> iss_video *video) }
>
> if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
> - spin_lock_irqsave(&pipe->lock, flags);
> + spin_lock(&pipe->lock);
> pipe->state &= ~ISS_PIPELINE_STREAM;
> - spin_unlock_irqrestore(&pipe->lock, flags);
> + spin_unlock(&pipe->lock);
> }
>
> buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-12-19 1:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-16 15:06 [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next() Dan Carpenter
2013-12-17 17:28 ` Laurent Pinchart
2013-12-19 1:03 ` [patch v2] " Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox