2023-10-17 08:29:41

by Wilczynski, Michal

[permalink] [raw]
Subject: [PATCH v3] ACPI: NFIT: Use cleanup.h helpers instead of devm_*()

The new cleanup.h facilities that arrived in v6.5-rc1 can replace the
the usage of devm semantics in acpi_nfit_init_interleave_set(). That
routine appears to only be using devm to avoid goto statements. The
new __free() annotation at variable declaration time can achieve the same
effect more efficiently.

There is no end user visible side effects of this patch, I was
motivated to send this cleanup to practice using the new helpers.

Suggested-by: Dave Jiang <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Dave Jiang <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Michal Wilczynski <[email protected]>
---

Dan, would you like me to give you credit for the changelog changes
with Co-developed-by tag ?

v3:
- changed changelog
v2:
- removed first commit from the patchset, as the commit couldn't
be marked as a fix
- squashed those commits together, since the second one were
mostly overwriting the previous one

drivers/acpi/nfit/core.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 3826f49d481b..67a844a705c4 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2257,26 +2257,23 @@ static int acpi_nfit_init_interleave_set(struct acpi_nfit_desc *acpi_desc,
struct nd_region_desc *ndr_desc,
struct acpi_nfit_system_address *spa)
{
+ u16 nr = ndr_desc->num_mappings;
+ struct nfit_set_info2 *info2 __free(kfree) =
+ kcalloc(nr, sizeof(*info2), GFP_KERNEL);
+ struct nfit_set_info *info __free(kfree) =
+ kcalloc(nr, sizeof(*info), GFP_KERNEL);
struct device *dev = acpi_desc->dev;
struct nd_interleave_set *nd_set;
- u16 nr = ndr_desc->num_mappings;
- struct nfit_set_info2 *info2;
- struct nfit_set_info *info;
int i;

+ if (!info || !info2)
+ return -ENOMEM;
+
nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
if (!nd_set)
return -ENOMEM;
import_guid(&nd_set->type_guid, spa->range_guid);

- info = devm_kcalloc(dev, nr, sizeof(*info), GFP_KERNEL);
- if (!info)
- return -ENOMEM;
-
- info2 = devm_kcalloc(dev, nr, sizeof(*info2), GFP_KERNEL);
- if (!info2)
- return -ENOMEM;
-
for (i = 0; i < nr; i++) {
struct nd_mapping_desc *mapping = &ndr_desc->mapping[i];
struct nvdimm *nvdimm = mapping->nvdimm;
@@ -2337,8 +2334,6 @@ static int acpi_nfit_init_interleave_set(struct acpi_nfit_desc *acpi_desc,
}

ndr_desc->nd_set = nd_set;
- devm_kfree(dev, info);
- devm_kfree(dev, info2);

return 0;
}
--
2.41.0


2023-10-18 04:28:48

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v3] ACPI: NFIT: Use cleanup.h helpers instead of devm_*()

Michal Wilczynski wrote:
> The new cleanup.h facilities that arrived in v6.5-rc1 can replace the
> the usage of devm semantics in acpi_nfit_init_interleave_set(). That
> routine appears to only be using devm to avoid goto statements. The
> new __free() annotation at variable declaration time can achieve the same
> effect more efficiently.
>
> There is no end user visible side effects of this patch, I was
> motivated to send this cleanup to practice using the new helpers.
>
> Suggested-by: Dave Jiang <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Dave Jiang <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Michal Wilczynski <[email protected]>
> ---
>
> Dan, would you like me to give you credit for the changelog changes
> with Co-developed-by tag ?

Nope, this looks good to me, thanks for fixing it up.

Reviewed-by: Dan Williams <[email protected]>

2023-10-18 10:40:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3] ACPI: NFIT: Use cleanup.h helpers instead of devm_*()

On Wed, Oct 18, 2023 at 6:28 AM Dan Williams <[email protected]> wrote:
>
> Michal Wilczynski wrote:
> > The new cleanup.h facilities that arrived in v6.5-rc1 can replace the
> > the usage of devm semantics in acpi_nfit_init_interleave_set(). That
> > routine appears to only be using devm to avoid goto statements. The
> > new __free() annotation at variable declaration time can achieve the same
> > effect more efficiently.
> >
> > There is no end user visible side effects of this patch, I was
> > motivated to send this cleanup to practice using the new helpers.
> >
> > Suggested-by: Dave Jiang <[email protected]>
> > Suggested-by: Andy Shevchenko <[email protected]>
> > Reviewed-by: Dave Jiang <[email protected]>
> > Reviewed-by: Andy Shevchenko <[email protected]>
> > Signed-off-by: Michal Wilczynski <[email protected]>
> > ---
> >
> > Dan, would you like me to give you credit for the changelog changes
> > with Co-developed-by tag ?
>
> Nope, this looks good to me, thanks for fixing it up.
>
> Reviewed-by: Dan Williams <[email protected]>

Are you going to apply it too, or should I take it?

2023-12-21 21:26:19

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH v3] ACPI: NFIT: Use cleanup.h helpers instead of devm_*()

Rafael J. Wysocki wrote:
> On Wed, Oct 18, 2023 at 6:28 AM Dan Williams <[email protected]> wrote:
> >
> > Michal Wilczynski wrote:
> > > The new cleanup.h facilities that arrived in v6.5-rc1 can replace the
> > > the usage of devm semantics in acpi_nfit_init_interleave_set(). That
> > > routine appears to only be using devm to avoid goto statements. The
> > > new __free() annotation at variable declaration time can achieve the same
> > > effect more efficiently.
> > >
> > > There is no end user visible side effects of this patch, I was
> > > motivated to send this cleanup to practice using the new helpers.
> > >
> > > Suggested-by: Dave Jiang <[email protected]>
> > > Suggested-by: Andy Shevchenko <[email protected]>
> > > Reviewed-by: Dave Jiang <[email protected]>
> > > Reviewed-by: Andy Shevchenko <[email protected]>
> > > Signed-off-by: Michal Wilczynski <[email protected]>
> > > ---
> > >
> > > Dan, would you like me to give you credit for the changelog changes
> > > with Co-developed-by tag ?
> >
> > Nope, this looks good to me, thanks for fixing it up.
> >
> > Reviewed-by: Dan Williams <[email protected]>
>
> Are you going to apply it too, or should I take it?

I'm prepping the nvdimm tree for 6.8. I will take it.

Ira