Re: [PATCH] mmc: block: enable cache-flushing when mmc cache is on

[Michael Wu <michael@allwinnertech.com>]

On 24/03/2022 19:27, Ulf Hansson wrote:
> On Thu, 17 Mar 2022 at 10:14, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> On Wed, 16 Mar 2022 at 17:08, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>
>>> On 16.3.2022 16.46, Christian Löhle wrote:
>>>>> So we are not going to let the block layer know about SD cache?
>>>>> Or is it a separate change?
>>>>
>>>> I have some code for this laying around, but as it requires reading, parsing and writing Function Registers,
>>>> in particular PEH, it's a lot of boilerplate code to get the functionality, but I'll clean it up and send a patch in the coming weeks.
>>>>
>>>
>>> We have the sd cache flush.  We would presumably just need to call blk_queue_write_cache()
>>> for the !mmc_card_mmc(card) case e.g.
>>>
>>>          if (mmc_has_reliable_write(card)) {
>>>                  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);
>>
>> To me, this seems like the most reasonable thing to do.
>>
>> However, I have to admit that it's not clear to me, if there was a
>> good reason to why commit f4c5522b0a88 ("mmc: Reliable write
>> support.") also added support for REQ_FLUSH (write back cache) and why
>> not only REQ_FUA. I assumed this was wrong too, right?
>>

Hi Ulf,

1. I've found the reason. If we only enable REQ_FUA, there won't be any 
effect -- The block layer won't send any request with FUA flag to the 
driver.
If we want REQ_FUA to take effect, we must enable REQ_FLUSH. But on the 
contrary, REQ_FLUSH does not rely on REQ_FUA.
In the previous patch(commit f4c5522b0a88 ("mmc: Reliable write 
support.")), REQ_FLUSH was added to make REQ_FUA effective. I've done 
experiments to prove this.

2. Why block layer requires REQ_FLUSH to make REQ_FUA effective? I did 
not find the reason. Does anyone know about this? Thank you.

>> When it comes to patches for stable kernels. mmc_cache_enabled() was
>> introduced quite recently in v5.13, so for older kernels that call
>> needs to be replaced with something else.
>>
>> In any case, the relevant commits that can be considered as needs to
>> be fixed seems like these:
>> commit f4c5522b0a88 ("mmc: Reliable write support.")
>> commit 881d1c25f765 ("mmc: core: Add cache control for eMMC4.5 device")
>> commit 130206a615a9 ("mmc: core: Add support for cache ctrl for SD cards")
>>
>> [...]
> 
> Michael, are you planning to send a v2 for this? Or are there any
> parts that are still unclear to you?

Dear Ulf, Sorry for the delay. I was trying to figure out the SD cache 
stuff, so a few day was taken...

> 
> Kind regards
> Uffe

-- 
Best Regards,
Michael Wu