RE: [PATCH 2/2] dma/ep93xx_dma: prevent ep93xx_dma_tasklet() toaccess empty list

[H Hartley Sweeten]

On Tuesday, November 22, 2011 12:46 PM, Mika Westerberg wrote:
> From: Rafal Prylowski <prylowski@xxxxxxxxxxx>
>
> If dma_terminate_all() is called before the ep93xx_dma_tasklet() gets to run,
> it tries to access an empty ->active list causing following OOPS:

[snip]

> As we expect that the ->active list is never empty when the ep93xx_dma_tasklet()
> is called, we fix this by adding a new flag per channel EP93XX_DMA_IS_RUNNING
> which determines whether the channel is running or not.
> 
> We also add BUG_ON() to ep93xx_dma_get_active() to make sure that similar
> problems will be caught early.
> 
> Signed-off-by: Rafal Prylowski <prylowski@xxxxxxxxxxx>
> [added a flag instead of just checking for empty list]
> Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxx>
> ---
>  drivers/dma/ep93xx_dma.c |   25 ++++++++++++++++---------
>  1 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
> index 6181811..cc3a302 100644
> --- a/drivers/dma/ep93xx_dma.c
> +++ b/drivers/dma/ep93xx_dma.c
> @@ -155,6 +155,8 @@ struct ep93xx_dma_chan {
>  	unsigned long			flags;
>  /* Channel is configured for cyclic transfers */
>  #define EP93XX_DMA_IS_CYCLIC		0
> +/* Channel is enabled */
> +#define EP93XX_DMA_IS_RUNNING		1
>  
>  	int				buffer;
>  	dma_cookie_t			last_completed;
> @@ -246,6 +248,7 @@ static void ep93xx_dma_set_active(struct ep93xx_dma_chan *edmac,
>  static struct ep93xx_dma_desc *
>  ep93xx_dma_get_active(struct ep93xx_dma_chan *edmac)
>  {
> +	BUG_ON(list_empty(&edmac->active));
>  	return list_first_entry(&edmac->active, struct ep93xx_dma_desc, node);

Mika,

Thanks for looking into this.

I still don't like the BUG_ON here.  Is it even possible to get here with
an empty list now that your catching it in the tasklet?

But, ep93xx_dma_set_active() also has a BUG_ON test so...

Acked-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/