Re: [PATCH 07/11] ntfs: reuse compression output workspace across write units

2026=EB=85=84 7=EC=9B=94 21=EC=9D=BC (=ED=99=94) =EC=98=A4=ED=9B=84 6:55, N=
amjae Jeon <[email protected]>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
>
> ntfs_write_cb() allocates output pages and creates input and output
> vmaps for every compression unit. Sequential writes repeatedly pay
> those allocation and page-table costs even though each unit has the
> same maximum output size.
> Allocate and map the output workspace once per write request. Access
>
> input sub-blocks with kmap_local_page(), and reuse the output pages and
> mapping for every compression unit in the request.
>
> Signed-off-by: Namjae Jeon <[email protected]>

Looks good to me.

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

> ---
>  fs/ntfs/compress.c | 135 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 85 insertions(+), 50 deletions(-)
>
> diff --git a/fs/ntfs/compress.c b/fs/ntfs/compress.c
> index f073a53f6136..f3c14518f78e 100644
> --- a/fs/ntfs/compress.c
> +++ b/fs/ntfs/compress.c
> @@ -894,6 +894,12 @@ struct compress_context {
>         s16 prev[NTFS_SB_SIZE];
>  };
>
> +struct ntfs_compress_workspace {
> +       struct page **pages;
> +       char *outbuf;
> +       unsigned int nr_pages;
> +};
> +
>  /*
>   * Hash the next 3-byte sequence in the input buffer
>   */
> @@ -1247,12 +1253,69 @@ static int ntfs_compress_block(struct compress_co=
ntext *pctx,
>         return xout;
>  }
>
> +static int ntfs_compress_workspace_init(struct ntfs_inode *ni,
> +                                       struct ntfs_compress_workspace *w=
s)
> +{
> +       unsigned int size, i;
> +
> +       size =3D ni->itype.compressed.block_size + 2 *
> +               (ni->itype.compressed.block_size / NTFS_SB_SIZE) + 2;
> +       ws->nr_pages =3D DIV_ROUND_UP(size, PAGE_SIZE);
> +       ws->pages =3D kcalloc(ws->nr_pages, sizeof(*ws->pages), GFP_NOFS)=
;
> +       if (!ws->pages)
> +               return -ENOMEM;
> +
> +       for (i =3D 0; i < ws->nr_pages; i++) {
> +               ws->pages[i] =3D alloc_page(GFP_NOFS);
> +               if (!ws->pages[i])
> +                       goto free_pages;
> +       }
> +
> +       ws->outbuf =3D vmap(ws->pages, ws->nr_pages, VM_MAP, PAGE_KERNEL)=
;
> +       if (!ws->outbuf)
> +               goto free_pages;
> +       return 0;
> +
> +free_pages:
> +       while (i)
> +               put_page(ws->pages[--i]);
> +       kfree(ws->pages);
> +       return -ENOMEM;
> +}
> +
> +static void ntfs_compress_workspace_free(struct ntfs_compress_workspace =
*ws)
> +{
> +       unsigned int i;
> +
> +       vunmap(ws->outbuf);
> +       for (i =3D 0; i < ws->nr_pages; i++)
> +               put_page(ws->pages[i]);
> +       kfree(ws->pages);
> +}
> +
> +static void ntfs_copy_cb(struct page **pages, int pages_per_cb,
> +                        unsigned int page_offset,
> +                        struct ntfs_compress_workspace *ws, unsigned int=
 bytes)
> +{
> +       unsigned int copied =3D 0, i;
> +
> +       for (i =3D 0; i < pages_per_cb && copied < bytes; i++) {
> +               unsigned int offset =3D i ? 0 : page_offset;
> +               unsigned int len =3D min(bytes - copied, PAGE_SIZE - offs=
et);
> +               void *addr =3D kmap_local_page(pages[i]);
> +
> +               memcpy(ws->outbuf + copied, addr + offset, len);
> +               kunmap_local(addr);
> +               copied +=3D len;
> +       }
> +}
> +
>  static int ntfs_write_cb(struct ntfs_inode *ni, loff_t pos, struct page =
**pages,
>                 int pages_per_cb, unsigned int page_offset,
> -               struct compress_context *ctx)
> +               struct compress_context *ctx, struct ntfs_compress_worksp=
ace *ws)
>  {
>         struct ntfs_volume *vol =3D ni->vol;
> -       char *outbuf =3D NULL, *pbuf, *inbuf, *in_mapping;
> +       char *outbuf =3D ws->outbuf, *pbuf;
>         u32 compsz, p, insz =3D ni->itype.compressed.block_size;
>         s32 rounded, bio_size;
>         int sz;
> @@ -1264,55 +1327,32 @@ static int ntfs_write_cb(struct ntfs_inode *ni, l=
off_t pos, struct page **pages,
>         static char twozeroes[] =3D {0x02, 0xb0, 0x00, 0x00, 0x00};
>         /* more compressed zeroes, to be followed by some count */
>         static char morezeroes[] =3D {0x03, 0xb0, 0x02, 0x00};
> -       struct page **pages_disk =3D NULL, *pg;
>         s64 bio_lcn;
>         struct runlist_element *rlc, *rl;
>         int i, err;
> -       int pages_count =3D (round_up(ni->itype.compressed.block_size + 2=
 *
> -               (ni->itype.compressed.block_size / NTFS_SB_SIZE) + 2, PAG=
E_SIZE)) / PAGE_SIZE;
>         u32 cb_clusters =3D ni->itype.compressed.block_clusters;
>         size_t new_rl_count;
>         struct bio *bio =3D NULL;
>         loff_t cb_pos, new_length;
>         s64 new_vcn;
>
> -       in_mapping =3D vmap(pages, pages_per_cb, VM_MAP, PAGE_KERNEL_RO);
> -       if (!in_mapping)
> -               return -ENOMEM;
> -       inbuf =3D in_mapping + page_offset;
> -
> -       /* may need 2 extra bytes per block and 2 more bytes */
> -       pages_disk =3D kcalloc(pages_count, sizeof(struct page *), GFP_NO=
FS);
> -       if (!pages_disk) {
> -               vunmap(in_mapping);
> -               return -ENOMEM;
> -       }
> -
> -       for (i =3D 0; i < pages_count; i++) {
> -               pg =3D alloc_page(GFP_KERNEL);
> -               if (!pg) {
> -                       err =3D -ENOMEM;
> -                       goto out;
> -               }
> -               pages_disk[i] =3D pg;
> -               lock_page(pg);
> -       }
> -
> -       outbuf =3D vmap(pages_disk, pages_count, VM_MAP, PAGE_KERNEL);
> -       if (!outbuf) {
> -               err =3D -ENOMEM;
> -               goto out;
> -       }
> -
>         compsz =3D 0;
>         allzeroes =3D true;
>         for (p =3D 0; (p < insz) && !fail; p +=3D NTFS_SB_SIZE) {
> +               unsigned int input_offset =3D page_offset + p;
> +               unsigned int page_idx =3D input_offset >> PAGE_SHIFT;
> +               const char *input;
> +               void *addr;
> +
>                 if ((p + NTFS_SB_SIZE) < insz)
>                         bsz =3D NTFS_SB_SIZE;
>                 else
>                         bsz =3D insz - p;
>                 pbuf =3D &outbuf[compsz];
> -               sz =3D ntfs_compress_block(ctx, &inbuf[p], bsz, pbuf);
> +               addr =3D kmap_local_page(pages[page_idx]);
> +               input =3D addr + offset_in_page(input_offset);
> +               sz =3D ntfs_compress_block(ctx, input, bsz, pbuf);
> +               kunmap_local(addr);
>                 if (sz < 0) {
>                         err =3D sz;
>                         goto out;
> @@ -1352,14 +1392,12 @@ static int ntfs_write_cb(struct ntfs_inode *ni, l=
off_t pos, struct page **pages,
>                 rounded =3D ((compsz - 1) | (vol->cluster_size - 1)) + 1;
>                 memset(&outbuf[compsz], 0, rounded - compsz);
>                 bio_size =3D rounded;
> -               pages =3D pages_disk;
>         } else if (allzeroes) {
>                 err =3D ntfs_non_resident_attr_punch_hole(ni, new_vcn, cb=
_clusters);
>                 goto out;
>         } else {
> -               memcpy(outbuf, inbuf, insz);
> +               ntfs_copy_cb(pages, pages_per_cb, page_offset, ws, insz);
>                 bio_size =3D insz;
> -               pages =3D pages_disk;
>         }
>
>         new_length =3D ntfs_bytes_to_cluster(vol, round_up(bio_size, vol-=
>cluster_size));
> @@ -1398,7 +1436,7 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof=
f_t pos, struct page **pages,
>                                                 ((s64)i << PAGE_SHIFT));
>                 }
>
> -               if (!bio_add_page(bio, pages[i], page_size, 0)) {
> +               if (!bio_add_page(bio, ws->pages[i], page_size, 0)) {
>                         err =3D submit_bio_wait(bio);
>                         bio_put(bio);
>                         if (err)
> @@ -1443,17 +1481,6 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lo=
ff_t pos, struct page **pages,
>                 ntfs_error(vol->sb, "Failed to free hot clusters.");
>         kvfree(rlc);
>  out:
> -       if (outbuf)
> -               vunmap(outbuf);
> -       for (i =3D 0; i < pages_count; i++) {
> -               pg =3D pages_disk[i];
> -               if (pg) {
> -                       unlock_page(pg);
> -                       put_page(pg);
> -               }
> -       }
> -       kfree(pages_disk);
> -       vunmap(in_mapping);
>         NInoSetFileNameDirty(ni);
>         mark_mft_record_dirty(ni);
>
> @@ -1463,6 +1490,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 ntfs_compress_workspace ws =3D {};
>         struct compress_context *ctx;
>         struct folio *folio;
>         struct page **pages =3D NULL, *page;
> @@ -1483,6 +1511,12 @@ int ntfs_compress_write(struct ntfs_inode *ni, lof=
f_t pos, size_t count,
>                 kfree(pages);
>                 return -ENOMEM;
>         }
> +       err =3D ntfs_compress_workspace_init(ni, &ws);
> +       if (err) {
> +               kvfree(ctx);
> +               kfree(pages);
> +               return err;
> +       }
>
>         while (count) {
>                 pgoff_t index;
> @@ -1551,7 +1585,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, ctx);
> +               err =3D ntfs_write_cb(ni, pos, pages, pages_per_cb, page_=
offset, ctx, &ws);
>                 if (!err && pos + copied > ni->initialized_size) {
>                         mutex_lock(&ni->mrec_lock);
>                         err =3D ntfs_attr_set_initialized_size(ni, pos + =
copied);
> @@ -1581,6 +1615,7 @@ int ntfs_compress_write(struct ntfs_inode *ni, loff=
_t pos, size_t count,
>         }
>
>  out:
> +       ntfs_compress_workspace_free(&ws);
>         kvfree(ctx);
>         kfree(pages);
>         if (err < 0)
> --
> 2.34.1
>


--=20
Thanks,
Hyunchul