Hi Krzysztof, On 19 February 2016 at 13:45, Lars-Peter Clausen <lars@xxxxxxxxxx> wrote: > On 02/19/2016 09:10 AM, Anand Moon wrote: >> 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. > > Hi, > > tasklet_init() does not spwan the tasklet function, tasklet_schedule() does > that. > > But there is still room for optimization here. If you want to move the > tasklet_init() call please move it into pl330_probe() next to where the > channel is allocated. There is no need to re-initialize the tasklet each > time the channel is requested. > > - Lars > After looking at the history of the change logs. I found below changes. commit da331ba8e9c5de72a27e50f71105395bba6eebe0 Author: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> Date: Wed Jul 3 15:00:43 2013 -0700 drivers/dma/pl330.c: fix locking in pl330_free_chan_resources() tasklet_kill() may sleep so call it before taking pch->lock. --------------------------------------------- sorry for the noise. Next time I will be more careful. -Anand Moon -- 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
