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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Fri, 25 Dec 2020 14:29:14 +0000
From:   Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
To:     Eric Biggers <ebiggers@...nel.org>
CC:     "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "pali@...nel.org" <pali@...nel.org>,
        "dsterba@...e.cz" <dsterba@...e.cz>,
        "aaptel@...e.com" <aaptel@...e.com>,
        "willy@...radead.org" <willy@...radead.org>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "joe@...ches.com" <joe@...ches.com>,
        "mark@...mstone.com" <mark@...mstone.com>,
        "nborisov@...e.com" <nborisov@...e.com>,
        "linux-ntfs-dev@...ts.sourceforge.net" 
        <linux-ntfs-dev@...ts.sourceforge.net>,
        "anton@...era.com" <anton@...era.com>,
        "dan.carpenter@...cle.com" <dan.carpenter@...cle.com>,
        "hch@....de" <hch@....de>
Subject: RE: [PATCH v14 06/10] fs/ntfs3: Add compression

From: Konstantin Komarov
> Sent: Friday, December 11, 2020 7:28 PM
> To: 'Eric Biggers' <ebiggers@...nel.org>
> Cc: linux-fsdevel@...r.kernel.org; viro@...iv.linux.org.uk; linux-kernel@...r.kernel.org; pali@...nel.org; dsterba@...e.cz;
> aaptel@...e.com; willy@...radead.org; rdunlap@...radead.org; joe@...ches.com; mark@...mstone.com; nborisov@...e.com;
> linux-ntfs-dev@...ts.sourceforge.net; anton@...era.com; dan.carpenter@...cle.com; hch@....de
> Subject: RE: [PATCH v14 06/10] fs/ntfs3: Add compression
> 
> From: Eric Biggers <ebiggers@...nel.org>
> Sent: Friday, December 4, 2020 9:39 PM
> > To: Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
> > Cc: linux-fsdevel@...r.kernel.org; viro@...iv.linux.org.uk; linux-kernel@...r.kernel.org; pali@...nel.org; dsterba@...e.cz;
> > aaptel@...e.com; willy@...radead.org; rdunlap@...radead.org; joe@...ches.com; mark@...mstone.com; nborisov@...e.com;
> > linux-ntfs-dev@...ts.sourceforge.net; anton@...era.com; dan.carpenter@...cle.com; hch@....de
> > 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@...agon-software.com>
> > > ---
> > >  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.