linux-kernel - Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave

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: <13dcf3d9-06ca-d793-525d-12f6d7cd27c1@ti.com>
Date:   Wed, 5 Feb 2020 10:10:22 +0200
From:   Peter Ujfalusi <peter.ujfalusi@...com>
To:     Vinod Koul <vkoul@...nel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>
CC:     dmaengine <dmaengine@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH 0/3] dmaengine: Stear users towards
 dma_request_slave_chan()

Vinod,

On 05/02/2020 6.43, Vinod Koul wrote:
> On 04-02-20, 13:21, Andy Shevchenko wrote:
>> On Tue, Feb 4, 2020 at 8:21 AM Vinod Koul <vkoul@...nel.org> wrote:
>>>
>>> On 03-02-20, 12:37, Andy Shevchenko wrote:
>>>> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@...com> wrote:
>>>>
>>>>> dma_request_slave_channel_reason() no longer have user in mainline, it
>>>>> can be removed.
>>>>>
>>>>> Advise users of dma_request_slave_channel() and
>>>>> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
>>>>
>>>> How? There are legacy ARM boards you have to care / remove before.
>>>> DMAengine subsystem makes a p*s off decisions without taking care of
>>>> (I'm talking now about dma release callback, for example) end users.
>>>
>>> Can you elaborate issue you are seeing with dma_release callback?
>>
>>
>> [    7.980381] intel-lpss 0000:00:1e.3: WARN: Device release is not
>> defined so it is not safe to unbind this driver while in use
> 
> Yes that is expected but is not valid in your case.

In which case it is valid?

> Anyway this will be turned off before the release.

Looking at the commit which added it and I still don't get the point.
If any of the channel is in use then we should not allow the DMA driver
to go away at all.
Imho there should be a function to check if we can proceed with the
.remove of the driver and fail it if any of the channels are in use.

Hrm, base/dd.c __device_release_driver() does not check the .remove's
return value, so it can not fail.

What is expected if the .remove returns with OK but we still have
channels in use?

After the remove all sorts of things got yanked which might makes the
still in use channels cause issues down the road.

I'm curious why it is a good thing to remotely try to support unbind
when the driver is in use.
It is like one wants to support ext4 removal even when your rootfs is ext4.

I think krefing the DMA driver for channel request/release is just fine,
if user wants to break the system we should not assist...

>> It's not limited to that driver, but actually all I'm maintaining.
>>
>> Users are not happy!
>>
>> -- 
>> With Best Regards,
>> Andy Shevchenko
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki