linux-kernel - Re: [PATCH] mmc: dw_mmc: Fix IDMAC operation with pages bigger than 4K

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 for Android: free password hash cracker in your pocket

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

Message-Id: <bd334314-25f7-4e87-bcab-e3baed9a7ab5@app.fastmail.com>
Date: Tue, 27 Aug 2024 09:27:34 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Sam Protsenko" <semen.protsenko@...aro.org>
Cc: "jh80.chung" <jh80.chung@...sung.com>,
 "Ulf Hansson" <ulf.hansson@...aro.org>, "Christoph Hellwig" <hch@....de>,
 "Chris Ball" <cjb@...top.org>, "Will Newton" <will.newton@...il.com>,
 "Matt Fleming" <matt@...sole-pimps.org>,
 "Christian Brauner" <brauner@...nel.org>, "Jens Axboe" <axboe@...nel.dk>,
 "Sumit Semwal" <sumit.semwal@...aro.org>,
 "Dan Carpenter" <dan.carpenter@...aro.org>,
 "Anders Roxell" <anders.roxell@...aro.org>,
 "Naresh Kamboju" <naresh.kamboju@...aro.org>,
 "linux-mmc @ vger . kernel . org" <linux-mmc@...r.kernel.org>,
 linux-block <linux-block@...r.kernel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mmc: dw_mmc: Fix IDMAC operation with pages bigger than 4K

On Tue, Aug 27, 2024, at 03:28, Sam Protsenko wrote:
> On Thu, Mar 7, 2024 at 1:52 AM Arnd Bergmann <arnd@...db.de> wrote:

>>
>> The change looks good to me.
>>
>> I see that the host->ring_size depends on PAGE_SIZE as well:
>>
>> #define DESC_RING_BUF_SZ        PAGE_SIZE
>> host->ring_size = DESC_RING_BUF_SZ / sizeof(struct idmac_desc_64addr);
>> host->sg_cpu = dmam_alloc_coherent(host->dev,
>>                DESC_RING_BUF_SZ, &host->sg_dma, GFP_KERNEL);
>>
>> I don't see any reason for the ring buffer size to be tied to
>> PAGE_SIZE at all, it was probably picked as a reasonable
>> default in the initial driver but isn't necessarily ideal.
>>
>> From what I can see, the number of 4KB elements in the
>> ring can be as small as 128 (4KB pages, 64-bit addresses)
>> or as big as 4096 (64KB pages, 32-bit addresses), which is
>> quite a difference. If you are still motivated to drill
>> down into this, could you try changing DESC_RING_BUF_SZ
>> to a fixed size of either 4KB or 64KB and test again
>> with the opposite page size, to see if that changes the
>> throughput?
>>
>
> Sorry for the huge delay. Just ran the tests:
>
> - 4K pages, DESC_RING_BUF_SZ = 4K: 97 MB/s
> - 4K pages, DESC_RING_BUF_SZ = 16K: 98 MB/s
> - 4K pages, DESC_RING_BUF_SZ = 64K: 97 MB/s
> - 16K pages, DESC_RING_BUF_SZ = 4K: 123 MB/s
> - 16K pages, DESC_RING_BUF_SZ = 16K: 125 MB/s
> - 16K pages, DESC_RING_BUF_SZ = 64K: 124 MB/s
> - 64K pages, DESC_RING_BUF_SZ = 4K: 137 MB/s
> - 64K pages, DESC_RING_BUF_SZ = 16K: 135 MB/s
> - 64K pages, DESC_RING_BUF_SZE = 64K: 138 MB/s

Thanks!

> From what you said, it looks like it may make a sense to reduce
> DESC_RING_BUF_SZ down to 4 KiB? If so, I'd suggest we do that in a
> separate patch, as this one actually fixes kernel panic when 16k/64k
> pages are enabled. Please let me know what you think.

Agreed, sounds good to me.

     Arnd