| Message ID | 20181009232137.1941290-1-terrelln@fb.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: Add zstd support to grub btrfs | expand |
Dear Nick, Thank you very much for your patches. Am 10.10.2018 um 01:21 schrieb Nick Terrell: > This patch set imports the upstream zstd library, adds zstd support to the > btrfs module, and adds a test case. I've also tested the patch set by storing > my boot partition in btrfs with and without zstd compression and rebooting. > > The fist patch imports the files needed to support zstd decompression from > zstd-1.3.6 as-is. It is a very large patch. In case it doesn't make it, > I've included the commit hash and the script I used to download the files. > > Upstream zstd commit hash: 4fa456d7f12f8b27bd3b2f5dfd4f46898cb31c24 > Upstream zstd commit name: Merge pull request #1354 from facebook/dev > > --- > #!/bin/sh -e > > curl -L -O https://github.com/facebook/zstd/releases/download/v1.3.6/zstd-1.3.6.tar.gz > curl -L -O https://github.com/facebook/zstd/releases/download/v1.3.6/zstd-1.3.6.tar.gz.sha256 > sha256sum --check zstd-1.3.6.tar.gz.sha256 > tar xzf zstd-1.3.6.tar.gz > > SRC_LIB="zstd-1.3.6/lib" > DST_LIB="grub-core/lib/zstd" > rm -rf $DST_LIB > mkdir -p $DST_LIB > cp $SRC_LIB/zstd.h $DST_LIB/ > cp $SRC_LIB/common/*.[hc] $DST_LIB/ > cp $SRC_LIB/decompress/*.[hc] $DST_LIB/ > rm $DST_LIB/{pool.[hc],threading.[hc]} > rm -rf zstd-1.3.6* > echo SUCCESS! > --- Sorry for being ignorant, but you explain, why the library needs to be imported and it is not enough to use that library as an external dependency? Importing the library means, it has to be maintained in the GRUB repository, which will result in some maintenance burden. Kind regards, Paul
> On Oct 10, 2018, at 12:34 AM, Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > Sorry for being ignorant, but you explain, why the library needs to be imported and it is not enough to use that library as an external dependency? > > Importing the library means, it has to be maintained in the GRUB repository, which will result in some maintenance burden. I've imported zstd because thats the way the rest of the decompressors are imported. We could potentially use libzstd as an external dependency, since its only dependency is libc and GRUB provides the definitions of the libc functions that zstd needs to decompress (memcpy, and memmove). Theres some other stuff in the library that requires libc functionality that GRUB doesn't provide, but that isn't used during decompression. We strip those files out in the import. Let me know if you want me to switch to an external dependency. Best, Nick
Dear Nick, Am 10.10.2018 um 22:28 schrieb Nick Terrell: >> On Oct 10, 2018, at 12:34 AM, Paul Menzel wrote: >> >> Sorry for being ignorant, but you explain, why the library needs to >> be imported and it is not enough to use that library as an external >> dependency? >> >> Importing the library means, it has to be maintained in the GRUB >> repository, which will result in some maintenance burden. > > I've imported zstd because thats the way the rest of the > decompressors are imported. > > We could potentially use libzstd as an external dependency, since its > only dependency is libc and GRUB provides the definitions of the libc > functions that zstd needs to decompress (memcpy, and memmove). Theres > some other stuff in the library that requires libc functionality that > GRUB doesn't provide, but that isn't used during decompression. We > strip those files out in the import. Thank you for the explanation. > Let me know if you want me to switch to an external dependency. I cannot make that decision. The main developers and the distribution folks should comment on that. Kind regards, Paul
On Wed, Oct 10, 2018 at 08:28:27PM +0000, Nick Terrell wrote: > > On Oct 10, 2018, at 12:34 AM, Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > > > Sorry for being ignorant, but you explain, why the library needs to be imported and it is not enough to use that library as an external dependency? > > > > Importing the library means, it has to be maintained in the GRUB repository, which will result in some maintenance burden. > > I've imported zstd because thats the way the rest of the decompressors are imported. > > We could potentially use libzstd as an external dependency, since its only dependency is libc > and GRUB provides the definitions of the libc functions that zstd needs to decompress > (memcpy, and memmove). Theres some other stuff in the library that requires libc functionality > that GRUB doesn't provide, but that isn't used during decompression. We strip those files > out in the import. > > Let me know if you want me to switch to an external dependency. I do not think it is possible. Or it can be at least difficult. Even if it is possible it would require a major rework of GRUB build machine. So, even if current solution is not perfect I would like to stick to it. And zstd library integration is not very difficult. So, I do not think it will add a lot of burden in the future. Daniel
On Tue, Oct 09, 2018 at 04:21:36PM -0700, Nick Terrell wrote: > Import zstd-1.3.6 from upstream [1]. Only the files need for decompression > are imported. > > I used the latest zstd release, which includes patches [2] to build cleanly > in GRUB. > > Upstream zstd commit hash: 4fa456d7f12f8b27bd3b2f5dfd4f46898cb31c24 > Upstream zstd commit name: Merge pull request #1354 from facebook/dev > > I've included the script used to import zstd-1.3.6 below. > > [1] https://github.com/facebook/zstd/releases/tag/v1.3.6 > [2] https://github.com/facebook/zstd/pull/1344 > > --- I think that you should replace the dashes with something different here. Otherwise "git am" may tear off the rest of commit message starting from here. > #!/bin/sh -e > > curl -L -O https://github.com/facebook/zstd/releases/download/v1.3.6/zstd-1.3.6.tar.gz > curl -L -O https://github.com/facebook/zstd/releases/download/v1.3.6/zstd-1.3.6.tar.gz.sha256 > sha256sum --check zstd-1.3.6.tar.gz.sha256 > tar xzf zstd-1.3.6.tar.gz > > SRC_LIB="zstd-1.3.6/lib" > DST_LIB="grub-core/lib/zstd" > rm -rf $DST_LIB > mkdir -p $DST_LIB > cp $SRC_LIB/zstd.h $DST_LIB/ > cp $SRC_LIB/common/*.[hc] $DST_LIB/ > cp $SRC_LIB/decompress/*.[hc] $DST_LIB/ > rm $DST_LIB/{pool.[hc],threading.[hc]} > rm -rf zstd-1.3.6* > echo SUCCESS! > --- Ditto. > Signed-off-by: Nick Terrell <terrelln@fb.com> If you fix things above you can add Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Daniel
Hi Nick, CC-ing Goffredo. On Tue, Oct 09, 2018 at 04:21:35PM -0700, Nick Terrell wrote: > Hi all, > > This patch set imports the upstream zstd library, adds zstd support to the > btrfs module, and adds a test case. I've also tested the patch set by storing > my boot partition in btrfs with and without zstd compression and rebooting. > > The fist patch imports the files needed to support zstd decompression from > zstd-1.3.6 as-is. It is a very large patch. In case it doesn't make it, > I've included the commit hash and the script I used to download the files. > > Upstream zstd commit hash: 4fa456d7f12f8b27bd3b2f5dfd4f46898cb31c24 > Upstream zstd commit name: Merge pull request #1354 from facebook/dev In general I am happy with the patches. If everything is done what I have asked for then I will get them. However, there is one problem. Another big btrfs (RAID) patchset is lurking around for months. It is almost ready. Goffredo, I am looking at you... So, I would like to take RAID patchset first. If it is in the tree then I will ask you to rebase zstd patchset on top of RAID patchset and I will take it. Nick, are you OK with that? Daniel
> On Oct 11, 2018, at 11:15 AM, Daniel Kiper <dkiper@net-space.pl> wrote: > > Hi Nick, > > CC-ing Goffredo. > > On Tue, Oct 09, 2018 at 04:21:35PM -0700, Nick Terrell wrote: >> Hi all, >> >> This patch set imports the upstream zstd library, adds zstd support to the >> btrfs module, and adds a test case. I've also tested the patch set by storing >> my boot partition in btrfs with and without zstd compression and rebooting. >> >> The fist patch imports the files needed to support zstd decompression from >> zstd-1.3.6 as-is. It is a very large patch. In case it doesn't make it, >> I've included the commit hash and the script I used to download the files. >> >> Upstream zstd commit hash: 4fa456d7f12f8b27bd3b2f5dfd4f46898cb31c24 >> Upstream zstd commit name: Merge pull request #1354 from facebook/dev > > In general I am happy with the patches. If everything is done what > I have asked for then I will get them. However, there is one problem. > Another big btrfs (RAID) patchset is lurking around for months. It is > almost ready. Goffredo, I am looking at you... So, I would like to take > RAID patchset first. If it is in the tree then I will ask you to rebase > zstd patchset on top of RAID patchset and I will take it. Nick, are you > OK with that? That works for me, thanks! > Daniel
Hi all, This patch set imports the upstream zstd library, adds zstd support to the btrfs module, and adds a test case. I've also tested the patch set by storing my boot partition in btrfs with and without zstd compression and rebooting. The fist patch imports the files needed to support zstd decompression from zstd-1.3.6 as-is. It is a very large patch. In case it doesn't make it, I've included the commit hash and the script I used to download the files. Upstream zstd commit hash: 4fa456d7f12f8b27bd3b2f5dfd4f46898cb31c24 Upstream zstd commit name: Merge pull request #1354 from facebook/dev --- #!/bin/sh -e curl -L -O https://github.com/facebook/zstd/releases/download/v1.3.6/zstd-1.3.6.tar.gz curl -L -O https://github.com/facebook/zstd/releases/download/v1.3.6/zstd-1.3.6.tar.gz.sha256 sha256sum --check zstd-1.3.6.tar.gz.sha256 tar xzf zstd-1.3.6.tar.gz SRC_LIB="zstd-1.3.6/lib" DST_LIB="grub-core/lib/zstd" rm -rf $DST_LIB mkdir -p $DST_LIB cp $SRC_LIB/zstd.h $DST_LIB/ cp $SRC_LIB/common/*.[hc] $DST_LIB/ cp $SRC_LIB/decompress/*.[hc] $DST_LIB/ rm $DST_LIB/{pool.[hc],threading.[hc]} rm -rf zstd-1.3.6* echo SUCCESS! --- Best, Nick Terrell Changelog: v1 -> v2: - Switch to upstream zstd-1.3.6 and drop all the local patches. - Fix comments from Daniel Kiper. v2 -> v3: - Remove an extra file accidentally included in the first patch. - Use grub_error() to set grub_errno in grub_btrfs_zstd_decompress(). - Fix style and formatting comments. Nick Terrell (2): Import upstream zstd-1.3.6 btrfs: Add zstd support to grub btrfs Makefile.util.def | 10 +- grub-core/Makefile.core.def | 10 +- grub-core/fs/btrfs.c | 112 +- grub-core/lib/zstd/bitstream.h | 458 ++++ grub-core/lib/zstd/compiler.h | 133 ++ grub-core/lib/zstd/cpu.h | 215 ++ grub-core/lib/zstd/debug.c | 44 + grub-core/lib/zstd/debug.h | 123 + grub-core/lib/zstd/entropy_common.c | 236 ++ grub-core/lib/zstd/error_private.c | 48 + grub-core/lib/zstd/error_private.h | 76 + grub-core/lib/zstd/fse.h | 708 ++++++ grub-core/lib/zstd/fse_decompress.c | 309 +++ grub-core/lib/zstd/huf.h | 334 +++ grub-core/lib/zstd/huf_decompress.c | 1096 +++++++++ grub-core/lib/zstd/mem.h | 374 ++++ grub-core/lib/zstd/xxhash.c | 876 ++++++++ grub-core/lib/zstd/xxhash.h | 305 +++ grub-core/lib/zstd/zstd.h | 1516 +++++++++++++ grub-core/lib/zstd/zstd_common.c | 81 + grub-core/lib/zstd/zstd_decompress.c | 3108 ++++++++++++++++++++++++++ grub-core/lib/zstd/zstd_errors.h | 92 + grub-core/lib/zstd/zstd_internal.h | 257 +++ tests/btrfs_test.in | 1 + tests/util/grub-fs-tester.in | 2 +- 25 files changed, 10520 insertions(+), 4 deletions(-) create mode 100644 grub-core/lib/zstd/bitstream.h create mode 100644 grub-core/lib/zstd/compiler.h create mode 100644 grub-core/lib/zstd/cpu.h create mode 100644 grub-core/lib/zstd/debug.c create mode 100644 grub-core/lib/zstd/debug.h create mode 100644 grub-core/lib/zstd/entropy_common.c create mode 100644 grub-core/lib/zstd/error_private.c create mode 100644 grub-core/lib/zstd/error_private.h create mode 100644 grub-core/lib/zstd/fse.h create mode 100644 grub-core/lib/zstd/fse_decompress.c create mode 100644 grub-core/lib/zstd/huf.h create mode 100644 grub-core/lib/zstd/huf_decompress.c create mode 100644 grub-core/lib/zstd/mem.h create mode 100644 grub-core/lib/zstd/xxhash.c create mode 100644 grub-core/lib/zstd/xxhash.h create mode 100644 grub-core/lib/zstd/zstd.h create mode 100644 grub-core/lib/zstd/zstd_common.c create mode 100644 grub-core/lib/zstd/zstd_decompress.c create mode 100644 grub-core/lib/zstd/zstd_errors.h create mode 100644 grub-core/lib/zstd/zstd_internal.h -- 2.17.1