Re: [PATCH 1/2] pinctrl: pinconf-generic: resize the pin config array directly

[Masahiro Yamada <masahiroy@xxxxxxxxxx>]

On Tue, Nov 21, 2023 at 7:28 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> pinconf_generic_parse_dt_config() allocates memory that is large enough
> to contain all the config parameters. Then, kmemdup() copies the found
> configs to the memory with the exact size.
>
> There is no need to allocate memory twice; you can directly resize the
> initial memory using krealloc_array().
>
> I also changed kcalloc() to kmalloc_array() to keep the consistency with
> krealloc_array(). This change has no impact because you do not need to
> zero out the 'cfg' array.
>
> Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>



Sorry, I retract this patch set.

krealloc() does not save any memory
when the new_size is smaller than the current size.













> ---
>
>  drivers/pinctrl/pinconf-generic.c | 25 ++++++++-----------------
>  1 file changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
> index 8313cb5f3b3c..ba4fe2466e78 100644
> --- a/drivers/pinctrl/pinconf-generic.c
> +++ b/drivers/pinctrl/pinconf-generic.c
> @@ -247,7 +247,6 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
>  {
>         unsigned long *cfg;
>         unsigned int max_cfg, ncfg = 0;
> -       int ret;
>
>         if (!np)
>                 return -EINVAL;
> @@ -256,7 +255,7 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
>         max_cfg = ARRAY_SIZE(dt_params);
>         if (pctldev)
>                 max_cfg += pctldev->desc->num_custom_params;
> -       cfg = kcalloc(max_cfg, sizeof(*cfg), GFP_KERNEL);
> +       cfg = kmalloc_array(max_cfg, sizeof(*cfg), GFP_KERNEL);
>         if (!cfg)
>                 return -ENOMEM;
>
> @@ -266,30 +265,22 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
>                 parse_dt_cfg(np, pctldev->desc->custom_params,
>                              pctldev->desc->num_custom_params, cfg, &ncfg);
>
> -       ret = 0;
> -
>         /* no configs found at all */
>         if (ncfg == 0) {
> +               kfree(cfg);
>                 *configs = NULL;
>                 *nconfigs = 0;
> -               goto out;
> +               return 0;
>         }
>
> -       /*
> -        * Now limit the number of configs to the real number of
> -        * found properties.
> -        */
> -       *configs = kmemdup(cfg, ncfg * sizeof(unsigned long), GFP_KERNEL);
> -       if (!*configs) {
> -               ret = -ENOMEM;
> -               goto out;
> -       }
> +       /* Now resize the array to store the real number of found properties. */
> +       *configs = krealloc_array(cfg, ncfg, sizeof(unsigned long), GFP_KERNEL);
> +       if (!*configs)
> +               return -ENOMEM;
>
>         *nconfigs = ncfg;
>
> -out:
> -       kfree(cfg);
> -       return ret;
> +       return 0;
>  }
>  EXPORT_SYMBOL_GPL(pinconf_generic_parse_dt_config);
>
> --
> 2.40.1
>


-- 
Best Regards
Masahiro Yamada