| Message ID | 20131216150612.GA15506@elgon.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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,
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,
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,
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> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html