| Message ID | 1481123711-7205-1-git-send-email-arvind.yadav.cs@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On Wed, 7 Dec 2016 20:45:11 +0530 Arvind Yadav <arvind.yadav.cs@gmail.com> wrote: > In function denali_init, the memory allocated for denali->buf.buf > is live within the function only. After the allocation it is > immediately freed with devm_kfree. There is no need to allocate > memory for denali->buf.buf with devm function so replace > devm_kzalloc with kzalloc and devm_kfree with kfree. > > Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> > --- > drivers/mtd/nand/denali.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) Not sure it's any better than the previous logic (see the diffstat). The devm_kzalloc() approach has the benefit of simplifying the different error paths. > > diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c > index 0476ae8..ce3c31f 100644 > --- a/drivers/mtd/nand/denali.c > +++ b/drivers/mtd/nand/denali.c > @@ -1476,8 +1476,7 @@ int denali_init(struct denali_nand_info *denali) > } > > /* allocate a temporary buffer for nand_scan_ident() */ > - denali->buf.buf = devm_kzalloc(denali->dev, PAGE_SIZE, > - GFP_DMA | GFP_KERNEL); > + denali->buf.buf = kzalloc(PAGE_SIZE, GFP_DMA | GFP_KERNEL); > if (!denali->buf.buf) > return -ENOMEM; > > @@ -1492,6 +1491,7 @@ int denali_init(struct denali_nand_info *denali) > if (request_irq(denali->irq, denali_isr, IRQF_SHARED, > DENALI_NAND_NAME, denali)) { > pr_err("Spectra: Unable to allocate IRQ\n"); > + kfree(denali->buf.buf); > return -ENODEV; > } > > @@ -1512,11 +1512,12 @@ int denali_init(struct denali_nand_info *denali) > */ > if (nand_scan_ident(mtd, denali->max_banks, NULL)) { > ret = -ENXIO; > + kfree(denali->buf.buf); > goto failed_req_irq; > } > > /* allocate the right size buffer now */ > - devm_kfree(denali->dev, denali->buf.buf); > + kfree(denali->buf.buf); > denali->buf.buf = devm_kzalloc(denali->dev, > mtd->writesize + mtd->oobsize, > GFP_KERNEL);
yes, if Memory is live out side function. Then devm_kzalloc() approach has the benefit of simplifying the different error paths. Here, Memory is alive with in function. we are going to free allocate memory then why we need devm api. In this case Devm will first add this entry to list and immediately it will remove from list. In this case, It's just a overhead for devm api. -Arvind On Thursday 29 December 2016 10:54 PM, Boris Brezillon wrote: > The > devm_kzalloc() approach has the benefit of simplifying the different > error paths.
On Mon, 2 Jan 2017 13:26:01 +0530 Arvind Yadav <arvind.yadav.cs@gmail.com> wrote: > yes, if Memory is live out side function. Then devm_kzalloc() > approach has the benefit of simplifying the different error paths. > > Here, Memory is alive with in function. we are going to free allocate memory > then why we need devm api. In this case Devm will first add this entry to > list and immediately it will remove from list. In this case, It's just a > overhead > for devm api. Yes, it adds a small overhead, but ITOH, it simplifies the code (see the kfree() calls you added in different error paths with your approach). Sometime a small runtime overhead (especially when the code is executed once at probe time) is acceptable if it improves readability.
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c index 0476ae8..ce3c31f 100644 --- a/drivers/mtd/nand/denali.c +++ b/drivers/mtd/nand/denali.c @@ -1476,8 +1476,7 @@ int denali_init(struct denali_nand_info *denali) } /* allocate a temporary buffer for nand_scan_ident() */ - denali->buf.buf = devm_kzalloc(denali->dev, PAGE_SIZE, - GFP_DMA | GFP_KERNEL); + denali->buf.buf = kzalloc(PAGE_SIZE, GFP_DMA | GFP_KERNEL); if (!denali->buf.buf) return -ENOMEM; @@ -1492,6 +1491,7 @@ int denali_init(struct denali_nand_info *denali) if (request_irq(denali->irq, denali_isr, IRQF_SHARED, DENALI_NAND_NAME, denali)) { pr_err("Spectra: Unable to allocate IRQ\n"); + kfree(denali->buf.buf); return -ENODEV; } @@ -1512,11 +1512,12 @@ int denali_init(struct denali_nand_info *denali) */ if (nand_scan_ident(mtd, denali->max_banks, NULL)) { ret = -ENXIO; + kfree(denali->buf.buf); goto failed_req_irq; } /* allocate the right size buffer now */ - devm_kfree(denali->dev, denali->buf.buf); + kfree(denali->buf.buf); denali->buf.buf = devm_kzalloc(denali->dev, mtd->writesize + mtd->oobsize, GFP_KERNEL);
In function denali_init, the memory allocated for denali->buf.buf is live within the function only. After the allocation it is immediately freed with devm_kfree. There is no need to allocate memory for denali->buf.buf with devm function so replace devm_kzalloc with kzalloc and devm_kfree with kfree. Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> --- drivers/mtd/nand/denali.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)