Hi Ulf and Linus,
I understand your concerns about the current packed commands
support and the problem of transition to blk_mq.
But as I mentioned in previous email thread, the packed commands
feature is still in use on eMMC4.5 or eMMC5.1 devices on hosts
without SW/HW CMDQ support.
Taking this feature out meaning stop supporting one of important
performance features of eMMC devices.
Looking forward this feature is replaced with a CMDQ, but should we
kill it for those who doesn’t have a CMDQ?
Thanks,
Alex
On 11/25/16, 3:30 PM, "linux-mmc-owner@xxxxxxxxxxxxxxx on behalf of Ulf Hansson" <linux-mmc-owner@xxxxxxxxxxxxxxx on behalf of ulf.hansson@xxxxxxxxxx> wrote:
>On 25 November 2016 at 10:35, Linus Walleij <linus.walleij@xxxxxxxxxx> 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!
>>
>> - 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.
>>
>> - There is no Device Tree binding or code to mark a host as
>> supporting packed read or write commands, just this flag
>> in caps2, so for sure there are not any DT systems using
>> it either.
>>
>> It has other problems as well: mmc_blk_prep_packed_list() is
>> speculatively picking requests out of the request queue with
>> blk_fetch_request() making the MMC/SD stack harder to convert
>> to the multiqueue block layer. By this we get rid of an
>> obstacle.
>>
>> The way I see it this is just cruft littering the MMC/SD
>> stack.
>>
>> Cc: Namjae Jeon <namjae.jeon@xxxxxxxxxxx>
>> Cc: Maya Erez <qca_merez@xxxxxxxxxxxxxxxx>
>> Acked-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
>> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
>Thanks, applied for next!
>
>Kind regards
>Uffe
>
��.n��������+%������w��{.n�����{��i��)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
