RE: [PATCH v14 06/10] fs/ntfs3: Add compression

[Konstantin Komarov]

From: Konstantin Komarov
> Sent: Friday, December 11, 2020 7:28 PM
> To: 'Eric Biggers' <ebiggers@xxxxxxxxxx>
> Cc: linux-fsdevel@xxxxxxxxxxxxxxx; viro@xxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; pali@xxxxxxxxxx; dsterba@xxxxxxx;
> aaptel@xxxxxxxx; willy@xxxxxxxxxxxxx; rdunlap@xxxxxxxxxxxxx; joe@xxxxxxxxxxx; mark@xxxxxxxxxxxxx; nborisov@xxxxxxxx;
> linux-ntfs-dev@xxxxxxxxxxxxxxxxxxxxx; anton@xxxxxxxxxx; dan.carpenter@xxxxxxxxxx; hch@xxxxxx
> Subject: RE: [PATCH v14 06/10] fs/ntfs3: Add compression
> 
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
> Sent: Friday, December 4, 2020 9:39 PM
> > To: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
> > Cc: linux-fsdevel@xxxxxxxxxxxxxxx; viro@xxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; pali@xxxxxxxxxx; dsterba@xxxxxxx;
> > aaptel@xxxxxxxx; willy@xxxxxxxxxxxxx; rdunlap@xxxxxxxxxxxxx; joe@xxxxxxxxxxx; mark@xxxxxxxxxxxxx; nborisov@xxxxxxxx;
> > linux-ntfs-dev@xxxxxxxxxxxxxxxxxxxxx; anton@xxxxxxxxxx; dan.carpenter@xxxxxxxxxx; hch@xxxxxx
> > Subject: Re: [PATCH v14 06/10] fs/ntfs3: Add compression
> >
> > On Fri, Dec 04, 2020 at 06:45:56PM +0300, Konstantin Komarov wrote:
> > > This adds compression
> > >
> > > Signed-off-by: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
> > > ---
> > >  fs/ntfs3/lib/common_defs.h       | 196 +++++++++++
> > >  fs/ntfs3/lib/decompress_common.c | 314 +++++++++++++++++
> > >  fs/ntfs3/lib/decompress_common.h | 558 +++++++++++++++++++++++++++++++
> > >  fs/ntfs3/lib/lzx_common.c        | 204 +++++++++++
> > >  fs/ntfs3/lib/lzx_common.h        |  31 ++
> > >  fs/ntfs3/lib/lzx_constants.h     | 113 +++++++
> > >  fs/ntfs3/lib/lzx_decompress.c    | 553 ++++++++++++++++++++++++++++++
> > >  fs/ntfs3/lib/xpress_constants.h  |  23 ++
> > >  fs/ntfs3/lib/xpress_decompress.c | 165 +++++++++
> > >  fs/ntfs3/lznt.c                  | 452 +++++++++++++++++++++++++
> > >  10 files changed, 2609 insertions(+)
> > >  create mode 100644 fs/ntfs3/lib/common_defs.h
> > >  create mode 100644 fs/ntfs3/lib/decompress_common.c
> > >  create mode 100644 fs/ntfs3/lib/decompress_common.h
> > >  create mode 100644 fs/ntfs3/lib/lzx_common.c
> > >  create mode 100644 fs/ntfs3/lib/lzx_common.h
> > >  create mode 100644 fs/ntfs3/lib/lzx_constants.h
> > >  create mode 100644 fs/ntfs3/lib/lzx_decompress.c
> > >  create mode 100644 fs/ntfs3/lib/xpress_constants.h
> > >  create mode 100644 fs/ntfs3/lib/xpress_decompress.c
> > >  create mode 100644 fs/ntfs3/lznt.c
> >
> > This really could use a much better commit message.  Including mentioning where
> > the LZX and XPRESS decompression code came from
> > (https://github.com/ebiggers/ntfs-3g-system-compression).
> >
> 
> Hi Eric! Fixed in V15!
> 
> > Also note you've marked the files as "SPDX-License-Identifier: GPL-2.0",
> > but they really are "SPDX-License-Identifier: GPL-2.0-or-later".
> >
> 
> Thanks, fixed as well.
> 
> > Also I still think you should consider using the simpler version from
> > ntfs-3g-system-compression commit 3ddd227ee8e3, which I had originally intended
> > to be included in NTFS-3G itself.  That version was fewer lines of code and
> > fewer files, as it was simplified for decompression-only.  The latest version
> > (the one you're using) is shared with a project that also implements compression
> > (so that I can more easily maintain both projects), so it's more complex than
> > needed for decompression-only support.  But in the kernel context it may make
> > sense to go with a simpler version.  There are a few performance optimizations
> > you'd miss by going with the older version, but they weren't too significant,
> > and probably you don't need to squeeze out every bit of performance possible
> > when reading XPRESS and LZX-compressed files in this context?
> >
> > - Eric
> 
> We used the newest code initially. Looking at the commit you've pointed out, but it will
> take some time as needs to pass full set of tests with new code. On the differences you've
> mentioned between the first and latest code in ntfs-3g system compression: we've removed
> the optimizations (needed to go into kernel), also the reparse points stuff is being parsed by
> ntfs3 driver itself. Given this, doesn't seems there are many differences remain.
> Also, we'll follow your recommendations, if you highly recommend to stick to the commit
> you've mentioned (but at this moment it seems "newer=better" formula still valid).
> 
> Thanks!

Hi Eric! The code in V16 is now based on the initial ntfs-3g plugin code, just as you suggested.

Best regards.