diff mbox

[RFC,RESEND,11/11] vb2: dma-contig: Add WARN_ON_ONCE() to check for potential bugs

Message ID 1441972234-8643-12-git-send-email-sakari.ailus@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sakari Ailus Sept. 11, 2015, 11:50 a.m. UTC
The scatterlist should always be present when the cache would need to be
flushed. Each buffer type has its own means to provide that. Add
WARN_ON_ONCE() to check the scatterist exists.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Laurent Pinchart Dec. 15, 2016, 9:57 p.m. UTC | #1
Hi Sakari,

Thank you for the patch.

On Friday 11 Sep 2015 14:50:34 Sakari Ailus wrote:
> The scatterlist should always be present when the cache would need to be
> flushed. Each buffer type has its own means to provide that. Add
> WARN_ON_ONCE() to check the scatterist exists.

Do you think such a check is really needed ? Have you run into this before ?

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 65ee122..58c35c5
> 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -145,6 +145,9 @@ static void vb2_dc_prepare(void *buf_priv)
>  	    !dma_get_attr(DMA_ATTR_NON_CONSISTENT, buf->attrs))
>  		return;
> 
> +	if (WARN_ON_ONCE(!sgt))
> +		return;
> +
>  	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  }
> 
> @@ -161,6 +164,9 @@ static void vb2_dc_finish(void *buf_priv)
>  	    !dma_get_attr(DMA_ATTR_NON_CONSISTENT, buf->attrs))
>  		return;
> 
> +	if (WARN_ON_ONCE(!sgt))
> +		return;
> +
>  	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  }
Sakari Ailus Dec. 17, 2016, 12:50 a.m. UTC | #2
On Thu, Dec 15, 2016 at 11:57:54PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Friday 11 Sep 2015 14:50:34 Sakari Ailus wrote:
> > The scatterlist should always be present when the cache would need to be
> > flushed. Each buffer type has its own means to provide that. Add
> > WARN_ON_ONCE() to check the scatterist exists.
> 
> Do you think such a check is really needed ? Have you run into this before ?

I think I may have, but the reason was that the code is non-trivial and
letting the user know what went wrong and where is nice. I guess this one
could be dropped, too.

> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/v4l2-core/videobuf2-dma-contig.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 65ee122..58c35c5
> > 100644
> > --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > @@ -145,6 +145,9 @@ static void vb2_dc_prepare(void *buf_priv)
> >  	    !dma_get_attr(DMA_ATTR_NON_CONSISTENT, buf->attrs))
> >  		return;
> > 
> > +	if (WARN_ON_ONCE(!sgt))
> > +		return;
> > +
> >  	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> >  }
> > 
> > @@ -161,6 +164,9 @@ static void vb2_dc_finish(void *buf_priv)
> >  	    !dma_get_attr(DMA_ATTR_NON_CONSISTENT, buf->attrs))
> >  		return;
> > 
> > +	if (WARN_ON_ONCE(!sgt))
> > +		return;
> > +
> >  	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> >  }
>
diff mbox

Patch

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 65ee122..58c35c5 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -145,6 +145,9 @@  static void vb2_dc_prepare(void *buf_priv)
 	    !dma_get_attr(DMA_ATTR_NON_CONSISTENT, buf->attrs))
 		return;
 
+	if (WARN_ON_ONCE(!sgt))
+		return;
+
 	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 }
 
@@ -161,6 +164,9 @@  static void vb2_dc_finish(void *buf_priv)
 	    !dma_get_attr(DMA_ATTR_NON_CONSISTENT, buf->attrs))
 		return;
 
+	if (WARN_ON_ONCE(!sgt))
+		return;
+
 	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 }