Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1337996

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

Show all headers | View raw

hi Lars-Peter,

On 19 February 2016 at 13:45, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 02/19/2016 09:10 AM, Anand Moon wrote:
>> Hi Krzysztof,
>>
>> On 19 February 2016 at 12:50, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> On 19.02.2016 15:39, Anand Moon wrote:
>>>> Hi Krzysztof,
>>>>
>>>> On 19 February 2016 at 11:36, Krzysztof Kozlowski
>>>> <k.kozlowski@samsung.com> wrote:
>>>>> 2016-02-19 2:21 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>>>> From: Anand Moon <linux.amoon@gmail.com>
>>>>>>
>>>>>> 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
>

Thanks for clearing my miss-concept.

Best Regards.
-Anand moon

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] dmaengine: pl330: initialize tasklet after spin_unlock_irqrestore Anand Moon <linux.amoon@gmail.com> - 2016-02-18 18:30 +0100
  Re: [PATCH] dmaengine: pl330: initialize tasklet after spin_unlock_irqrestore Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-02-19 07:10 +0100
    Re: [PATCH] dmaengine: pl330: initialize tasklet after spin_unlock_irqrestore Anand Moon <linux.amoon@gmail.com> - 2016-02-19 07:50 +0100
      Re: [PATCH] dmaengine: pl330: initialize tasklet after  spin_unlock_irqrestore Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-02-19 08:30 +0100
        Re: [PATCH] dmaengine: pl330: initialize tasklet after  spin_unlock_irqrestore Lars-Peter Clausen <lars@metafoo.de> - 2016-02-19 09:20 +0100
          Re: [PATCH] dmaengine: pl330: initialize tasklet after spin_unlock_irqrestore Anand Moon <linux.amoon@gmail.com> - 2016-02-19 09:50 +0100
          Re: [PATCH] dmaengine: pl330: initialize tasklet after spin_unlock_irqrestore Anand Moon <linux.amoon@gmail.com> - 2016-02-21 18:40 +0100
        Re: [PATCH] dmaengine: pl330: initialize tasklet after spin_unlock_irqrestore Anand Moon <linux.amoon@gmail.com> - 2016-02-19 09:20 +0100

csiph-web