Re: [PATCH] mmc: block: enable cache-flushing when mmc cache is on
From: Michael Wu
Date: Wed Mar 16 2022 - 05:54:40 EST
On 14/03/2022 17:37, Avri Altman wrote:
On 14/03/2022 14:54, Adrian Hunter wrote:
On 12/03/2022 06:43, Michael Wu wrote:
The mmc core enable cache on default. But it only enables
cache-flushing when host supports cmd23 and eMMC supports reliable
write.
For hosts which do not support cmd23 or eMMCs which do not support
reliable write, the cache can not be flushed by `sync` command.
This may leads to cache data lost.
This patch enables cache-flushing as long as cache is enabled, no
matter host supports cmd23 and/or eMMC supports reliable write or not.
Fixes tag?
Hi Adrian,
My patch intend to fix the cache problem brought by the following two
patches:
Fixes: d0c97cfb81ebc ("mmc: core: Use CMD23 for multiblock transfers when
we can.")
Fixes: e9d5c746246c8 ("mmc/block: switch to using blk_queue_write_cache()")
I'm not sure if this is what you referred to ("Fixes tag"). Please correct me if I
misunderstood.
Signed-off-by: Michael Wu <michael@xxxxxxxxxxxxxxxxx>
---
drivers/mmc/core/block.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 689eb9afeeed..1e508c079c1e 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2279,6 +2279,8 @@ static struct mmc_blk_data
*mmc_blk_alloc_req(struct mmc_card *card,
struct mmc_blk_data *md;
int devidx, ret;
char cap_str[10];
+ bool enable_cache = false;
+ bool enable_fua = false;
devidx = ida_simple_get(&mmc_blk_ida, 0, max_devices, GFP_KERNEL);
if (devidx < 0) {
@@ -2375,12 +2377,18 @@ static struct mmc_blk_data
*mmc_blk_alloc_req(struct mmc_card *card,
md->flags |= MMC_BLK_CMD23;
}
- if (mmc_card_mmc(card) &&
- md->flags & MMC_BLK_CMD23 &&
- ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
- card->ext_csd.rel_sectors)) {
- md->flags |= MMC_BLK_REL_WR;
- blk_queue_write_cache(md->queue.queue, true, true);
+ if (mmc_card_mmc(card)) {
+ if (md->flags & MMC_BLK_CMD23 &&
+ ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN)
||
+ card->ext_csd.rel_sectors)) {
+ md->flags |= MMC_BLK_REL_WR;
+ enable_fua = true;
+ }
+
+ if (mmc_cache_enabled(card->host))
+ enable_cache = true;
+
+ blk_queue_write_cache(md->queue.queue, enable_cache,
+ enable_fua);
}
Seems like we should inform block layer about SD card cache also
I saw another mail by Avri Altman, which says few days will be needed to ask
internally. Shall I wait or make another change here on 'inform block layer
about SD card cache'?
Please don't wait.
Thanks,
Avri
string_get_size((u64)size, 512, STRING_UNITS_2,
--
Best Regards,
Michael Wu
Hi Avril & Adrian,
Thanks for your efforts. Could we have an agreement now --
1. enabling-cache and cmd23/reliable-write should be independent;
> On 14/03/2022 18:32, Adrian Hunter wrote:
>> On 14/03/2022 09:26, Avri Altman wrote:
>>> Hi,
>>>> The mmc core enable cache on default. But it only enables
cache-flushing
>>>> when host supports cmd23 and eMMC supports reliable write.
>>>> For hosts which do not support cmd23 or eMMCs which do not support
>>>> reliable write, the cache can not be flushed by `sync` command.
>>>> This may leads to cache data lost.
>>>> This patch enables cache-flushing as long as cache is enabled, no
>>>> matter host supports cmd23 and/or eMMC supports reliable write or
>>>> not.
>>> I looked in the spec and indeed couldn't find why enabling cache is
>>> dependent of cmd23/reliable write.
>>> Nor I was able to find the original commit log.
>>
>> Reliable write was added first, so it might have been an oversight:
>>
>> commit 881d1c25f765938a95def5afe39486ce39f9fc96
>> Author: Seungwon Jeon <tgih.jun@xxxxxxxxxxx>
>> Date: Fri Oct 14 14:03:21 2011 +0900
>>
>> mmc: core: Add cache control for eMMC4.5 device
>>
>> This patch adds cache feature of eMMC4.5 Spec.
>> If device supports cache capability, host can utilize some
>> specific operations.
>>
>> Signed-off-by: Seungwon Jeon <tgih.jun@xxxxxxxxxxx>
>> Signed-off-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
>> Signed-off-by: Chris Ball <cjb@xxxxxxxxxx>
Here's what I found in the spec JESD84-B51:
> 6.6.31 Cache
> Caching of data shall apply only for the single block
> read/write(CMD17/24), pre-defined multiple block
> read/write(CMD23+CMD18/25) and open ended multiple block
> read/write(CMD18/25+CMD12) commands and excludes any other access
> e.g., to the register space(e.g., CMD6).
Which means with CMD18/25+CMD12 (without using CMD23), the cache can
also be enabled. Maybe this could be an evidence of the independence
between enabling-cache and cmd23/reliable-write?
2. We don't consider supporting SD in this change.
> On 14/03/2022 19:10, Avri Altman wrote:
>> Here is what our SD system guys wrote:
>> " In SD we don’t support reliable write and this eMMC driver may not
>> be utilizing the cache feature we added in SD5.0.
>> The method of cache flush is different between SD and eMMC."
>>
>> So adding SD seems to be out of scope of this change.
Is there anything else I can do about this patch? Thanks again.
--
Best Regards,
Michael Wu