linux-kernel - Re: [PATCH V14 07/24] mmc: block: Use data timeout in card_busy

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:   Wed, 22 Nov 2017 15:43:07 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     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>,
        Linus Walleij <linus.walleij@...aro.org>,
        Shawn Lin <shawn.lin@...k-chips.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH V14 07/24] mmc: block: Use data timeout in card_busy_detect()

On 22 November 2017 at 08:40, Adrian Hunter <adrian.hunter@...el.com> wrote:
> On 21/11/17 17:39, Ulf Hansson wrote:
>> On 21 November 2017 at 14:42, Adrian Hunter <adrian.hunter@...el.com> wrote:
>>> card_busy_detect() has a 10 minute timeout. However the correct timeout is
>>> the data timeout. Change card_busy_detect() to use the data timeout.
>>
>> Unfortunate I don't think there is "correct" timeout for this case.
>>
>> The data->timeout_ns is to indicate for the host to how long the
>> maximum time it's allowed to take between blocks that are written to
>> the data lines.
>>
>> I haven't found a definition of the busy timeout, after the data write
>> has completed. The spec only mentions that the device moves to
>> programming state and pulls DAT0 to indicate busy.
>
> To me it reads more like the timeout is for each block, including the last
> i.e. the same timeout for "busy".  Note the card is also busy between blocks.

I don't think that is the same timeout. Or maybe it is.

In the eMMC 5.1 spec, there are mentions about "buffer busy signal"
and "programming busy signal", see section 6.15.3 (Timings - Data
Write).

Anyway, whether any of them is specified, is to me unclear.

>
> Equally it is the timeout we give the host controller.  So either the host
> controller does not have a timeout for "busy" - which begs the question why
> it has a timeout at all - or it invents its own "busy" timeout - which begs
> the question why it isn't in the spec.

Well, there are some vague hints in section 6.8.2 (Time-out
conditions), but I don't find these timeouts values being referred to
in 6.15 (Timings). And that puzzles me.

Moreover, the below is quoted from section 6.6.8.1 (Block write):
------
Some Devices may require long and unpredictable times to write a block
of data. After receiving a block of data and completing the CRC check,
the Device will begin writing and hold the DAT0 line low. The host may
poll the status of the Device with a SEND_STATUS command (CMD13) at
any time, and the Device will respond with its status (except in Sleep
state). The status bit READY_FOR_DATA indicates whether the Device can
accept new data or not. The host may deselect the Device by issuing
CMD7 that will then displace the Device into the Disconnect State and
release the DAT0 line without interrupting the write operation. When
reselecting the Device, it will reactivate busy indication by pulling
DAT0 to low. See 6.15 for details of busy indication.
------

>
>>
>> Sure, 10 min seems crazy, perhaps something along the lines of 10-20 s
>> is more reasonable. What do you think?
>
> We give SD cards a generous 3 seconds for writes.  SDHCI has long had a 10
> second software timer for the whole request, which strongly suggests that
> requests have always completed within 10 seconds.  So that puts the range of
> an arbitrary timeout 3-10 s.

>From the reasoning above, I guess we could try out 10 s. That is at
least a lot better than 10 minutes.

I also see that we have at three different places (switch, erase,
block data transfers) implementing busy signal detection. It would be
nice to try to align those pieces of code, because they are quite
similar. Of course, this deserves it's own separate task to try to fix
up.

BTW, perhaps we should move this to a separate change on top of the
series? Or is there as specific need for this in regards to blkmq and
CQE?

Kind regards
Uffe