linux-kernel - Re: [PATCH] mtd: revert "spi-nor: intel: provide a range for poll

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: <ca4d6a2f-b3c8-2b52-431e-81f3cb956db7@nokia.com>
Date:   Thu, 23 Jul 2020 11:05:41 +0200
From:   Alexander Sverdlin <alexander.sverdlin@...ia.com>
To:     Tudor.Ambarus@...rochip.com, luisalberto@...gle.com
Cc:     vigneshr@...com, bbrezillon@...nel.org, richard@....at,
        jethro@...tanix.com, linux-kernel@...r.kernel.org,
        linux-mtd@...ts.infradead.org, miquel.raynal@...tlin.com,
        mika.westerberg@...ux.intel.com
Subject: Re: [PATCH] mtd: revert "spi-nor: intel: provide a range for
 poll_timout"

Hello Tudor,

On 22/07/2020 19:03, Tudor.Ambarus@...rochip.com wrote:
> On 7/22/20 7:37 PM, Alexander Sverdlin wrote:

[...]

>> I've performed my testing as well and got the following results:
>>
>> Vanilla Linux 4.9 (i.e. before the introduction of the offending
>> patch):
>>
>> dd if=/dev/flash/by-name/XXX of=/dev/null bs=4k
>> 1280+0 records in
>> 1280+0 records out
>> 5242880 bytes (5.2 MB, 5.0 MiB) copied, 3.91981 s, 1.3 MB/s
>>
>> Vanilla 4.19 (i.e. with offending patch):
>>
>> dd if=/dev/flash/by-name/XXX of=/dev/null bs=4k
>> 1280+0 records in
>> 1280+0 records out
>> 5242880 bytes (5.2 MB, 5.0 MiB) copied, 6.70891 s, 781 kB/s
>>
>> 4.19 + revert:
>>
>> dd if=/dev/flash/by-name/XXX of=/dev/null bs=4k
>> 1280+0 records in
>> 1280+0 records out
>> 5242880 bytes (5.2 MB, 5.0 MiB) copied, 3.90503 s, 1.3 MB/s
>>
>> Therefore it looks good from my PoV:
>>
>> Tested-by: Alexander Sverdlin <alexander.sverdlin@...il.com>

[...]

> would you put 10 us here
>>>                                 INTEL_SPI_TIMEOUT * 1000);
>>>  }
>>>
>>> @@ -301,7 +301,7 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi)
>>>       u32 val;
>>>
>>>       return readl_poll_timeout(ispi->sregs + SSFSTS_CTL, val,
>>> -                               !(val & SSFSTS_CTL_SCIP), 40,
>>> +                               !(val & SSFSTS_CTL_SCIP), 0,
> 
> also here, and re-do a test? I'm curios if the performance will be
> as it was before.

with 10us it looks like this:

dd if=/dev/flash/by-name/... of=/dev/null bs=4k
1280+0 records in
1280+0 records out
5242880 bytes (5.2 MB, 5.0 MiB) copied, 4.33816 s, 1.2 MB/s

Which means, there is a performance regression and it would depend on
the test case, how bad it will be...

-- 
Best regards,
Alexander Sverdlin.