* [PATCH] mtd: fsl_ifc_nand: Recover corrupted empty page for preventing read-only mount in UBIFS
@ 2014-04-01 1:49 ` Eunbong Song
0 siblings, 0 replies; 4+ messages in thread
From: Eunbong Song @ 2014-04-01 1:49 UTC (permalink / raw)
To: computersforpeace, artem.bityutskiy; +Cc: linux-kernel, linux-mtd
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 2436 bytes --]
Even if the meaning of EUCLEAN was changed by commit edbc4540.
There is still possibility of read-only mount in UBIFS with ubifs_scan() "corrupt empty space at LEB".
So i made this patch for fix that problem.
This patch do as follow.
- If there are ecc errors which is equal to or less than chip->ecc.strength in page.
- Check that page has how many zero bits, and if zero bits are equal to or less than
chip->ecc.strength then overwrite 1 to zero bits in buf.
ubifs_scan() cannot detect corrupted empty space because buf is recovered by this patch.
And this is safe because ecc controller can correct up to chip->ecc.strength bits.
Signed-off-by: Eunbong Song <eunb.song@samsung.com>
---
drivers/mtd/nand/fsl_ifc_nand.c | 41 +++++++++++++++++++++++++++++++++++++++
1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 90ca7e7..2129c39 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -277,6 +277,42 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum)
return 1;
}
+static int num_zero_bits(uint8_t val)
+{
+ int i, ret=0;
+
+ for(i=7; i>=0 ; i--)
+ if(!(0x1 & (val >> i)))
+ ret++;
+
+ return ret;
+}
+
+static int is_corrupted_blank(struct mtd_info *mtd, uint8_t * buf)
+{
+ struct nand_chip *chip = mtd->priv;
+ int i;
+ int zero_bits = 0;
+
+ for (i = 0; i < mtd->writesize ; i++) {
+ if(buf[i] != 0xff) {
+ zero_bits += num_zero_bits(buf[i]);
+ }
+ }
+
+ if(zero_bits && (zero_bits <= chip->ecc.strength)){
+ return 1;
+ }
+
+ return 0;
+}
+
+static void recover_corrupted_blank(struct mtd_info *mtd, uint8_t * buf)
+{
+ memset(buf, 0xff, mtd->writesize);
+ return;
+}
+
/* returns nonzero if entire page is blank */
static int check_read_ecc(struct mtd_info *mtd, struct fsl_ifc_ctrl *ctrl,
u32 *eccstat, unsigned int bufnum)
@@ -760,6 +796,11 @@ static int fsl_ifc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
mtd->ecc_stats.failed++;
+ if(nctrl->max_bitflips && (nctrl->max_bitflips <= chip->ecc.strength)){
+ if(is_corrupted_blank(mtd, buf))
+ recover_corrupted_blank(mtd, buf);
+ }
+
return nctrl->max_bitflips;
}
--
1.7.0.1
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] mtd: fsl_ifc_nand: Recover corrupted empty page for preventing read-only mount in UBIFS
@ 2014-04-01 1:49 ` Eunbong Song
0 siblings, 0 replies; 4+ messages in thread
From: Eunbong Song @ 2014-04-01 1:49 UTC (permalink / raw)
To: computersforpeace, artem.bityutskiy; +Cc: linux-mtd, linux-kernel
Even if the meaning of EUCLEAN was changed by commit edbc4540.
There is still possibility of read-only mount in UBIFS with ubifs_scan() "corrupt empty space at LEB".
So i made this patch for fix that problem.
This patch do as follow.
- If there are ecc errors which is equal to or less than chip->ecc.strength in page.
- Check that page has how many zero bits, and if zero bits are equal to or less than
chip->ecc.strength then overwrite 1 to zero bits in buf.
ubifs_scan() cannot detect corrupted empty space because buf is recovered by this patch.
And this is safe because ecc controller can correct up to chip->ecc.strength bits.
Signed-off-by: Eunbong Song <eunb.song@samsung.com>
---
drivers/mtd/nand/fsl_ifc_nand.c | 41 +++++++++++++++++++++++++++++++++++++++
1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 90ca7e7..2129c39 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -277,6 +277,42 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum)
return 1;
}
+static int num_zero_bits(uint8_t val)
+{
+ int i, ret=0;
+
+ for(i=7; i>=0 ; i--)
+ if(!(0x1 & (val >> i)))
+ ret++;
+
+ return ret;
+}
+
+static int is_corrupted_blank(struct mtd_info *mtd, uint8_t * buf)
+{
+ struct nand_chip *chip = mtd->priv;
+ int i;
+ int zero_bits = 0;
+
+ for (i = 0; i < mtd->writesize ; i++) {
+ if(buf[i] != 0xff) {
+ zero_bits += num_zero_bits(buf[i]);
+ }
+ }
+
+ if(zero_bits && (zero_bits <= chip->ecc.strength)){
+ return 1;
+ }
+
+ return 0;
+}
+
+static void recover_corrupted_blank(struct mtd_info *mtd, uint8_t * buf)
+{
+ memset(buf, 0xff, mtd->writesize);
+ return;
+}
+
/* returns nonzero if entire page is blank */
static int check_read_ecc(struct mtd_info *mtd, struct fsl_ifc_ctrl *ctrl,
u32 *eccstat, unsigned int bufnum)
@@ -760,6 +796,11 @@ static int fsl_ifc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
mtd->ecc_stats.failed++;
+ if(nctrl->max_bitflips && (nctrl->max_bitflips <= chip->ecc.strength)){
+ if(is_corrupted_blank(mtd, buf))
+ recover_corrupted_blank(mtd, buf);
+ }
+
return nctrl->max_bitflips;
}
--
1.7.0.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: fsl_ifc_nand: Recover corrupted empty page for preventing read-only mount in UBIFS
2014-04-01 1:49 ` Eunbong Song
@ 2014-08-21 1:52 ` Scott Wood
-1 siblings, 0 replies; 4+ messages in thread
From: Scott Wood @ 2014-08-21 1:52 UTC (permalink / raw)
To: eunb.song; +Cc: computersforpeace, artem.bityutskiy, linux-mtd, linux-kernel
On Tue, 2014-04-01 at 01:49 +0000, Eunbong Song wrote:
> Even if the meaning of EUCLEAN was changed by commit edbc4540.
> There is still possibility of read-only mount in UBIFS with ubifs_scan() "corrupt empty space at LEB".
> So i made this patch for fix that problem.
Please elaborate on the nature of the problem.
> This patch do as follow.
> - If there are ecc errors which is equal to or less than chip->ecc.strength in page.
> - Check that page has how many zero bits, and if zero bits are equal to or less than
> chip->ecc.strength then overwrite 1 to zero bits in buf.
This is difficult to parse, with no mention in this sentence that you're
talking about corrupted empty pages.
> ubifs_scan() cannot detect corrupted empty space because buf is recovered by this patch.
> And this is safe because ecc controller can correct up to chip->ecc.strength bits.
So the concern is that is_blank is failing to report a page that has not
been written to but has errors that would have been correctable if the
page had been written?
Do most drivers handle this?
> Signed-off-by: Eunbong Song <eunb.song@samsung.com>
> ---
> drivers/mtd/nand/fsl_ifc_nand.c | 41 +++++++++++++++++++++++++++++++++++++++
> 1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index 90ca7e7..2129c39 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -277,6 +277,42 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum)
> return 1;
> }
>
> +static int num_zero_bits(uint8_t val)
> +{
> + int i, ret=0;
> +
> + for(i=7; i>=0 ; i--)
> + if(!(0x1 & (val >> i)))
> + ret++;
Whitespace (here and elsewhere)
Also, use hweight8(~val) instead of reimplementing it. Or better, use
hweight64() and process the data in larger chunks.
> + return ret;
> +}
> +
> +static int is_corrupted_blank(struct mtd_info *mtd, uint8_t * buf)
> +{
> + struct nand_chip *chip = mtd->priv;
> + int i;
> + int zero_bits = 0;
> +
> + for (i = 0; i < mtd->writesize ; i++) {
> + if(buf[i] != 0xff) {
> + zero_bits += num_zero_bits(buf[i]);
> + }
> + }
> +
> + if(zero_bits && (zero_bits <= chip->ecc.strength)){
> + return 1;
> + }
> +
> + return 0;
> +}
What if it's a page that legitimately has only a handful of zero bits?
You need to count zero bits in the ECC as well.
Also, this could be combined with is_blank().
> +static void recover_corrupted_blank(struct mtd_info *mtd, uint8_t * buf)
> +{
> + memset(buf, 0xff, mtd->writesize);
> + return;
> +}
> +
> /* returns nonzero if entire page is blank */
> static int check_read_ecc(struct mtd_info *mtd, struct fsl_ifc_ctrl *ctrl,
> u32 *eccstat, unsigned int bufnum)
> @@ -760,6 +796,11 @@ static int fsl_ifc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
> mtd->ecc_stats.failed++;
>
> + if(nctrl->max_bitflips && (nctrl->max_bitflips <= chip->ecc.strength)){
> + if(is_corrupted_blank(mtd, buf))
> + recover_corrupted_blank(mtd, buf);
> + }
If the page is blank except for errors, most likely max_bitflips will be
zero because fsl_ifc_run_command() already considered it an
uncorrectable error and set ECCER instead. Moving corrupted blank page
detection into is_blank() wouldn't have this problem.
How did you test this patch?
-Scott
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: fsl_ifc_nand: Recover corrupted empty page for preventing read-only mount in UBIFS
@ 2014-08-21 1:52 ` Scott Wood
0 siblings, 0 replies; 4+ messages in thread
From: Scott Wood @ 2014-08-21 1:52 UTC (permalink / raw)
To: eunb.song; +Cc: artem.bityutskiy, computersforpeace, linux-mtd, linux-kernel
On Tue, 2014-04-01 at 01:49 +0000, Eunbong Song wrote:
> Even if the meaning of EUCLEAN was changed by commit edbc4540.
> There is still possibility of read-only mount in UBIFS with ubifs_scan() "corrupt empty space at LEB".
> So i made this patch for fix that problem.
Please elaborate on the nature of the problem.
> This patch do as follow.
> - If there are ecc errors which is equal to or less than chip->ecc.strength in page.
> - Check that page has how many zero bits, and if zero bits are equal to or less than
> chip->ecc.strength then overwrite 1 to zero bits in buf.
This is difficult to parse, with no mention in this sentence that you're
talking about corrupted empty pages.
> ubifs_scan() cannot detect corrupted empty space because buf is recovered by this patch.
> And this is safe because ecc controller can correct up to chip->ecc.strength bits.
So the concern is that is_blank is failing to report a page that has not
been written to but has errors that would have been correctable if the
page had been written?
Do most drivers handle this?
> Signed-off-by: Eunbong Song <eunb.song@samsung.com>
> ---
> drivers/mtd/nand/fsl_ifc_nand.c | 41 +++++++++++++++++++++++++++++++++++++++
> 1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index 90ca7e7..2129c39 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -277,6 +277,42 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum)
> return 1;
> }
>
> +static int num_zero_bits(uint8_t val)
> +{
> + int i, ret=0;
> +
> + for(i=7; i>=0 ; i--)
> + if(!(0x1 & (val >> i)))
> + ret++;
Whitespace (here and elsewhere)
Also, use hweight8(~val) instead of reimplementing it. Or better, use
hweight64() and process the data in larger chunks.
> + return ret;
> +}
> +
> +static int is_corrupted_blank(struct mtd_info *mtd, uint8_t * buf)
> +{
> + struct nand_chip *chip = mtd->priv;
> + int i;
> + int zero_bits = 0;
> +
> + for (i = 0; i < mtd->writesize ; i++) {
> + if(buf[i] != 0xff) {
> + zero_bits += num_zero_bits(buf[i]);
> + }
> + }
> +
> + if(zero_bits && (zero_bits <= chip->ecc.strength)){
> + return 1;
> + }
> +
> + return 0;
> +}
What if it's a page that legitimately has only a handful of zero bits?
You need to count zero bits in the ECC as well.
Also, this could be combined with is_blank().
> +static void recover_corrupted_blank(struct mtd_info *mtd, uint8_t * buf)
> +{
> + memset(buf, 0xff, mtd->writesize);
> + return;
> +}
> +
> /* returns nonzero if entire page is blank */
> static int check_read_ecc(struct mtd_info *mtd, struct fsl_ifc_ctrl *ctrl,
> u32 *eccstat, unsigned int bufnum)
> @@ -760,6 +796,11 @@ static int fsl_ifc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
> mtd->ecc_stats.failed++;
>
> + if(nctrl->max_bitflips && (nctrl->max_bitflips <= chip->ecc.strength)){
> + if(is_corrupted_blank(mtd, buf))
> + recover_corrupted_blank(mtd, buf);
> + }
If the page is blank except for errors, most likely max_bitflips will be
zero because fsl_ifc_run_command() already considered it an
uncorrectable error and set ECCER instead. Moving corrupted blank page
detection into is_blank() wouldn't have this problem.
How did you test this patch?
-Scott
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-08-21 1:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-01 1:49 [PATCH] mtd: fsl_ifc_nand: Recover corrupted empty page for preventing read-only mount in UBIFS Eunbong Song
2014-04-01 1:49 ` Eunbong Song
2014-08-21 1:52 ` Scott Wood
2014-08-21 1:52 ` Scott Wood
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.