linux-kernel - Re: [PATCH v2 1/2] PM: hibernate: dynamically allocate crc->unc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0hL_q9t2Tdu5DVZNqV_YkNpofV9S+N-rRRrAY3er5X_7Q@mail.gmail.com>
Date: Thu, 11 Sep 2025 15:10:39 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Xueqin Luo <luoxueqin@...inos.cn>
Cc: rafael@...nel.org, pavel@...nel.org, lenb@...nel.org, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] PM: hibernate: dynamically allocate
 crc->unc_len/unc for configurable threads

On Thu, Sep 11, 2025 at 10:10 AM Xueqin Luo <luoxueqin@...inos.cn> wrote:
>
> The current implementation uses fixed-size arrays for crc->unc_len and
> crc->unc, which limits the number of compression threads to a compile-time
> constant (CMP_THREADS). This patch converts them to dynamically allocated
> arrays, sized according to the actual number of threads selected at runtime.

Please don't say "this patch" (or similar) in patch changelogs.  It's
better to use imperative sentences like "Convert them to dynamically
allocated arrays, ...".

>
> Signed-off-by: Xueqin Luo <luoxueqin@...inos.cn>
> ---
>  kernel/power/swap.c | 44 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 0beff7eeaaba..bd149a54c081 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -585,8 +585,8 @@ struct crc_data {
>         wait_queue_head_t go;                     /* start crc update */
>         wait_queue_head_t done;                   /* crc update done */
>         u32 *crc32;                               /* points to handle's crc32 */
> -       size_t *unc_len[CMP_THREADS];             /* uncompressed lengths */
> -       unsigned char *unc[CMP_THREADS];          /* uncompressed data */
> +       size_t **unc_len;                                     /* uncompressed lengths */
> +       unsigned char **unc;                              /* uncompressed data */
>  };
>
>  /*
> @@ -721,7 +721,21 @@ static int save_compressed_image(struct swap_map_handle *handle,
>
>         crc = kzalloc(sizeof(*crc), GFP_KERNEL);
>         if (!crc) {
> -               pr_err("Failed to allocate crc\n");
> +               pr_err("Failed to allocate crc structure\n");
> +               ret = -ENOMEM;
> +               goto out_clean;
> +       }
> +
> +       crc->unc_len = kcalloc(nr_threads, sizeof(size_t *), GFP_KERNEL);
> +       if (!crc->unc_len) {
> +               pr_err("Failed to allocate crc->unc_len for %d threads\n", nr_threads);
> +               ret = -ENOMEM;
> +               goto out_clean;
> +       }
> +
> +       crc->unc = kcalloc(nr_threads, sizeof(unsigned char *), GFP_KERNEL);
> +       if (!crc->unc) {
> +               pr_err("Failed to allocate crc->unc for %d threads\n", nr_threads);
>                 ret = -ENOMEM;
>                 goto out_clean;
>         }

Can you avoid code duplication by defining helpers for allocating and
freeing them both and using those helpers where applicable (image
creation and uncompression)?

> @@ -886,6 +900,10 @@ static int save_compressed_image(struct swap_map_handle *handle,
>  out_clean:
>         hib_finish_batch(&hb);
>         if (crc) {
> +               if (crc->unc)
> +                       kfree(crc->unc);
> +               if (crc->unc_len)
> +                       kfree(crc->unc_len);
>                 if (crc->thr)
>                         kthread_stop(crc->thr);
>                 kfree(crc);
> @@ -1241,7 +1259,21 @@ static int load_compressed_image(struct swap_map_handle *handle,
>
>         crc = kzalloc(sizeof(*crc), GFP_KERNEL);
>         if (!crc) {
> -               pr_err("Failed to allocate crc\n");
> +               pr_err("Failed to allocate crc structure\n");
> +               ret = -ENOMEM;
> +               goto out_clean;
> +       }
> +
> +       crc->unc_len = kcalloc(nr_threads, sizeof(size_t *), GFP_KERNEL);
> +       if (!crc->unc_len) {
> +               pr_err("Failed to allocate crc->unc_len for %d threads\n", nr_threads);
> +               ret = -ENOMEM;
> +               goto out_clean;
> +       }
> +
> +       crc->unc = kcalloc(nr_threads, sizeof(unsigned char *), GFP_KERNEL);
> +       if (!crc->unc) {
> +               pr_err("Failed to allocate crc->unc for %d threads\n", nr_threads);
>                 ret = -ENOMEM;
>                 goto out_clean;
>         }
> @@ -1507,6 +1539,10 @@ static int load_compressed_image(struct swap_map_handle *handle,
>         for (i = 0; i < ring_size; i++)
>                 free_page((unsigned long)page[i]);
>         if (crc) {
> +               if (crc->unc)
> +                       kfree(crc->unc);
> +               if (crc->unc_len)
> +                       kfree(crc->unc_len);
>                 if (crc->thr)
>                         kthread_stop(crc->thr);
>                 kfree(crc);
> --
> 2.43.0
>