Re: [PATCH] mmc: block: delete packed command support

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

 



Hi,


On 11/21/16, 1:11 PM, "Arnd Bergmann" <arnd@xxxxxxxx> wrote:

>On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
>> I've had it with this code now.
>> 
>> The packed command support is a complex hurdle in the MMC/SD block
>> layer, around 500+ lines of code which was introduced in 2013 in
>> commits
>> 
>> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
>> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>> 
>> ...and since then it has been rotting. The original author of the
>> code has disappeared from the community and the mail address is
>> bouncing.
>> 
>> For the code to be exercised the host must flag that it supports
>> packed commands, so in mmc_blk_prep_packed_list() which is called for
>> every single request, the following construction appears:
>> 
>> u8 max_packed_rw = 0;
>> 
>> if ((rq_data_dir(cur) == WRITE) &&
>>     mmc_host_packed_wr(card->host))
>>         max_packed_rw = card->ext_csd.max_packed_writes;
>> 
>> if (max_packed_rw == 0)
>>     goto no_packed;
>> 
>> This has the following logical deductions:
>> 
>> - Only WRITE commands can really be packed, so the solution is
>>   only half-done: we support packed WRITE but not packed READ.
>>   The packed command support has not been finalized by supporting
>>   reads in three years!
>
>Packed reads don't make a lot of sense, there is very little
>for an MMC to optimize in reads that it can't already do without
>the packing. For writes, packing makes could be an important
>performance optimization, if the eMMC supports it.
>
>I've added Luca Porzio  and Alex Lemberg to Cc. I think they
>are subscribed to the list already, but it would be good to
>get some estimate from them too about how common the packed
>write support is on existing hardware from their respective
>employers before we kill it off.

Correct, in general there is no value in using packed for Read.
But I can’t say this for all existing flash management solution.
The eMMC spec allows to use it for Read as well.

>
>If none of Samsung/Micron/Sandisk are currently shipping
>devices that support eMMC-4.5 packed commands but don't
>support the eMMC-5.1 command queuing (which IIRC is a more
>general way to achieve the same), we probably don't need to
>worry about it too much. 

Please note that even by having eMMC-5.1 device,
not all chipset vendors are having HW/SW CMDQ support.
So they might be using packed commands instead.

>
>> - mmc_host_packed_wr() is just a static inline that checks
>>   host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
>>   that NO upstream host sets this capability flag! No driver
>>   in the kernel is using it, and we can't test it. Packed
>>   command may be supported in out-of-tree code, but I doubt
>>   it. I doubt that the code is even working anymore due to
>>   other refactorings in the MMC block layer, who would
>>   notice if patches affecting it broke packed commands?
>>   No one.
>
>This is a very good indication that it's not really used.
>It would be useful to also check out the Android AOSP
>kernel tree to see if it's in there, if anything important
>supports packed commands, it's probably in there.

As far as I can say from reviewing the mobile (Android)
platforms kernel source (from different vendors), 
many of them are enabling Packed Commands.

>
>	Arnd

��.n��������+%������w��{.n�����{��i��)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥




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

  Powered by Linux