Re: [PATCH v2] nvdimm: ndtest: Return -ENOMEM if devm_kcalloc() fails in ndtest_probe()

[Dave Jiang]

On 9/24/25 12:42 AM, Guangshuo Li wrote:
> Hi Alison, Dave, and all,
> 
> Thanks for the feedback. I’ve adopted your suggestions. Below is what I plan 
> to take in v3.
> 
> -       p->dcr_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> -                                 sizeof(dma_addr_t), GFP_KERNEL);
> -       p->label_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> -                                   sizeof(dma_addr_t), GFP_KERNEL);
> -       p->dimm_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> -                                  sizeof(dma_addr_t), GFP_KERNEL);
> 
> +       p->dcr_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> +                                 sizeof(dma_addr_t), GFP_KERNEL);
> +       if (!p->dcr_dma) {
> +               rc = -ENOMEM;
> +               goto err;
> +       }
> +
> +       p->label_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> +                                   sizeof(dma_addr_t), GFP_KERNEL);
> +       if (!p->label_dma) {
> +               rc = -ENOMEM;
> +               goto err;
> +       }
> +
> +       p->dimm_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> +                                  sizeof(dma_addr_t), GFP_KERNEL);
> +       if (!p->dimm_dma) {
> +               rc = -ENOMEM;
> +               goto err;
> +       }

You'll need to create new goto labels because you'll have to free previously 
allocated memory in the error path. Diff below is uncompiled and untested.

diff --git a/tools/testing/nvdimm/test/ndtest.c 
b/tools/testing/nvdimm/test/ndtest.c
index 68a064ce598c..49d326819ea9 100644
--- a/tools/testing/nvdimm/test/ndtest.c
+++ b/tools/testing/nvdimm/test/ndtest.c
@@ -841,6 +841,7 @@ static void ndtest_remove(struct platform_device *pdev)

 static int ndtest_probe(struct platform_device *pdev)
 {
+       struct device *dev = &pdev->dev;
        struct ndtest_priv *p;
        int rc;

@@ -848,12 +849,23 @@ static int ndtest_probe(struct platform_device *pdev)
        if (ndtest_bus_register(p))
                return -ENOMEM;

-       p->dcr_dma = devm_kcalloc(&p->pdev.dev, NUM_DCR,
-                                sizeof(dma_addr_t), GFP_KERNEL);
-       p->label_dma = devm_kcalloc(&p->pdev.dev, NUM_DCR,
-                                  sizeof(dma_addr_t), GFP_KERNEL);
-       p->dimm_dma = devm_kcalloc(&p->pdev.dev, NUM_DCR,
-                                 sizeof(dma_addr_t), GFP_KERNEL);
+       p->dcr_dma = devm_kcalloc(dev, NUM_DCR, sizeof(dma_addr_t), GFP_KERNEL);
+       if (!p->dcr_dma)
+               return -ENOMEM;
+
+       p->label_dma = devm_kcalloc(dev, NUM_DCR, sizeof(dma_addr_t),
+                                   GFP_KERNEL);
+       if (!p->label_dma) {
+               rc = -ENOMEM;
+               goto err_label_dma;
+       }
+
+       p->dimm_dma = devm_kcalloc(dev, NUM_DCR, sizeof(dma_addr_t),
+                                  GFP_KERNEL);
+       if (!p->dimm_dma) {
+               rc = -ENOMEM;
+               goto err_dimm_dma;
+       }

        rc = ndtest_nvdimm_init(p);
        if (rc)
@@ -863,7 +875,7 @@ static int ndtest_probe(struct platform_device *pdev)
        if (rc)
                goto err;

-       rc = devm_add_action_or_reset(&pdev->dev, put_dimms, p);
+       rc = devm_add_action_or_reset(dev, put_dimms, p);
        if (rc)
                goto err;
@@ -872,6 +884,11 @@ static int ndtest_probe(struct platform_device *pdev)
        return 0;

 err:
+       devm_kfree(dev, p->dimm_dma);
+err_dimm_dma:
+       devm_kfree(dev, p->label_dma);
+err_label_dma:
+       devm_kfree(dev, p->dcr_dma);
        pr_err("%s:%d Failed nvdimm init\n", __func__, __LINE__);
        return rc;
 }

> 
> If this looks good, I’ll send v3 accordingly. Also, if you’re comfortable 
> with the changes, may I add your Reviewed-by tags?

Please don't add review tags until they are given.

> 
> Best regards,
> Guangshuo