Re: [PATCH v3] imx-sdma: fix pagefault when channel is disabled during interrupt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 27, 2018 at 04:14:38PM +0100, Thierry Bultel wrote:
> Add a spinlock and an 'enabled' boolean on channel descriptor, to avoid
> using buffer descriptors in the interrupt context,
> when sdma_disable_channel is called meanwhile.
> 
> Signed-off-by: Thierry Bultel <tbultel@xxxxxxxxxxxxxxx>
> ---
> 
> Changes since V2:
>  * Applied Vinod Koul's review about empty lines
> 
> Changes since V1:
>  * Applied Shunyong Yang's review advice:
>  * Use a bool type for the enabled field
>  * Fixed the race condition
> 
>  drivers/dma/imx-sdma.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index d1651a5..0df6d54 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -330,6 +330,7 @@ struct sdma_channel {
>  	unsigned int			chn_real_count;
>  	struct tasklet_struct		tasklet;
>  	struct imx_dma_data		data;
> +	bool				enabled;
>  };
>  
>  #define IMX_DMA_SG_LOOP		BIT(0)
> @@ -588,7 +589,13 @@ static int sdma_config_ownership(struct sdma_channel *sdmac,
>  
>  static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
>  {
> +	unsigned long flags;
> +	struct sdma_channel *sdmac = &sdma->channel[channel];

we need an empty line here too...

>  	writel(BIT(channel), sdma->regs + SDMA_H_START);
> +
> +	spin_lock_irqsave(&sdmac->lock, flags);
> +	sdmac->enabled = 1;
> +	spin_unlock_irqrestore(&sdmac->lock, flags);
>  }
>  
>  /*
> @@ -677,6 +684,14 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
>  	struct sdma_buffer_descriptor *bd;
>  	int error = 0;
>  	enum dma_status	old_status = sdmac->status;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdmac->lock, flags);
> +	if (!sdmac->enabled) {
> +		spin_unlock_irqrestore(&sdmac->lock, flags);
> +		return;
> +	}
> +	spin_unlock_irqrestore(&sdmac->lock, flags);
>  
>  	/*
>  	 * loop mode. Iterate over descriptors, re-setup them and
> @@ -930,10 +945,15 @@ static int sdma_disable_channel(struct dma_chan *chan)
>  	struct sdma_channel *sdmac = to_sdma_chan(chan);
>  	struct sdma_engine *sdma = sdmac->sdma;
>  	int channel = sdmac->channel;
> +	unsigned long flags;
>  
>  	writel_relaxed(BIT(channel), sdma->regs + SDMA_H_STATSTOP);
>  	sdmac->status = DMA_ERROR;
>  
> +	spin_lock_irqsave(&sdmac->lock, flags);
> +	sdmac->enabled = 0;
> +	spin_unlock_irqrestore(&sdmac->lock, flags);
> +
>  	return 0;
>  }
>  
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux