On 02/01/16 16:31, Russell King - ARM Linux wrote: > On Sat, Jan 02, 2016 at 12:29:19PM +0000, Russell King - ARM Linux wrote: >> On Tue, Dec 29, 2015 at 03:44:38PM +0200, Adrian Hunter wrote: >>> On 21/12/15 13:41, Russell King wrote: >>>> Unnecessarily mapping and unmapping the align buffer for SD cards is >>>> expensive: performance measurements on iMX6 show that this gives a hit >>>> of 10% on hdparm buffered disk reads. >>>> >>>> MMC/SD card IO comes from the mm/vfs which gives us page based IO, so >>>> for this case, the align buffer is not going to be used. However, we >>>> still map and unmap this buffer. >>>> >>>> Eliminate this by switching the align buffer to be a DMA coherent >>>> buffer, which needs no DMA maintenance to access the buffer. >>> >>> Did you consider putting the align buffer in the same allocation >>> as the adma_table? >> >> It's not clear whether host->adma_table_sz would be appropriately >> aligned. > > Looking at this closer, it would not be appropriately aligned to place > the alignment buffer after the adma table, but placing the alignment > buffer in the allocation first would give appropriate alignment. > > The buffer sizes are: > > Table 32-bit 64-bit > alignment 512 1024 (entry sz * 128) > adma 2056 3084 (desc sz * (128 * 2 + 1)) > > Allocating the two together gives advantages and disadvantages: > > * for 32-bit address sizes, instead of allocating two order-0 pages, > we end up allocating one order-0 page, so halve the coherent DMA > allocation of the driver. > > * for 64-bit address sizes, we allocate one order-1 page instead, > which may be more prone to failure with a 4k page size, and it > also means the ADMA table will overlap a page boundary. I've no > idea whether that is an issue for any SDHCI controllers or not. > > I could add additional complexity to do different allocations in the > 32-bit and 64-bit paths, but that results in a _far_ more complicated > cleanup path, and much more prone to errors. > > In any case, such a patch should be entirely separate from _this_ > patch given that such a change may cause breakage and could need > to be reworked as a result. Indeed, it's a _separate_ change in > itself, for a different purpose from _this_ patch.
Agreed. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
