Re: [PATCH] dmaengine: pl330: initialize tasklet after spin_unlock_irqrestore

[Krzysztof Kozlowski]

On 19.02.2016 15:39, Anand Moon wrote:
> Hi Krzysztof,
> 
> On 19 February 2016 at 11:36, Krzysztof Kozlowski
> <k.kozlowski@xxxxxxxxxxx> wrote:
>> 2016-02-19 2:21 GMT+09:00 Anand Moon <linux.amoon@xxxxxxxxx>:
>>> From: Anand Moon <linux.amoon@xxxxxxxxx>
>>>
>>> pl330_tasklet tasklet uses the same spinlock pch->lock for safe IRQ locking.
>>> It's safe to initialize pl330_tasklet tasklet after release of the locking.
>>
>> This is tasklet init, not tasklet execution (which you are referring
>> to in first sentence). I don't get how usage of spinlock during
>> execution guarantees the safeness during init... Please describe why
>> this is safe.
>>
>> Best regards,
>> Krzysztof
>>
> 
> http://lxr.free-electrons.com/source/drivers/dma/pl330.c#L1972
> 
> pl330_tasklet function which is initiated by tasklet_init is trying to lock
> using same spin_unlock_irqsave/restore pch->lock.

tasklet_init does not call pl330_tasklet (if this is what you mean by
"initiated"). What is the correlation? Why are you referring to the
locks in pl330_tasklet?

> So better release the pch->lock and then initialize  the tasklet_init.

Why "better"?

Best regards,
Krzysztof

> 
>>>
>>> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx>
>>> ---
>>>  drivers/dma/pl330.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>>> index 17ee758..df2cab1 100644
>>> --- a/drivers/dma/pl330.c
>>> +++ b/drivers/dma/pl330.c
>>> @@ -2091,10 +2091,10 @@ static int pl330_alloc_chan_resources(struct dma_chan *chan)
>>>                 return -ENOMEM;
>>>         }
>>>
>>> -       tasklet_init(&pch->task, pl330_tasklet, (unsigned long) pch);
>>> -
>>>         spin_unlock_irqrestore(&pch->lock, flags);
>>>
>>> +       tasklet_init(&pch->task, pl330_tasklet, (unsigned long) pch);
>>> +
>>>         return 1;
>>>  }
>>>
>>> --
>>> 1.9.1
>>>
>>> --
>>> 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
> 
>