Received-SPF: pass (google.com: domain of linux-kernel+bounces-184061-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240520090106.2898681-1-hsiangkao@linux.alibaba.com>
In-Reply-To: <20240520090106.2898681-1-hsiangkao@linux.alibaba.com>
From: Sandeep Dhavale <dhavale@google.com>
Date: Mon, 20 May 2024 11:24:44 -0700
Message-ID: <CAB=BE-Qr5mukXPPqqduH2Rr3cBrkP_WUiJ1udhtfmmrHFmObcQ@mail.gmail.com>
Subject: Re: [PATCH] erofs: avoid allocating DEFLATE streams before mounting
To: Gao Xiang <hsiangkao@linux.alibaba.com>
Cc: linux-erofs@lists.ozlabs.org, LKML <linux-kernel@vger.kernel.org>, 
	stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, May 20, 2024 at 2:06=E2=80=AFAM Gao Xiang <hsiangkao@linux.alibaba.=
com> wrote:
>
> Currently, each DEFLATE stream takes one 32 KiB permanent internal
> window buffer even if there is no running instance which uses DEFLATE
> algorithm.
>
> It's unexpected and wasteful on embedded devices with limited resources
> and servers with hundreds of CPU cores if DEFLATE is enabled but unused.
>
> Fixes: ffa09b3bd024 ("erofs: DEFLATE compression support")
> Cc: <stable@vger.kernel.org> # 6.6+
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

LGTM.

Reviewed-by: Sandeep Dhavale <dhavale@google.com>

Thanks,
Sandeep.

> ---
>  fs/erofs/decompressor_deflate.c | 55 +++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 26 deletions(-)
>
> diff --git a/fs/erofs/decompressor_deflate.c b/fs/erofs/decompressor_defl=
ate.c
> index 81e65c453ef0..3a3461561a3c 100644
> --- a/fs/erofs/decompressor_deflate.c
> +++ b/fs/erofs/decompressor_deflate.c
> @@ -46,39 +46,15 @@ int __init z_erofs_deflate_init(void)
>         /* by default, use # of possible CPUs instead */
>         if (!z_erofs_deflate_nstrms)
>                 z_erofs_deflate_nstrms =3D num_possible_cpus();
> -
> -       for (; z_erofs_deflate_avail_strms < z_erofs_deflate_nstrms;
> -            ++z_erofs_deflate_avail_strms) {
> -               struct z_erofs_deflate *strm;
> -
> -               strm =3D kzalloc(sizeof(*strm), GFP_KERNEL);
> -               if (!strm)
> -                       goto out_failed;
> -
> -               /* XXX: in-kernel zlib cannot shrink windowbits currently=
 */
> -               strm->z.workspace =3D vmalloc(zlib_inflate_workspacesize(=
));
> -               if (!strm->z.workspace) {
> -                       kfree(strm);
> -                       goto out_failed;
> -               }
> -
> -               spin_lock(&z_erofs_deflate_lock);
> -               strm->next =3D z_erofs_deflate_head;
> -               z_erofs_deflate_head =3D strm;
> -               spin_unlock(&z_erofs_deflate_lock);
> -       }
>         return 0;
> -
> -out_failed:
> -       erofs_err(NULL, "failed to allocate zlib workspace");
> -       z_erofs_deflate_exit();
> -       return -ENOMEM;
>  }
>
>  int z_erofs_load_deflate_config(struct super_block *sb,
>                         struct erofs_super_block *dsb, void *data, int si=
ze)
>  {
>         struct z_erofs_deflate_cfgs *dfl =3D data;
> +       static DEFINE_MUTEX(deflate_resize_mutex);
> +       static bool inited;
>
>         if (!dfl || size < sizeof(struct z_erofs_deflate_cfgs)) {
>                 erofs_err(sb, "invalid deflate cfgs, size=3D%u", size);
> @@ -89,9 +65,36 @@ int z_erofs_load_deflate_config(struct super_block *sb=
,
>                 erofs_err(sb, "unsupported windowbits %u", dfl->windowbit=
s);
>                 return -EOPNOTSUPP;
>         }
> +       mutex_lock(&deflate_resize_mutex);
> +       if (!inited) {
> +               for (; z_erofs_deflate_avail_strms < z_erofs_deflate_nstr=
ms;
> +                    ++z_erofs_deflate_avail_strms) {
> +                       struct z_erofs_deflate *strm;
> +
> +                       strm =3D kzalloc(sizeof(*strm), GFP_KERNEL);
> +                       if (!strm)
> +                               goto failed;
> +                       /* XXX: in-kernel zlib cannot customize windowbit=
s */
> +                       strm->z.workspace =3D vmalloc(zlib_inflate_worksp=
acesize());
> +                       if (!strm->z.workspace) {
> +                               kfree(strm);
> +                               goto failed;
> +                       }
>
> +                       spin_lock(&z_erofs_deflate_lock);
> +                       strm->next =3D z_erofs_deflate_head;
> +                       z_erofs_deflate_head =3D strm;
> +                       spin_unlock(&z_erofs_deflate_lock);
> +               }
> +               inited =3D true;
> +       }
> +       mutex_unlock(&deflate_resize_mutex);
>         erofs_info(sb, "EXPERIMENTAL DEFLATE feature in use. Use at your =
own risk!");
>         return 0;
> +failed:
> +       mutex_unlock(&deflate_resize_mutex);
> +       z_erofs_deflate_exit();
> +       return -ENOMEM;
>  }
>
>  int z_erofs_deflate_decompress(struct z_erofs_decompress_req *rq,
> --
> 2.39.3
>