Re: [PATCH 02/11] ntfs: support large pages in compressed 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_write() derives its page count by shifting the compression
> block size and assumes that every compression block begins at a page
> boundary. This produces a zero page count for small compression blocks on
> large-page systems and ignores an in-page compression block offset.
>
> Map every page covering the compression block, pass the in-page offset to
> ntfs_write_cb(), and stage uncompressed output in page-aligned pages.
>
> Signed-off-by: Namjae Jeon <[email protected]>

Looks good to me.

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

> ---
>  fs/ntfs/compress.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/fs/ntfs/compress.c b/fs/ntfs/compress.c
> index 6b78a8efe3ac..006b8831836c 100644
> --- a/fs/ntfs/compress.c
> +++ b/fs/ntfs/compress.c
> @@ -1258,11 +1258,11 @@ static int ntfs_compress_block(const char *inbuf,=
 const int bufsize,
>  }
>
>  static int ntfs_write_cb(struct ntfs_inode *ni, loff_t pos, struct page =
**pages,
> -               int pages_per_cb)
> +               int pages_per_cb, unsigned int page_offset)
>  {
>         struct ntfs_volume *vol =3D ni->vol;
> -       char *outbuf =3D NULL, *pbuf, *inbuf;
> -       u32 compsz, p, insz =3D pages_per_cb << PAGE_SHIFT;
> +       char *outbuf =3D NULL, *pbuf, *inbuf, *in_mapping;
> +       u32 compsz, p, insz =3D ni->itype.compressed.block_size;
>         s32 rounded, bio_size;
>         int sz;
>         unsigned int bsz;
> @@ -1284,14 +1284,15 @@ static int ntfs_write_cb(struct ntfs_inode *ni, l=
off_t pos, struct page **pages,
>         loff_t new_length;
>         s64 new_vcn;
>
> -       inbuf =3D vmap(pages, pages_per_cb, VM_MAP, PAGE_KERNEL_RO);
> -       if (!inbuf)
> +       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(inbuf);
> +               vunmap(in_mapping);
>                 return -ENOMEM;
>         }
>
> @@ -1361,7 +1362,9 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof=
f_t pos, struct page **pages,
>                 err =3D 0;
>                 goto out;
>         } else {
> +               memcpy(outbuf, inbuf, insz);
>                 bio_size =3D insz;
> +               pages =3D pages_disk;
>         }
>
>         new_vcn =3D ntfs_bytes_to_cluster(vol,
> @@ -1420,7 +1423,8 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof=
f_t pos, struct page **pages,
>                                         GFP_NOIO);
>                         bio->bi_iter.bi_sector =3D
>                                 ntfs_bytes_to_sector(vol,
> -                                               ntfs_cluster_to_bytes(vol=
, bio_lcn + i));
> +                                               ntfs_cluster_to_bytes(vol=
, bio_lcn) +
> +                                               ((s64)i << PAGE_SHIFT));
>                 }
>
>                 if (!bio_add_page(bio, pages[i], page_size, 0)) {
> @@ -1437,7 +1441,8 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof=
f_t pos, struct page **pages,
>         err =3D submit_bio_wait(bio);
>         bio_put(bio);
>  out:
> -       vunmap(outbuf);
> +       if (outbuf)
> +               vunmap(outbuf);
>         for (i =3D 0; i < pages_count; i++) {
>                 pg =3D pages_disk[i];
>                 if (pg) {
> @@ -1446,7 +1451,7 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof=
f_t pos, struct page **pages,
>                 }
>         }
>         kfree(pages_disk);
> -       vunmap(inbuf);
> +       vunmap(in_mapping);
>         NInoSetFileNameDirty(ni);
>         mark_mft_record_dirty(ni);
>
> @@ -1458,12 +1463,15 @@ int ntfs_compress_write(struct ntfs_inode *ni, lo=
ff_t pos, size_t count,
>  {
>         struct folio *folio;
>         struct page **pages =3D NULL, *page;
> -       int pages_per_cb =3D ni->itype.compressed.block_size >> PAGE_SHIF=
T;
> +       int pages_per_cb;
>         int cb_size =3D ni->itype.compressed.block_size, cb_off, err =3D =
0;
>         int i, ip;
>         size_t written =3D 0;
>         struct address_space *mapping =3D VFS_I(ni)->i_mapping;
>
> +       pages_per_cb =3D DIV_ROUND_UP(offset_in_page(pos & ~(cb_size - 1)=
) +
> +                       cb_size, PAGE_SIZE);
> +
>         pages =3D kmalloc_array(pages_per_cb, sizeof(struct page *), GFP_=
NOFS);
>         if (!pages)
>                 return -ENOMEM;
> @@ -1471,6 +1479,7 @@ int ntfs_compress_write(struct ntfs_inode *ni, loff=
_t pos, size_t count,
>         while (count) {
>                 pgoff_t index;
>                 size_t copied, bytes;
> +               unsigned int page_offset;
>                 int off;
>
>                 off =3D pos & (cb_size - 1);
> @@ -1479,6 +1488,8 @@ int ntfs_compress_write(struct ntfs_inode *ni, loff=
_t pos, size_t count,
>                         bytes =3D count;
>
>                 cb_off =3D pos & ~(cb_size - 1);
> +               page_offset =3D offset_in_page(cb_off);
> +               pages_per_cb =3D DIV_ROUND_UP(page_offset + cb_size, PAGE=
_SIZE);
>                 index =3D cb_off >> PAGE_SHIFT;
>
>                 if (unlikely(fault_in_iov_iter_readable(from, bytes))) {
> @@ -1527,7 +1538,7 @@ int ntfs_compress_write(struct ntfs_inode *ni, loff=
_t pos, size_t count,
>                         }
>                 }
>
> -               err =3D ntfs_write_cb(ni, pos, pages, pages_per_cb);
> +               err =3D ntfs_write_cb(ni, pos, pages, pages_per_cb, page_=
offset);
>
>                 for (i =3D 0; i < pages_per_cb; i++) {
>                         folio =3D page_folio(pages[i]);
> --
> 2.34.1
>


--=20
Thanks,
Hyunchul