Re: [PATCH 06/11] ntfs: reuse the compression context during writes

2026=EB=85=84 7=EC=9B=94 21=EC=9D=BC (=ED=99=94) =EC=98=A4=ED=9B=84 6:54, N=
amjae Jeon <[email protected]>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
>
> ntfs_compress_block() allocates and initializes a roughly 40 KiB match
> finder context for every 4 KiB sub-block. A 64 KiB compression unit thus
> performs sixteen large allocations even though the calls are serialized.
>
> Allocate one context for the complete write request and reset its hash
> chains for each sub-block as before.
>
> Signed-off-by: Namjae Jeon <[email protected]>

Looks good to me.

Reviewed-by: Hyunchul Lee <[email protected]>

> ---
>  fs/ntfs/compress.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/fs/ntfs/compress.c b/fs/ntfs/compress.c
> index a993cba3b961..f073a53f6136 100644
> --- a/fs/ntfs/compress.c
> +++ b/fs/ntfs/compress.c
> @@ -1072,10 +1072,9 @@ static void ntfs_skip_position(struct compress_con=
text *pctx, const int i)
>   * header (minimal size is 2, maximum size is 4098)
>   * A negative error code if an error has been met.
>   */
> -static int ntfs_compress_block(const char *inbuf, const int bufsize,
> -                              char *outbuf)
> +static int ntfs_compress_block(struct compress_context *pctx,
> +                              const char *inbuf, const int bufsize, char=
 *outbuf)
>  {
> -       struct compress_context *pctx;
>         int i; /* current position */
>         int j; /* end of best match from current position */
>         int k; /* end of best match from next position */
> @@ -1090,10 +1089,6 @@ static int ntfs_compress_block(const char *inbuf, =
const int bufsize,
>         int tag;    /* current value of tag */
>         int ntag;   /* count of bits still undefined in tag */
>
> -       pctx =3D kvzalloc(sizeof(struct compress_context), GFP_NOFS);
> -       if (!pctx)
> -               return -ENOMEM;
> -
>         /*
>          * All hash chains start as empty.  The special value '-1' indica=
tes the
>          * end of each hash chain.
> @@ -1249,16 +1244,12 @@ static int ntfs_compress_block(const char *inbuf,=
 const int bufsize,
>                 xout =3D NTFS_SB_SIZE + 2;
>         }
>
> -       /*
> -        * Free the compression context and return the total number of by=
tes
> -        * written to 'outbuf'.
> -        */
> -       kvfree(pctx);
>         return xout;
>  }
>
>  static int ntfs_write_cb(struct ntfs_inode *ni, loff_t pos, struct page =
**pages,
> -               int pages_per_cb, unsigned int page_offset)
> +               int pages_per_cb, unsigned int page_offset,
> +               struct compress_context *ctx)
>  {
>         struct ntfs_volume *vol =3D ni->vol;
>         char *outbuf =3D NULL, *pbuf, *inbuf, *in_mapping;
> @@ -1321,7 +1312,7 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof=
f_t pos, struct page **pages,
>                 else
>                         bsz =3D insz - p;
>                 pbuf =3D &outbuf[compsz];
> -               sz =3D ntfs_compress_block(&inbuf[p], bsz, pbuf);
> +               sz =3D ntfs_compress_block(ctx, &inbuf[p], bsz, pbuf);
>                 if (sz < 0) {
>                         err =3D sz;
>                         goto out;
> @@ -1472,6 +1463,7 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof=
f_t pos, struct page **pages,
>  int ntfs_compress_write(struct ntfs_inode *ni, loff_t pos, size_t count,
>                 struct iov_iter *from)
>  {
> +       struct compress_context *ctx;
>         struct folio *folio;
>         struct page **pages =3D NULL, *page;
>         int pages_per_cb;
> @@ -1486,6 +1478,11 @@ int ntfs_compress_write(struct ntfs_inode *ni, lof=
f_t pos, size_t count,
>         pages =3D kmalloc_array(pages_per_cb, sizeof(struct page *), GFP_=
NOFS);
>         if (!pages)
>                 return -ENOMEM;
> +       ctx =3D kvzalloc_obj(*ctx, GFP_NOFS);
> +       if (!ctx) {
> +               kfree(pages);
> +               return -ENOMEM;
> +       }
>
>         while (count) {
>                 pgoff_t index;
> @@ -1554,7 +1551,7 @@ int ntfs_compress_write(struct ntfs_inode *ni, loff=
_t pos, size_t count,
>                         goto release_pages;
>                 }
>
> -               err =3D ntfs_write_cb(ni, pos, pages, pages_per_cb, page_=
offset);
> +               err =3D ntfs_write_cb(ni, pos, pages, pages_per_cb, page_=
offset, ctx);
>                 if (!err && pos + copied > ni->initialized_size) {
>                         mutex_lock(&ni->mrec_lock);
>                         err =3D ntfs_attr_set_initialized_size(ni, pos + =
copied);
> @@ -1584,6 +1581,7 @@ int ntfs_compress_write(struct ntfs_inode *ni, loff=
_t pos, size_t count,
>         }
>
>  out:
> +       kvfree(ctx);
>         kfree(pages);
>         if (err < 0)
>                 written =3D err;
> --
> 2.34.1
>


--=20
Thanks,
Hyunchul