[v5,05/14] dmaengine: tegra-apb: Prevent race conditions of tasklet vs free list
diff mbox series

Message ID 20200123230325.3037-6-digetx@gmail.com
State Changes Requested
Headers show
Series
  • NVIDIA Tegra APB DMA driver fixes and improvements
Related show

Commit Message

Dmitry Osipenko Jan. 23, 2020, 11:03 p.m. UTC
The interrupt handler puts a half-completed DMA descriptor on a free list
and then schedules tasklet to process bottom half of the descriptor that
executes client's callback, this creates possibility to pick up the busy
descriptor from the free list. Thus let's disallow descriptor's re-use
until it is fully processed.

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/dma/tegra20-apb-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jon Hunter Jan. 29, 2020, 11:01 a.m. UTC | #1
On 23/01/2020 23:03, Dmitry Osipenko wrote:
> The interrupt handler puts a half-completed DMA descriptor on a free list
> and then schedules tasklet to process bottom half of the descriptor that
> executes client's callback, this creates possibility to pick up the busy
> descriptor from the free list. Thus let's disallow descriptor's re-use
> until it is fully processed.
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/dma/tegra20-apb-dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 1b8a11804962..aafad50d075e 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -281,7 +281,7 @@ static struct tegra_dma_desc *tegra_dma_desc_get(
>  
>  	/* Do not allocate if desc are waiting for ack */
>  	list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) {
> -		if (async_tx_test_ack(&dma_desc->txd)) {
> +		if (async_tx_test_ack(&dma_desc->txd) && !dma_desc->cb_count) {
>  			list_del(&dma_desc->node);
>  			spin_unlock_irqrestore(&tdc->lock, flags);
>  			dma_desc->txd.flags = 0;
> 

I think we should mark this for stable as well. I would make this the
2nd patch in the series as it is related to #1.

Jon
Dmitry Osipenko Jan. 29, 2020, 2:39 p.m. UTC | #2
29.01.2020 14:01, Jon Hunter пишет:
> 
> On 23/01/2020 23:03, Dmitry Osipenko wrote:
>> The interrupt handler puts a half-completed DMA descriptor on a free list
>> and then schedules tasklet to process bottom half of the descriptor that
>> executes client's callback, this creates possibility to pick up the busy
>> descriptor from the free list. Thus let's disallow descriptor's re-use
>> until it is fully processed.
>>
>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/dma/tegra20-apb-dma.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>> index 1b8a11804962..aafad50d075e 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
>> @@ -281,7 +281,7 @@ static struct tegra_dma_desc *tegra_dma_desc_get(
>>  
>>  	/* Do not allocate if desc are waiting for ack */
>>  	list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) {
>> -		if (async_tx_test_ack(&dma_desc->txd)) {
>> +		if (async_tx_test_ack(&dma_desc->txd) && !dma_desc->cb_count) {
>>  			list_del(&dma_desc->node);
>>  			spin_unlock_irqrestore(&tdc->lock, flags);
>>  			dma_desc->txd.flags = 0;
>>
> 
> I think we should mark this for stable as well. I would make this the
> 2nd patch in the series as it is related to #1.

Okay!

Patch
diff mbox series

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 1b8a11804962..aafad50d075e 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -281,7 +281,7 @@  static struct tegra_dma_desc *tegra_dma_desc_get(
 
 	/* Do not allocate if desc are waiting for ack */
 	list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) {
-		if (async_tx_test_ack(&dma_desc->txd)) {
+		if (async_tx_test_ack(&dma_desc->txd) && !dma_desc->cb_count) {
 			list_del(&dma_desc->node);
 			spin_unlock_irqrestore(&tdc->lock, flags);
 			dma_desc->txd.flags = 0;