Re: [PATCH v2 08/24] mmc: sdhci: avoid unnecessary mapping/unmapping of align buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux