diff mbox series

[v5,1/3] mtd: mtdpart: Make ECC stat handling consistent

Message ID 20180109085035.12438-1-boris.brezillon@free-electrons.com
State Accepted
Delegated to: Boris Brezillon
Headers show
Series [v5,1/3] mtd: mtdpart: Make ECC stat handling consistent | expand

Commit Message

Boris Brezillon Jan. 9, 2018, 8:50 a.m. UTC
part_read() and part_read_oob() where counting ECC failures and
bitflips differently. Adjust part_read_oob() to mimic what is done in
part_read(). This is in needed to use ->_read_oob() as a fallback when
when ->_read() is not implemented.

Note that bitflips and ECC failure accounting on MTD partitions is
broken by design, because nothing prevents concurrent accesses to the
underlying master MTD device between the moment we save the stats in a
local variable and the moment master->_read[_oob]() returns. It's not
something that can easily be fixed, so leave it like that for now.

Suggested-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Ladislav Michl <ladis@linux-mips.org>
---
Changes in v5:
- save master's ECC stats before calling ->_read_oob()

Changes in v4:
- new patch
---
 drivers/mtd/mtdpart.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Boris Brezillon Jan. 11, 2018, 9:06 a.m. UTC | #1
On Tue,  9 Jan 2018 09:50:33 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> part_read() and part_read_oob() where counting ECC failures and
> bitflips differently. Adjust part_read_oob() to mimic what is done in
> part_read(). This is in needed to use ->_read_oob() as a fallback when
> when ->_read() is not implemented.
> 
> Note that bitflips and ECC failure accounting on MTD partitions is
> broken by design, because nothing prevents concurrent accesses to the
> underlying master MTD device between the moment we save the stats in a
> local variable and the moment master->_read[_oob]() returns. It's not
> something that can easily be fixed, so leave it like that for now.

Applied the series (also fixed the typo reported by Robert).

> 
> Suggested-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Tested-by: Ladislav Michl <ladis@linux-mips.org>
> ---
> Changes in v5:
> - save master's ECC stats before calling ->_read_oob()
> 
> Changes in v4:
> - new patch
> ---
>  drivers/mtd/mtdpart.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index be088bccd593..79bf1f61c7a0 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -105,6 +105,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
>  		struct mtd_oob_ops *ops)
>  {
>  	struct mtd_part *part = mtd_to_part(mtd);
> +	struct mtd_ecc_stats stats;
>  	int res;
>  
>  	if (from >= mtd->size)
> @@ -126,13 +127,14 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
>  			return -EINVAL;
>  	}
>  
> +	stats = part->parent->ecc_stats;
>  	res = part->parent->_read_oob(part->parent, from + part->offset, ops);
> -	if (unlikely(res)) {
> -		if (mtd_is_bitflip(res))
> -			mtd->ecc_stats.corrected++;
> -		if (mtd_is_eccerr(res))
> -			mtd->ecc_stats.failed++;
> -	}
> +	if (unlikely(mtd_is_eccerr(res)))
> +		mtd->ecc_stats.failed +=
> +			part->parent->ecc_stats.failed - stats.failed;
> +	else
> +		mtd->ecc_stats.corrected +=
> +			part->parent->ecc_stats.corrected - stats.corrected;
>  	return res;
>  }
>
diff mbox series

Patch

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index be088bccd593..79bf1f61c7a0 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -105,6 +105,7 @@  static int part_read_oob(struct mtd_info *mtd, loff_t from,
 		struct mtd_oob_ops *ops)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
+	struct mtd_ecc_stats stats;
 	int res;
 
 	if (from >= mtd->size)
@@ -126,13 +127,14 @@  static int part_read_oob(struct mtd_info *mtd, loff_t from,
 			return -EINVAL;
 	}
 
+	stats = part->parent->ecc_stats;
 	res = part->parent->_read_oob(part->parent, from + part->offset, ops);
-	if (unlikely(res)) {
-		if (mtd_is_bitflip(res))
-			mtd->ecc_stats.corrected++;
-		if (mtd_is_eccerr(res))
-			mtd->ecc_stats.failed++;
-	}
+	if (unlikely(mtd_is_eccerr(res)))
+		mtd->ecc_stats.failed +=
+			part->parent->ecc_stats.failed - stats.failed;
+	else
+		mtd->ecc_stats.corrected +=
+			part->parent->ecc_stats.corrected - stats.corrected;
 	return res;
 }