diff mbox series

[2/2] mtd: nand: Only allocate ecc->{calc, code}_buf when actually needed

Message ID 20171205110929.6820-2-boris.brezillon@free-electrons.com
State Accepted
Delegated to: Boris Brezillon
Headers show
Series [1/2] mtd: nand: denali: Avoid using ecc->code_buf as a temporary buffer | expand

Commit Message

Boris Brezillon Dec. 5, 2017, 11:09 a.m. UTC
The only users of the ecc->{calc,code}_buf buffers are NAND controller
drivers implementing ecc->calculate() and/or ecc->correct(). Since the
->oobsize can be non-negligle, especially on modern NAND devices, we'd
better allocate it only when it is actually required.

Make ecc->{calc,code}_buf allocation dependent on the presence of
ecc->calculate() or ecc->correct().

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

Comments

Masahiro Yamada Dec. 5, 2017, 3:29 p.m. UTC | #1
2017-12-05 20:09 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> The only users of the ecc->{calc,code}_buf buffers are NAND controller
> drivers implementing ecc->calculate() and/or ecc->correct(). Since the
> ->oobsize can be non-negligle, especially on modern NAND devices, we'd
> better allocate it only when it is actually required.
>
> Make ecc->{calc,code}_buf allocation dependent on the presence of
> ecc->calculate() or ecc->correct().
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/mtd/nand/nand_base.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 71e70c4964f3..e6873d10c574 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -5318,21 +5318,9 @@ int nand_scan_tail(struct mtd_info *mtd)
>                 return -EINVAL;
>         }
>
> -       ecc->calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> -       if (!ecc->calc_buf)
> -               return -ENOMEM;
> -
> -       ecc->code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> -       if (!ecc->code_buf) {
> -               ret = -ENOMEM;
> -               goto err_free_buf;
> -       }
> -
>         chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
> -       if (!chip->data_buf) {
> -               ret = -ENOMEM;
> -               goto err_free_buf;
> -       }
> +       if (!chip->data_buf)
> +               return -ENOMEM;
>
>         /*
>          * FIXME: some NAND manufacturer drivers expect the first die to be
> @@ -5495,6 +5483,15 @@ int nand_scan_tail(struct mtd_info *mtd)
>                 goto err_nand_manuf_cleanup;
>         }
>
> +       if (ecc->correct || ecc->calculate) {
> +               ecc->calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> +               ecc->code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> +               if (!ecc->calc_buf || !ecc->code_buf) {
> +                       ret = -ENOMEM;
> +                       goto err_nand_manuf_cleanup;
> +               }
> +       }
> +
>         /* For many systems, the standard OOB write also works for raw */
>         if (!ecc->read_oob_raw)
>                 ecc->read_oob_raw = ecc->read_oob;
> --
> 2.11.0


I wondered if chip->data_buf allocation could be moved as well,
but otherwise, looks good to me.


Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>
diff mbox series

Patch

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 71e70c4964f3..e6873d10c574 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -5318,21 +5318,9 @@  int nand_scan_tail(struct mtd_info *mtd)
 		return -EINVAL;
 	}
 
-	ecc->calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
-	if (!ecc->calc_buf)
-		return -ENOMEM;
-
-	ecc->code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
-	if (!ecc->code_buf) {
-		ret = -ENOMEM;
-		goto err_free_buf;
-	}
-
 	chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
-	if (!chip->data_buf) {
-		ret = -ENOMEM;
-		goto err_free_buf;
-	}
+	if (!chip->data_buf)
+		return -ENOMEM;
 
 	/*
 	 * FIXME: some NAND manufacturer drivers expect the first die to be
@@ -5495,6 +5483,15 @@  int nand_scan_tail(struct mtd_info *mtd)
 		goto err_nand_manuf_cleanup;
 	}
 
+	if (ecc->correct || ecc->calculate) {
+		ecc->calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
+		ecc->code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
+		if (!ecc->calc_buf || !ecc->code_buf) {
+			ret = -ENOMEM;
+			goto err_nand_manuf_cleanup;
+		}
+	}
+
 	/* For many systems, the standard OOB write also works for raw */
 	if (!ecc->read_oob_raw)
 		ecc->read_oob_raw = ecc->read_oob;