linux-kernel - Re: [PATCH V13 07/10] mmc: block: blk-mq: Add support for direct completion

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]

Date:   Thu, 9 Nov 2017 13:34:16 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     Ulf Hansson <ulf.hansson@...aro.org>,
        linux-mmc <linux-mmc@...r.kernel.org>,
        linux-block <linux-block@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Bough Chen <haibo.chen@....com>,
        Alex Lemberg <alex.lemberg@...disk.com>,
        Mateusz Nowak <mateusz.nowak@...el.com>,
        Yuliy Izrailov <Yuliy.Izrailov@...disk.com>,
        Jaehoon Chung <jh80.chung@...sung.com>,
        Dong Aisheng <dongas86@...il.com>,
        Das Asutosh <asutoshd@...eaurora.org>,
        Zhangfei Gao <zhangfei.gao@...il.com>,
        Sahitya Tummala <stummala@...eaurora.org>,
        Harjani Ritesh <riteshh@...eaurora.org>,
        Venu Byravarasu <vbyravarasu@...dia.com>,
        Shawn Lin <shawn.lin@...k-chips.com>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH V13 07/10] mmc: block: blk-mq: Add support for direct completion

On Thu, Nov 9, 2017 at 8:27 AM, Adrian Hunter <adrian.hunter@...el.com> wrote:
> On 08/11/17 11:28, Linus Walleij wrote:
>> On Fri, Nov 3, 2017 at 2:20 PM, Adrian Hunter <adrian.hunter@...el.com> wrote:
>>
>>> For blk-mq, add support for completing requests directly in the ->done
>>> callback. That means that error handling and urgent background operations
>>> must be handled by recovery_work in that case.
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
>>
>> I tried enabling this on my MMC host (mmci) but I got weird
>> DMA error messages when I did.
>>
>> I guess this has not been tested on a non-DMA-coherent
>> system?
>
> I don't see what DMA-coherence has to do with anything.
>
> Possibilities:
>         - DMA unmapping doesn't work in an atomic context
>         - requests' DMA operations have to be synchronized with each other

So since MMCI need the post_req() hook called with
an error code to properly tear down any DMA operations,
I was worried that maybe your error path is not doing this
(passing an error code or calling in the right order).

I had a bunch of fallouts in my own patch set relating
to that.

>> I think I might be seeing this because the .pre and .post
>> callbacks need to be strictly sequenced, and this is
>> maybe not taken into account here?
>
> I looked at mmci but that did not seem to be the case.
>
>> Isn't there as risk
>> that the .post callback of the next request is called before
>> the .post callback of the previous request has returned
>> for example?
>
> Of course, the requests are treated as independent.  If the separate DMA
> operations require synchronization, that is for the host driver to fix.

They are treated as independent by the block layer but
it is the subsystems duty to serialize them for the hardware,

MMCI strictly requires that pre/post hooks per request
happen in the right order, so if you have prepared a second
request after submitting the first, and the first fails, you have
to back out by unpreparing the second one before unpreparing
the first. It is also the only host driver requireing to be passed
an error code in the last parameter to the post hook in
order to work properly.

I think your patch set handles that nicely though, because I
haven't seen any errors, it's just when we do this direct
completion I see problems.

Yours,
Linus Walleij