Pass -EUCLEN to userspace?
Boris Brezillon
boris.brezillon at free-electrons.com
Mon Apr 25 07:11:44 PDT 2016
On Mon, 25 Apr 2016 11:26:16 +0200
Boris Brezillon <boris.brezillon at free-electrons.com> wrote:
> > >
> > > Regarding the maximum number of bitflips per chunk, maybe we can make it
> > > part of the ioctl request instead of saving the statistics at the MTD
> > > level.
> > >
> > > How about creating a new ioctl taking a pointer to this struct as a
> > > parameter:
> > >
> > > struct mtd_extended_read_ops {
> > > /* Existing params */
> > > unsigned int mode;
> > > size_t len;
> > > size_t retlen;
> > > size_t ooblen;
> > > size_t oobretlen;
> > > uint32_t ooboffs;
> > > void *datbuf;
> > > void *oobbuf;
> > >
> > > /*
> > > * Param containing the maximum number of bitflips for this
> > > * read request.
> > > */
> > > unsigned int max_bitflips;
> > > };
> >
> > Not sure how this ioctl exactly should look like, but this would solve
> > the problem.
>
> Let me design a quick prototype, I'll let you follow up with the patch
> submission process...
Below is an untested patch adding a new ioctl returning the ECC/read stats.
Feel free to debug/enhance this implementation and submit it to the MTD
ML.
--->8---
From 2c07a2a015e6c0bdc2f9d91c9a1b971903da0493 Mon Sep 17 00:00:00 2001
From: Boris Brezillon <boris.brezillon at free-electrons.com>
Date: Mon, 25 Apr 2016 16:05:06 +0200
Subject: [PATCH] mtd: add a the MEMREAD ioctl to attach ECC stats to the read
request
TODO: add proper commit message + split changes into several patches.
Signed-off-by: Boris Brezillon <boris.brezillon at free-electrons.com>
---
drivers/mtd/devices/docg3.c | 10 +++++
drivers/mtd/mtdchar.c | 76 ++++++++++++++++++++++++++++++++++++++
drivers/mtd/mtdcore.c | 9 +++++
drivers/mtd/nand/nand_base.c | 10 +++++
drivers/mtd/onenand/onenand_base.c | 11 ++++++
include/linux/mtd/mtd.h | 7 ++++
include/uapi/mtd/mtd-abi.h | 50 +++++++++++++++++++++++++
7 files changed, 173 insertions(+)
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index b833e6c..965c97a 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -885,6 +885,8 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
u8 *buf = ops->datbuf;
size_t len, ooblen, nbdata, nboob;
u8 hwecc[DOC_ECC_BCH_SIZE], eccconf1;
+ struct mtd_req_stats *reqstats = ops->stats;
+ struct mtd_ecc_stats stats;
int max_bitflips = 0;
if (buf)
@@ -912,6 +914,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
ret = 0;
skip = from % DOC_LAYOUT_PAGE_SIZE;
mutex_lock(&docg3->cascade->lock);
+ stats = mtd->ecc_stats;
while (ret >= 0 && (len > 0 || ooblen > 0)) {
calc_block_sector(from - skip, &block0, &block1, &page, &ofs,
docg3->reliable);
@@ -983,6 +986,13 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
}
out:
+ if (reqstats) {
+ restats->corrected_bitflips += mtd->ecc_stats.corrected -
+ stats.corrected;
+ restats->uncorrectable_errors += mtd->ecc_stats.failed -
+ stats.failed;
+ }
+
mutex_unlock(&docg3->cascade->lock);
return ret;
err_in_read:
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 2a47a3f..708ecee 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -656,6 +656,75 @@ static int mtdchar_write_ioctl(struct mtd_info *mtd,
return ret;
}
+static int mtdchar_read_ioctl(struct mtd_info *mtd,
+ struct mtd_read_req __user *argp)
+{
+ struct mtd_read_req req;
+ struct mtd_req_stats stats;
+ struct mtd_oob_ops ops = { .stats = &stats };
+ void __user *usr_data, *usr_oob;
+ int ret;
+
+ if (copy_from_user(&req, argp, sizeof(req)))
+ return -EFAULT;
+
+ usr_data = (void __user *)(uintptr_t)req.usr_data;
+ usr_oob = (void __user *)(uintptr_t)req.usr_oob;
+ if ((usr_data && !access_ok(VERIFY_WRITE, usr_data, req.len)) ||
+ (usr_oob && !access_ok(VERIFY_WRITE, usr_oob, req.ooblen)))
+ return -EFAULT;
+
+ if (!mtd->_read_oob)
+ return -EOPNOTSUPP;
+
+ ops.mode = req.mode;
+ ops.len = (size_t)req.len;
+ ops.ooblen = (size_t)req.ooblen;
+ ops.ooboffs = 0;
+
+ if (usr_data) {
+ ops.datbuf = kzalloc(ops.len, GFP_KERNEL);
+ if (!ops.datbuf)
+ return -ENOMEM;
+ }
+
+ if (usr_oob) {
+ ops.oobbuf = kzalloc(ops.ooblen, GFP_KERNEL);
+ if (!ops.oobbuf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+
+ ret = mtd_read_oob(mtd, (loff_t)req.start, &ops);
+ if (ret)
+ goto out;
+
+ if (usr_data && copy_to_user(ops.datbuf, usr_data, ops.retlen)) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ if (usr_oob && copy_to_user(ops.oobbuf, usr_oob, ops.oobretlen)) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ req.len = ops.retlen;
+ req.ooblen = ops.oobretlen;
+ req.stats.max_bitflips = stats.max_bitflips;
+ req.stats.corrected_bitflips = stats.corrected_bitflips;
+ req.stats.uncorrectable_errors = stats.uncorrectable_errors;
+ if (copy_to_user(&req, argp, sizeof(req)))
+ ret = -EFAULT;
+
+out:
+ kfree(ops.datbuf);
+ kfree(ops.oobbuf);
+
+ return ret;
+}
+
static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
{
struct mtd_file_info *mfi = file->private_data;
@@ -850,6 +919,13 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
break;
}
+ case MEMREAD:
+ {
+ ret = mtdchar_read_ioctl(mtd,
+ (struct mtd_read_req __user *)arg);
+ break;
+ }
+
case MEMLOCK:
{
struct erase_info_user einfo;
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index e3936b8..ba6488f 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -988,6 +988,11 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
return -EOPNOTSUPP;
ledtrig_mtd_activity();
+
+ /* Make sure all counters are initialized to zero. */
+ if (ops->stats)
+ memset(ops->stats, 0, sizeof(*ops->stats));
+
/*
* In cases where ops->datbuf != NULL, mtd->_read_oob() has semantics
* similar to mtd->_read(), returning a non-negative integer
@@ -999,6 +1004,10 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
return ret_code;
if (mtd->ecc_strength == 0)
return 0; /* device lacks ecc */
+
+ if (ops->stats)
+ ops->stats->max_bitflips = ret_code;
+
return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;
}
EXPORT_SYMBOL_GPL(mtd_read_oob);
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 7bc37b4..25cab31 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2162,6 +2162,8 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
static int nand_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
{
+ struct mtd_req_stats *reqstats = ops->stats;
+ struct mtd_ecc_stats stats;
int ret = -ENOTSUPP;
ops->retlen = 0;
@@ -2185,11 +2187,19 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
goto out;
}
+ stats = mtd->ecc_stats;
if (!ops->datbuf)
ret = nand_do_read_oob(mtd, from, ops);
else
ret = nand_do_read_ops(mtd, from, ops);
+ if (reqstats) {
+ reqstats->uncorrectable_errors += mtd->ecc_stats.failed -
+ stats.failed;
+ reqstats->corrected_bitflips += mtd->ecc_stats.corrected -
+ stats.corrected;
+ }
+
out:
nand_release_device(mtd);
return ret;
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index a4b029a..2f700eb 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1491,6 +1491,8 @@ static int onenand_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
{
struct onenand_chip *this = mtd->priv;
+ struct mtd_req_stats *reqstats = ops->stats;
+ struct mtd_ecc_stats stats;
int ret;
switch (ops->mode) {
@@ -1504,12 +1506,21 @@ static int onenand_read_oob(struct mtd_info *mtd, loff_t from,
}
onenand_get_device(mtd, FL_READING);
+ stats = mtd->ecc_stats;
+
if (ops->datbuf)
ret = ONENAND_IS_4KB_PAGE(this) ?
onenand_mlc_read_ops_nolock(mtd, from, ops) :
onenand_read_ops_nolock(mtd, from, ops);
else
ret = onenand_read_oob_nolock(mtd, from, ops);
+
+ if (reqstats) {
+ reqstats->corrected_bitflips += mtd->ecc_stats.corrected -
+ stats.corrected;
+ reqstats->uncorrectable_errors += mtd->ecc_stats.failed -
+ stats.failed;
+ }
onenand_release_device(mtd);
return ret;
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 29a1706..bd5e8a1 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -64,6 +64,12 @@ struct mtd_erase_region_info {
unsigned long *lockmap; /* If keeping bitmap of locks */
};
+struct mtd_req_stats {
+ unsigned int max_bitflips;
+ unsigned int corrected_bitflips;
+ unsigned int uncorrectable_errors;
+};
+
/**
* struct mtd_oob_ops - oob operation operands
* @mode: operation mode
@@ -92,6 +98,7 @@ struct mtd_oob_ops {
uint32_t ooboffs;
uint8_t *datbuf;
uint8_t *oobbuf;
+ struct mtd_req_stats *stats;
};
#define MTD_MAX_OOBFREE_ENTRIES_LARGE 32
diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index 0ec1da2..a61a042 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -90,6 +90,52 @@ struct mtd_write_req {
__u8 padding[7];
};
+/**
+ * struct mtd_read_req_stats - statistics attached to a read request
+ *
+ * @max_bitflips: the maximum number of bitflips found in the read portion.
+ * This information can be used to decide when the data stored
+ * on a specific portion should be moved somewhere else to
+ * avoid data loss.
+ * @corrected_bitflips: the number of bitflips corrected during the read
+ * operation
+ * @uncorrectable_errors: the number of uncorrectable errors that happened
+ * during the read operation
+ */
+struct mtd_read_req_stats {
+ __u32 max_bitflips;
+ __u32 corrected_bitflips;
+ __u32 uncorrectable_errors;
+};
+
+/**
+ * struct mtd_read_req - data structure for requesting a write operation
+ *
+ * @start: start address
+ * @len: length of data buffer
+ * @ooblen: length of OOB buffer
+ * @usr_data: user-provided data buffer
+ * @usr_oob: user-provided OOB buffer
+ * @mode: MTD mode (see "MTD operation modes")
+ * @padding: reserved, must be set to 0
+ * @stats: statistics attached to the read request
+ *
+ * This structure supports ioctl(MEMWRITE) operations, allowing data and/or OOB
+ * writes in various modes. To write to OOB-only, set @usr_data == NULL, and to
+ * write data-only, set @usr_oob == NULL. However, setting both @usr_data and
+ * @usr_oob to NULL is not allowed.
+ */
+struct mtd_read_req {
+ __u64 start;
+ __u64 len;
+ __u64 ooblen;
+ __u64 usr_data;
+ __u64 usr_oob;
+ __u8 mode;
+ __u8 padding[7];
+ struct mtd_read_req_stats stats;
+};
+
#define MTD_ABSENT 0
#define MTD_RAM 1
#define MTD_ROM 2
@@ -203,6 +249,10 @@ struct otp_info {
* without OOB, e.g., NOR flash.
*/
#define MEMWRITE _IOWR('M', 24, struct mtd_write_req)
+/*
+ * Same as for MEMWRITE but for read operations.
+ */
+#define MEMREAD _IOWR('M', 25, struct mtd_read_req)
/*
* Obsolete legacy interface. Keep it in order not to break userspace
More information about the linux-mtd
mailing list