Hi Krzysztof, On 19 February 2016 at 12:50, Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx> wrote: > 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 > On SMP arch, tasklet_init could spawn the pl330_tasklet routine, it could be any CPU which could take up this task. So just for good timing of Initialization of the pl330_tasklet after spin_unlock_irqrestore. That is what I can figure out. My choice of words could be a problem. Best regards, -Anand Moon >> >>>> >>>> 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 >> >> > -- 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
