linux-kernel - Re: [PATCH] dmaengine: pl330: initialize tasklet after spin_unlock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56C6CF16.9000908@metafoo.de>
Date:	Fri, 19 Feb 2016 09:15:18 +0100
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Anand Moon <linux.amoon@...il.com>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>
Cc:	Vinod Koul <vinod.koul@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	dmaengine@...r.kernel.org,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] dmaengine: pl330: initialize tasklet after
 spin_unlock_irqrestore

On 02/19/2016 09:10 AM, Anand Moon wrote:
> Hi Krzysztof,
> 
> On 19 February 2016 at 12:50, Krzysztof Kozlowski
> <k.kozlowski@...sung.com> wrote:
>> On 19.02.2016 15:39, Anand Moon wrote:
>>> Hi Krzysztof,
>>>
>>> On 19 February 2016 at 11:36, Krzysztof Kozlowski
>>> <k.kozlowski@...sung.com> wrote:
>>>> 2016-02-19 2:21 GMT+09:00 Anand Moon <linux.amoon@...il.com>:
>>>>> From: Anand Moon <linux.amoon@...il.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