[PATCH next 1/2] mtd: rawnand: mtk: Factorize out the logic cleaning mtk chips

[PATCH next 1/2] mtd: rawnand: mtk: Factorize out the logic cleaning mtk chips

[Miquel Raynal]

There are some un-freed resources in one of the error path which would
benefit from a helper going through all the registered mtk chips one by
one and perform all the necessary cleanup. This is precisely what the
remove path does, so let's extract the logic in a helper.

There is no functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/mtk_nand.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index d65e6371675b..bf845dd16737 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -1429,6 +1429,23 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 	return 0;
 }
 
+static void mtk_nfc_nand_chips_cleanup(struct mtk_nfc *nfc)
+{
+	struct mtk_nfc_nand_chip *mtk_chip;
+	struct nand_chip *chip;
+	int ret;
+
+	while (!list_empty(&nfc->chips)) {
+		mtk_chip = list_first_entry(&nfc->chips,
+					    struct mtk_nfc_nand_chip, node);
+		chip = &mtk_chip->nand;
+		ret = mtd_device_unregister(nand_to_mtd(chip));
+		WARN_ON(ret);
+		nand_cleanup(chip);
+		list_del(&mtk_chip->node);
+	}
+}
+
 static int mtk_nfc_nand_chips_init(struct device *dev, struct mtk_nfc *nfc)
 {
 	struct device_node *np = dev->of_node;
@@ -1567,20 +1584,8 @@ static int mtk_nfc_probe(struct platform_device *pdev)
 static void mtk_nfc_remove(struct platform_device *pdev)
 {
 	struct mtk_nfc *nfc = platform_get_drvdata(pdev);
-	struct mtk_nfc_nand_chip *mtk_chip;
-	struct nand_chip *chip;
-	int ret;
-
-	while (!list_empty(&nfc->chips)) {
-		mtk_chip = list_first_entry(&nfc->chips,
-					    struct mtk_nfc_nand_chip, node);
-		chip = &mtk_chip->nand;
-		ret = mtd_device_unregister(nand_to_mtd(chip));
-		WARN_ON(ret);
-		nand_cleanup(chip);
-		list_del(&mtk_chip->node);
-	}
 
+	mtk_nfc_nand_chips_cleanup(nfc);
 	mtk_ecc_release(nfc->ecc);
 }
 
-- 
2.43.0

[PATCH next 2/2] mtd: rawnand: mtk: Fix init error path

[Miquel Raynal]

Reviewing a series converting the for_each_chil_of_node() loops into
their _scoped variants made me realize there was no cleanup of the
already registered NAND devices upon error which may leak memory on
systems with more than a chip when this error occurs. We should call the
_nand_chips_cleanup() function when this happens.

Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Backporting this change will probably fail. In practice, the memory leak
is very unlikely to happen so I guess we can live without it.
---
 drivers/mtd/nand/raw/mtk_nand.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index bf845dd16737..586868b4139f 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -1453,8 +1453,10 @@ static int mtk_nfc_nand_chips_init(struct device *dev, struct mtk_nfc *nfc)
 
 	for_each_child_of_node_scoped(np, nand_np) {
 		ret = mtk_nfc_nand_chip_init(dev, nfc, nand_np);
-		if (ret)
+		if (ret) {
+			mtk_nfc_nand_chips_cleanup(nfc);
 			return ret;
+		}
 	}
 
 	return 0;
-- 
2.43.0

Re: [PATCH next 2/2] mtd: rawnand: mtk: Fix init error path

[Pratyush Yadav]

On Mon, Aug 26 2024, Miquel Raynal wrote:

> Reviewing a series converting the for_each_chil_of_node() loops into
> their _scoped variants made me realize there was no cleanup of the
> already registered NAND devices upon error which may leak memory on
> systems with more than a chip when this error occurs. We should call the
> _nand_chips_cleanup() function when this happens.
>
> Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Did a quick scan. Can't see anything (obvious) else being leaked on
error paths.

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav

Re: [PATCH next 1/2] mtd: rawnand: mtk: Factorize out the logic cleaning mtk chips

[Pratyush Yadav]

On Mon, Aug 26 2024, Miquel Raynal wrote:

> There are some un-freed resources in one of the error path which would
> benefit from a helper going through all the registered mtk chips one by
> one and perform all the necessary cleanup. This is precisely what the
> remove path does, so let's extract the logic in a helper.
>
> There is no functional change.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav

Re: [PATCH next 2/2] mtd: rawnand: mtk: Fix init error path

[Miquel Raynal]

Hi Pratyush,

pratyush@kernel.org wrote on Wed, 04 Sep 2024 16:08:15 +0200:

> On Mon, Aug 26 2024, Miquel Raynal wrote:
> 
> > Reviewing a series converting the for_each_chil_of_node() loops into
> > their _scoped variants made me realize there was no cleanup of the
> > already registered NAND devices upon error which may leak memory on
> > systems with more than a chip when this error occurs. We should call the
> > _nand_chips_cleanup() function when this happens.
> >
> > Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> Did a quick scan. Can't see anything (obvious) else being leaked on
> error paths.

Great! Thanks a lot for the reviews!

> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

Cheers,
Miquèl

Re: [PATCH next 1/2] mtd: rawnand: mtk: Factorize out the logic cleaning mtk chips

[Matthias Brugger]

On 26/08/2024 17:30, Miquel Raynal wrote:
> There are some un-freed resources in one of the error path which would
> benefit from a helper going through all the registered mtk chips one by
> one and perform all the necessary cleanup. This is precisely what the
> remove path does, so let's extract the logic in a helper.
> 
> There is no functional change.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Matthias Brugger <matthias.bgg@kernel.org>

> ---
>   drivers/mtd/nand/raw/mtk_nand.c | 31 ++++++++++++++++++-------------
>   1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index d65e6371675b..bf845dd16737 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -1429,6 +1429,23 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
>   	return 0;
>   }
>   
> +static void mtk_nfc_nand_chips_cleanup(struct mtk_nfc *nfc)
> +{
> +	struct mtk_nfc_nand_chip *mtk_chip;
> +	struct nand_chip *chip;
> +	int ret;
> +
> +	while (!list_empty(&nfc->chips)) {
> +		mtk_chip = list_first_entry(&nfc->chips,
> +					    struct mtk_nfc_nand_chip, node);
> +		chip = &mtk_chip->nand;
> +		ret = mtd_device_unregister(nand_to_mtd(chip));
> +		WARN_ON(ret);
> +		nand_cleanup(chip);
> +		list_del(&mtk_chip->node);
> +	}
> +}
> +
>   static int mtk_nfc_nand_chips_init(struct device *dev, struct mtk_nfc *nfc)
>   {
>   	struct device_node *np = dev->of_node;
> @@ -1567,20 +1584,8 @@ static int mtk_nfc_probe(struct platform_device *pdev)
>   static void mtk_nfc_remove(struct platform_device *pdev)
>   {
>   	struct mtk_nfc *nfc = platform_get_drvdata(pdev);
> -	struct mtk_nfc_nand_chip *mtk_chip;
> -	struct nand_chip *chip;
> -	int ret;
> -
> -	while (!list_empty(&nfc->chips)) {
> -		mtk_chip = list_first_entry(&nfc->chips,
> -					    struct mtk_nfc_nand_chip, node);
> -		chip = &mtk_chip->nand;
> -		ret = mtd_device_unregister(nand_to_mtd(chip));
> -		WARN_ON(ret);
> -		nand_cleanup(chip);
> -		list_del(&mtk_chip->node);
> -	}
>   
> +	mtk_nfc_nand_chips_cleanup(nfc);
>   	mtk_ecc_release(nfc->ecc);
>   }
>   

Re: [PATCH next 2/2] mtd: rawnand: mtk: Fix init error path

[Matthias Brugger]

On 26/08/2024 17:30, Miquel Raynal wrote:
> Reviewing a series converting the for_each_chil_of_node() loops into
> their _scoped variants made me realize there was no cleanup of the
> already registered NAND devices upon error which may leak memory on
> systems with more than a chip when this error occurs. We should call the
> _nand_chips_cleanup() function when this happens.
> 
> Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Matthias Brugger <matthias.bgg@kernel.org>

> ---
> Backporting this change will probably fail. In practice, the memory leak
> is very unlikely to happen so I guess we can live without it.
> ---
>   drivers/mtd/nand/raw/mtk_nand.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index bf845dd16737..586868b4139f 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -1453,8 +1453,10 @@ static int mtk_nfc_nand_chips_init(struct device *dev, struct mtk_nfc *nfc)
>   
>   	for_each_child_of_node_scoped(np, nand_np) {
>   		ret = mtk_nfc_nand_chip_init(dev, nfc, nand_np);
> -		if (ret)
> +		if (ret) {
> +			mtk_nfc_nand_chips_cleanup(nfc);
>   			return ret;
> +		}
>   	}
>   
>   	return 0;

Re: [PATCH next 2/2] mtd: rawnand: mtk: Fix init error path

[Miquel Raynal]

On Mon, 2024-08-26 at 15:30:19 UTC, Miquel Raynal wrote:
> Reviewing a series converting the for_each_chil_of_node() loops into
> their _scoped variants made me realize there was no cleanup of the
> already registered NAND devices upon error which may leak memory on
> systems with more than a chip when this error occurs. We should call the
> _nand_chips_cleanup() function when this happens.
> 
> Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next.

Miquel

Re: [PATCH next 1/2] mtd: rawnand: mtk: Factorize out the logic cleaning mtk chips

[Miquel Raynal]

On Mon, 2024-08-26 at 15:30:18 UTC, Miquel Raynal wrote:
> There are some un-freed resources in one of the error path which would
> benefit from a helper going through all the registered mtk chips one by
> one and perform all the necessary cleanup. This is precisely what the
> remove path does, so let's extract the logic in a helper.
> 
> There is no functional change.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next.

Miquel