[PATCH v2 5/5] mtdchar: extend MEMREAD ioctl to return ECC statistics

From: Michał Kępień
Date: Thu Dec 16 2021 - 03:34:46 EST


While returning -EUCLEAN or -EBADMSG is a valid signal for user space
that the ECC algorithm detected errors during a read operation, that
signal is not granular enough to cover all use cases. For example,
knowing the maximum number of bitflips detected in a single ECC step
during a read operation performed on a given page may be useful when
dealing with an MTD partition whose ECC layout varies across pages (e.g.
a partition consisting of a bootloader area using a "custom" ECC layout
followed by data pages using a "standard" ECC layout).

Extend struct mtd_read_req with a newly introduced struct
mtd_read_req_ecc_stats and set the fields of that structure in
mtdchar_read_ioctl(). Use ECC statistics to ensure the error code
returned by the MEMREAD ioctl accounts for all ECC errors detected
during the read operation.

Link: https://www.infradead.org/pipermail/linux-mtd/2016-April/067085.html

Suggested-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
Signed-off-by: Michał Kępień <kernel@xxxxxxxxxx>
---
drivers/mtd/mtdchar.c | 26 ++++++++++++++++++++++++++
include/uapi/mtd/mtd-abi.h | 21 +++++++++++++++++++++
2 files changed, 47 insertions(+)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 6ad59c9eed2f..68cc91d82a5d 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -714,6 +714,10 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
if (!usr_oob)
req.ooblen = 0;

+ req.ecc_stats.uncorrectable_errors = 0;
+ req.ecc_stats.corrected_bitflips = 0;
+ req.ecc_stats.max_bitflips = 0;
+
if (req.start + req.len > mtd->size) {
ret = -EINVAL;
goto out;
@@ -738,12 +742,14 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
}

while (req.len > 0 || (!usr_data && req.ooblen > 0)) {
+ struct mtd_req_stats stats;
struct mtd_oob_ops ops = {
.mode = req.mode,
.len = min_t(size_t, req.len, datbuf_len),
.ooblen = min_t(size_t, req.ooblen, oobbuf_len),
.datbuf = datbuf,
.oobbuf = oobbuf,
+ .stats = &stats,
};

/*
@@ -757,6 +763,13 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
ops.len -= mtd_mod_by_ws(req.start + ops.len, mtd);

ret = mtd_read_oob(mtd, (loff_t)req.start, &ops);
+
+ req.ecc_stats.uncorrectable_errors +=
+ stats.uncorrectable_errors;
+ req.ecc_stats.corrected_bitflips += stats.corrected_bitflips;
+ req.ecc_stats.max_bitflips =
+ max(req.ecc_stats.max_bitflips, stats.max_bitflips);
+
if (ret && !mtd_is_bitflip_or_eccerr(ret))
break;

@@ -774,6 +787,19 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
usr_oob += ops.oobretlen;
}

+ /*
+ * As multiple iterations of the above loop (and therefore multiple
+ * mtd_read_oob() calls) may be necessary to complete the read request,
+ * adjust the final return code to ensure it accounts for all detected
+ * ECC errors.
+ */
+ if (!ret || mtd_is_bitflip(ret)) {
+ if (req.ecc_stats.uncorrectable_errors > 0)
+ ret = -EBADMSG;
+ else if (req.ecc_stats.corrected_bitflips > 0)
+ ret = -EUCLEAN;
+ }
+
out:
req.len = orig_len - req.len;
req.ooblen = orig_ooblen - req.ooblen;
diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index 337e6e597fad..bc68f266c174 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -91,6 +91,25 @@ struct mtd_write_req {
__u8 padding[7];
};

+/**
+ * struct mtd_read_req_ecc_stats - ECC statistics for a read operation
+ *
+ * @uncorrectable_errors: the number of uncorrectable errors that happened
+ * during the read operation
+ * @corrected_bitflips: the number of bitflips corrected during the read
+ * operation
+ * @max_bitflips: the maximum number of bitflips detected in any single ECC
+ * step for the data read during the operation; this information
+ * can be used to decide whether the data stored in a specific
+ * region of the MTD device should be moved somewhere else to
+ * avoid data loss.
+ */
+struct mtd_read_req_ecc_stats {
+ __u32 uncorrectable_errors;
+ __u32 corrected_bitflips;
+ __u32 max_bitflips;
+};
+
/**
* struct mtd_read_req - data structure for requesting a read operation
*
@@ -101,6 +120,7 @@ struct mtd_write_req {
* @usr_oob: user-provided OOB buffer
* @mode: MTD mode (see "MTD operation modes")
* @padding: reserved, must be set to 0
+ * @ecc_stats: ECC statistics for the read operation
*
* This structure supports ioctl(MEMREAD) operations, allowing data and/or OOB
* reads in various modes. To read from OOB-only, set @usr_data == NULL, and to
@@ -115,6 +135,7 @@ struct mtd_read_req {
__u64 usr_oob;
__u8 mode;
__u8 padding[7];
+ struct mtd_read_req_ecc_stats ecc_stats;
};

#define MTD_ABSENT 0
--
2.34.1