linux-kernel - [PATCH] mmc: core: Allow to avoid REQ_FUA if the eMMC supports an internal cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230316164514.1615169-1-ulf.hansson@linaro.org>
Date:   Thu, 16 Mar 2023 17:45:14 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     linux-mmc@...r.kernel.org, Ulf Hansson <ulf.hansson@...aro.org>
Cc:     Wenchao Chen <wenchao.chen666@...il.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Avri Altman <avri.altman@....com>,
        Christian Lohle <cloehle@...erstone.com>,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bean Huo <beanhuo@...ron.com>
Subject: [PATCH] mmc: core: Allow to avoid REQ_FUA if the eMMC supports an internal cache

REQ_FUA translates into so called "reliable writes" (atomic writes) for
eMMC cards, which is generally supported as it was introduced as a
mandatory feature already in the v4.3 (2007) of the eMMC spec. To fully
support the reliable writes (thus REQ_FUA), the mmc host driver needs to
support the CMD23 (MMC_CAP_CMD23) too, which is rather common nowadays.

File systems typically uses REQ_FUA for writing their meta-data and other
important information. Ideally it should provide an increased protection
against data-corruption, during sudden power-failures. This said, file
systems have other ways to handle sudden power-failures too, like using
checksums to detect partly-written data, for example.

It has been reported that the reliable writes are costly for some eMMCs,
leading to performance degradations. Exactly why, is in the implementation
details of the internals of the eMMC.

Moreover, in the v4.5 (2011) of the eMMC spec, the cache-control was
introduced as an optional feature. It allows the host to trigger a flush of
the eMMC's internal write-cache. In the past, before the cache-control
feature was added, the reliable write acted as trigger for the eMMC, to
also flush its internal write-cache, even if that too remains as an
implementation detail of the eMMC.

In a way to try to improve the situation with costly reliable writes and
REQ_FUA, let's add a new card quirk MMC_QUIRK_AVOID_REL_WRITE, which may be
set to avoid announcing the support for it. However, as mentioned above,
due to the specific relationship with the cache-control feature, we must
keep REQ_FUA unless that is supported too.

Reported-by: Wenchao Chen <wenchao.chen666@...il.com>
Acked-by: Bean Huo <beanhuo@...ron.com>
Acked-by: Avri Altman <avri.altman@....com>
Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
---

Updated since the RFC:
	Added a card quirk to maintain the current behaviour. The quirk isn't
	set for any cards yet, which is needed (a patch on top) to move forward
	with this.

---
 drivers/mmc/core/block.c | 16 ++++++++++++----
 drivers/mmc/core/card.h  |  5 +++++
 include/linux/mmc/card.h |  1 +
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 672ab90c4b2d..35292e36a1fb 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2409,8 +2409,7 @@ 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 cache_enabled = false;
-	bool fua_enabled = false;
+	bool cache_enabled, avoid_fua, fua_enabled = false;
 
 	devidx = ida_simple_get(&mmc_blk_ida, 0, max_devices, GFP_KERNEL);
 	if (devidx < 0) {
@@ -2494,11 +2493,20 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
 	     card->ext_csd.rel_sectors)) {
 		md->flags |= MMC_BLK_REL_WR;
+	}
+
+	/*
+	 * REQ_FUA is supported through eMMC reliable writes, which has been
+	 * reported to be a bit costly for some eMMCs. In these cases, let's
+	 * rely on the flush requests (REQ_OP_FLUSH) instead, if we can use the
+	 * cache-control feature too.
+	 */
+	cache_enabled = mmc_cache_enabled(card->host);
+	avoid_fua = cache_enabled && mmc_card_avoid_rel_write(card);
+	if (md->flags & MMC_BLK_REL_WR && !avoid_fua) {
 		fua_enabled = true;
 		cache_enabled = true;
 	}
-	if (mmc_cache_enabled(card->host))
-		cache_enabled  = true;
 
 	blk_queue_write_cache(md->queue.queue, cache_enabled, fua_enabled);
 
diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
index cfdd1ff40b86..2ab448fa2841 100644
--- a/drivers/mmc/core/card.h
+++ b/drivers/mmc/core/card.h
@@ -229,6 +229,11 @@ static inline int mmc_blksz_for_byte_mode(const struct mmc_card *c)
 	return c->quirks & MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
 }
 
+static inline int mmc_card_avoid_rel_write(const struct mmc_card *c)
+{
+	return c->quirks & MMC_QUIRK_AVOID_REL_WRITE;
+}
+
 static inline int mmc_card_disable_cd(const struct mmc_card *c)
 {
 	return c->quirks & MMC_QUIRK_DISABLE_CD;
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index c726ea781255..4d297d565c83 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -282,6 +282,7 @@ struct mmc_card {
 						/* for byte mode */
 #define MMC_QUIRK_NONSTD_SDIO	(1<<2)		/* non-standard SDIO card attached */
 						/* (missing CIA registers) */
+#define MMC_QUIRK_AVOID_REL_WRITE (1<<3)	/* Avoid rel-write (REQ_FUA) */
 #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4)		/* SDIO card has nonstd function interfaces */
 #define MMC_QUIRK_DISABLE_CD	(1<<5)		/* disconnect CD/DAT[3] resistor */
 #define MMC_QUIRK_INAND_CMD38	(1<<6)		/* iNAND devices have broken CMD38 */
-- 
2.34.1