On 10/10/2023 18:04, Ulf Hansson wrote:
On Tue, 10 Oct 2023 at 17:33, Bean Huo <beanhuo@xxxxxxxx> wrote:
Hi Ulf,
thanks for your comments, I didn't quite get your points:
On Tue, 2023-10-10 at 16:20 +0200, Ulf Hansson wrote:
@@ -2381,8 +2381,11 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct
mmc_queue *mq, struct request *req)
}
ret = mmc_blk_cqe_issue_flush(mq, req);
break;
- case REQ_OP_READ:
case REQ_OP_WRITE:
+ if (mmc_card_broken_cache_flush(card) &&
!card->written_flag)
It looks superfluous to me to check mmc_card_broken_cache_flush() and
!card->written_flag. Just set the card->written_flag unconditionally.
what did you mean "Just set the card->written_flag unconditionally."?
This means I just need to check card->written_flag and set card-
written_flag to true and false in the case
MMC_QUIRK_BROKEN_CACHE_FLUSH? or don't need to call
mmc_card_broken_cache_flush()?
I mean skip the checks above and just do the assignment below.
+ card->written_flag = true;
+ fallthrough;
+ case REQ_OP_READ:
if (host->cqe_enabled)
ret = mmc_blk_cqe_issue_rw_rq(mq,
req);
else
diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
index 4edf9057fa79..b7754a1b8d97 100644
--- a/drivers/mmc/core/card.h
+++ b/drivers/mmc/core/card.h
@@ -280,4 +280,8 @@ static inline int
mmc_card_broken_sd_cache(const struct mmc_card *c)
return c->quirks & MMC_QUIRK_BROKEN_SD_CACHE;
}
+static inline int mmc_card_broken_cache_flush(const struct
mmc_card *c)
+{
+ return c->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH;
+}
#endif
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 89cd48fcec79..47896c32086e 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1929,6 +1929,8 @@ static int mmc_init_card(struct mmc_host
*host, u32 ocr,
if (!oldcard)
host->card = card;
+ card->written_flag = false;
+
How about doing this after a successful flush operation instead? In
other words in _mmc_flush_cache().
Here initializes flag and the patch is intenting to eliminate the cache
flush command before writing. what do you mean adding in
mmc_flush_cache()?
mmc_init_card() is called while initializing and re-initializing the
card. So, it certainly works to reset the flag from here.
However, _mmc_flush_cache() is called before powering off the card,
which then would work similarly to the above, but also gets called for
REQ_OP_FLUSH. Wouldn't that mean that we may be able to skip some
unnecessary/troublesome cache flush requests if we would reset the
flag from mmc_flush_cache(), rather than from mmc_init_card()?
Kind regards
Uffe
Forgive me if I'm misunderstanding something, but wouldn't resetting the
flag on _mmc_flush_cache()
essentially disable cache when the MMC_QUIRK_BROKEN_CACHE_FLUSH is active?
From what I see, the card->written flag is supposed to stay true until
we need to reinitialize the
eMMC again, otherwise we would be skipping cache flushes we don't want
to skip. It's only before
writing for the first time that we want to skip the flush.
Regards,
Rafael
