Skip to contentSkip to conversation

btrfs: Add zstd support to grub btrfs

PATCH Superseded Nick Terrell 3 participants 12 messages Oct 9, 2018 – Oct 11, 2018

Subsystems (3)

v3 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Nick Terrell

author maintainer
#0 lore raw

v3 cover btrfs: Add zstd support to grub btrfs

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

author maintainer
#1 lore raw

PATCH v3 1/2 Import upstream zstd-1.3.6

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

---
#!/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!
---

---
v1 -> v2:
- Switch to upstream zstd-1.3.6 rather than using the kernel zstd.

v2 -> v3:
- Remove the symlink "up" accidently incldued in the patch.

 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 +++
 20 files changed, 10389 insertions(+)
 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
grub-core/lib/zstd/bitstream.h new file +458 -0
new file mode 100644
index 000000000..2f91460c5
@@ -0,0 +1,458 @@
+/* ******************************************************************
+   bitstream
+   Part of FSE library
+   Copyright (C) 2013-present, Yann Collet.
+
+   BSD 2-Clause License (http://www.opensource.org/licenses/bsd-license.php)
+
+   Redistribution and use in source and binary forms, with or without
+   modification, are permitted provided that the following conditions are
+   met:
+
+       * Redistributions of source code must retain the above copyright
+   notice, this list of conditions and the following disclaimer.
+       * Redistributions in binary form must reproduce the above
+   copyright notice, this list of conditions and the following disclaimer
+   in the documentation and/or other materials provided with the
+   distribution.
+
+   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+   You can contact the author at :
+   - Source repository : https://github.com/Cyan4973/FiniteStateEntropy
+****************************************************************** */
+#ifndef BITSTREAM_H_MODULE
+#define BITSTREAM_H_MODULE
+
+#if defined (__cplusplus)
+extern "C" {
+#endif
+
+/*
+*  This API consists of small unitary functions, which must be inlined for best performance.
+*  Since link-time-optimization is not available for all compilers,
+*  these functions are defined into a .h to be included.
+*/
+
+/*-****************************************
+*  Dependencies
+******************************************/
+#include "mem.h"            /* unaligned access routines */
+#include "debug.h"          /* assert(), DEBUGLOG(), RAWLOG() */
+#include "error_private.h"  /* error codes and messages */
+
+
+/*=========================================
+*  Target specific
+=========================================*/
+#if defined(__BMI__) && defined(__GNUC__)
+#  include <immintrin.h>   /* support for bextr (experimental) */
+#endif
+
+#define STREAM_ACCUMULATOR_MIN_32  25
+#define STREAM_ACCUMULATOR_MIN_64  57
+#define STREAM_ACCUMULATOR_MIN    ((U32)(MEM_32bits() ? STREAM_ACCUMULATOR_MIN_32 : STREAM_ACCUMULATOR_MIN_64))
+
+
+/*-******************************************
+*  bitStream encoding API (write forward)
+********************************************/
+/* bitStream can mix input from multiple sources.
+ * A critical property of these streams is that they encode and decode in **reverse** direction.
+ * So the first bit sequence you add will be the last to be read, like a LIFO stack.
+ */
+typedef struct {
+    size_t bitContainer;
+    unsigned bitPos;
+    char*  startPtr;
+    char*  ptr;
+    char*  endPtr;
+} BIT_CStream_t;
+
+MEM_STATIC size_t BIT_initCStream(BIT_CStream_t* bitC, void* dstBuffer, size_t dstCapacity);
+MEM_STATIC void   BIT_addBits(BIT_CStream_t* bitC, size_t value, unsigned nbBits);
+MEM_STATIC void   BIT_flushBits(BIT_CStream_t* bitC);
+MEM_STATIC size_t BIT_closeCStream(BIT_CStream_t* bitC);
+
+/* Start with initCStream, providing the size of buffer to write into.
+*  bitStream will never write outside of this buffer.
+*  `dstCapacity` must be >= sizeof(bitD->bitContainer), otherwise @return will be an error code.
+*
+*  bits are first added to a local register.
+*  Local register is size_t, hence 64-bits on 64-bits systems, or 32-bits on 32-bits systems.
+*  Writing data into memory is an explicit operation, performed by the flushBits function.
+*  Hence keep track how many bits are potentially stored into local register to avoid register overflow.
+*  After a flushBits, a maximum of 7 bits might still be stored into local register.
+*
+*  Avoid storing elements of more than 24 bits if you want compatibility with 32-bits bitstream readers.
+*
+*  Last operation is to close the bitStream.
+*  The function returns the final size of CStream in bytes.
+*  If data couldn't fit into `dstBuffer`, it will return a 0 ( == not storable)
+*/
+
+
+/*-********************************************
+*  bitStream decoding API (read backward)
+**********************************************/
+typedef struct {
+    size_t   bitContainer;
+    unsigned bitsConsumed;
+    const char* ptr;
+    const char* start;
+    const char* limitPtr;
+} BIT_DStream_t;
+
+typedef enum { BIT_DStream_unfinished = 0,
+               BIT_DStream_endOfBuffer = 1,
+               BIT_DStream_completed = 2,
+               BIT_DStream_overflow = 3 } BIT_DStream_status;  /* result of BIT_reloadDStream() */
+               /* 1,2,4,8 would be better for bitmap combinations, but slows down performance a bit ... :( */
+
+MEM_STATIC size_t   BIT_initDStream(BIT_DStream_t* bitD, const void* srcBuffer, size_t srcSize);
+MEM_STATIC size_t   BIT_readBits(BIT_DStream_t* bitD, unsigned nbBits);
+MEM_STATIC BIT_DStream_status BIT_reloadDStream(BIT_DStream_t* bitD);
+MEM_STATIC unsigned BIT_endOfDStream(const BIT_DStream_t* bitD);
+
+
+/* Start by invoking BIT_initDStream().
+*  A chunk of the bitStream is then stored into a local register.
+*  Local register size is 64-bits on 64-bits systems, 32-bits on 32-bits systems (size_t).
+*  You can then retrieve bitFields stored into the local register, **in reverse order**.
+*  Local register is explicitly reloaded from memory by the BIT_reloadDStream() method.
+*  A reload guarantee a minimum of ((8*sizeof(bitD->bitContainer))-7) bits when its result is BIT_DStream_unfinished.
+*  Otherwise, it can be less than that, so proceed accordingly.
+*  Checking if DStream has reached its end can be performed with BIT_endOfDStream().
+*/
+
+
+/*-****************************************
+*  unsafe API
+******************************************/
+MEM_STATIC void BIT_addBitsFast(BIT_CStream_t* bitC, size_t value, unsigned nbBits);
+/* faster, but works only if value is "clean", meaning all high bits above nbBits are 0 */
+
+MEM_STATIC void BIT_flushBitsFast(BIT_CStream_t* bitC);
+/* unsafe version; does not check buffer overflow */
+
+MEM_STATIC size_t BIT_readBitsFast(BIT_DStream_t* bitD, unsigned nbBits);
+/* faster, but works only if nbBits >= 1 */
+
+
+
+/*-**************************************************************
+*  Internal functions
+****************************************************************/
+MEM_STATIC unsigned BIT_highbit32 (U32 val)
+{
+    assert(val != 0);
+    {
+#   if defined(_MSC_VER)   /* Visual */
+        unsigned long r=0;
+        _BitScanReverse ( &r, val );
+        return (unsigned) r;
+#   elif defined(__GNUC__) && (__GNUC__ >= 3)   /* Use GCC Intrinsic */
+        return 31 - __builtin_clz (val);
+#   else   /* Software version */
+        static const unsigned DeBruijnClz[32] = { 0,  9,  1, 10, 13, 21,  2, 29,
+                                                 11, 14, 16, 18, 22, 25,  3, 30,
+                                                  8, 12, 20, 28, 15, 17, 24,  7,
+                                                 19, 27, 23,  6, 26,  5,  4, 31 };
+        U32 v = val;
+        v |= v >> 1;
+        v |= v >> 2;
+        v |= v >> 4;
+        v |= v >> 8;
+        v |= v >> 16;
+        return DeBruijnClz[ (U32) (v * 0x07C4ACDDU) >> 27];
+#   endif
+    }
+}
+
+/*=====    Local Constants   =====*/
+static const unsigned BIT_mask[] = {
+    0,          1,         3,         7,         0xF,       0x1F,
+    0x3F,       0x7F,      0xFF,      0x1FF,     0x3FF,     0x7FF,
+    0xFFF,      0x1FFF,    0x3FFF,    0x7FFF,    0xFFFF,    0x1FFFF,
+    0x3FFFF,    0x7FFFF,   0xFFFFF,   0x1FFFFF,  0x3FFFFF,  0x7FFFFF,
+    0xFFFFFF,   0x1FFFFFF, 0x3FFFFFF, 0x7FFFFFF, 0xFFFFFFF, 0x1FFFFFFF,
+    0x3FFFFFFF, 0x7FFFFFFF}; /* up to 31 bits */
+#define BIT_MASK_SIZE (sizeof(BIT_mask) / sizeof(BIT_mask[0]))
+
+/*-**************************************************************
+*  bitStream encoding
+****************************************************************/
+/*! BIT_initCStream() :
+ *  `dstCapacity` must be > sizeof(size_t)
+ *  @return : 0 if success,
+ *            otherwise an error code (can be tested using ERR_isError()) */
+MEM_STATIC size_t BIT_initCStream(BIT_CStream_t* bitC,
+                                  void* startPtr, size_t dstCapacity)
+{
+    bitC->bitContainer = 0;
+    bitC->bitPos = 0;
+    bitC->startPtr = (char*)startPtr;
+    bitC->ptr = bitC->startPtr;
+    bitC->endPtr = bitC->startPtr + dstCapacity - sizeof(bitC->bitContainer);
+    if (dstCapacity <= sizeof(bitC->bitContainer)) return ERROR(dstSize_tooSmall);
+    return 0;
+}
+
+/*! BIT_addBits() :
+ *  can add up to 31 bits into `bitC`.
+ *  Note : does not check for register overflow ! */
+MEM_STATIC void BIT_addBits(BIT_CStream_t* bitC,
+                            size_t value, unsigned nbBits)
+{
+    MEM_STATIC_ASSERT(BIT_MASK_SIZE == 32);
+    assert(nbBits < BIT_MASK_SIZE);
+    assert(nbBits + bitC->bitPos < sizeof(bitC->bitContainer) * 8);
+    bitC->bitContainer |= (value & BIT_mask[nbBits]) << bitC->bitPos;
+    bitC->bitPos += nbBits;
+}
+
+/*! BIT_addBitsFast() :
+ *  works only if `value` is _clean_,
+ *  meaning all high bits above nbBits are 0 */
+MEM_STATIC void BIT_addBitsFast(BIT_CStream_t* bitC,
+                                size_t value, unsigned nbBits)
+{
+    assert((value>>nbBits) == 0);
+    assert(nbBits + bitC->bitPos < sizeof(bitC->bitContainer) * 8);
+    bitC->bitContainer |= value << bitC->bitPos;
+    bitC->bitPos += nbBits;
+}
+
+/*! BIT_flushBitsFast() :
+ *  assumption : bitContainer has not overflowed
+ *  unsafe version; does not check buffer overflow */
+MEM_STATIC void BIT_flushBitsFast(BIT_CStream_t* bitC)
+{
+    size_t const nbBytes = bitC->bitPos >> 3;
+    assert(bitC->bitPos < sizeof(bitC->bitContainer) * 8);
+    MEM_writeLEST(bitC->ptr, bitC->bitContainer);
+    bitC->ptr += nbBytes;
+    assert(bitC->ptr <= bitC->endPtr);
+    bitC->bitPos &= 7;
+    bitC->bitContainer >>= nbBytes*8;
+}
+
+/*! BIT_flushBits() :
+ *  assumption : bitContainer has not overflowed
+ *  safe version; check for buffer overflow, and prevents it.
+ *  note : does not signal buffer overflow.
+ *  overflow will be revealed later on using BIT_closeCStream() */
+MEM_STATIC void BIT_flushBits(BIT_CStream_t* bitC)
+{
+    size_t const nbBytes = bitC->bitPos >> 3;
+    assert(bitC->bitPos < sizeof(bitC->bitContainer) * 8);
+    MEM_writeLEST(bitC->ptr, bitC->bitContainer);
+    bitC->ptr += nbBytes;
+    if (bitC->ptr > bitC->endPtr) bitC->ptr = bitC->endPtr;
+    bitC->bitPos &= 7;
+    bitC->bitContainer >>= nbBytes*8;
+}
+
+/*! BIT_closeCStream() :
+ *  @return : size of CStream, in bytes,
+ *            or 0 if it could not fit into dstBuffer */
+MEM_STATIC size_t BIT_closeCStream(BIT_CStream_t* bitC)
+{
+    BIT_addBitsFast(bitC, 1, 1);   /* endMark */
+    BIT_flushBits(bitC);
+    if (bitC->ptr >= bitC->endPtr) return 0; /* overflow detected */
+    return (bitC->ptr - bitC->startPtr) + (bitC->bitPos > 0);
+}
+
+
+/*-********************************************************
+*  bitStream decoding
+**********************************************************/
+/*! BIT_initDStream() :
+ *  Initialize a BIT_DStream_t.
+ * `bitD` : a pointer to an already allocated BIT_DStream_t structure.
+ * `srcSize` must be the *exact* size of the bitStream, in bytes.
+ * @return : size of stream (== srcSize), or an errorCode if a problem is detected
+ */
+MEM_STATIC size_t BIT_initDStream(BIT_DStream_t* bitD, const void* srcBuffer, size_t srcSize)
+{
+    if (srcSize < 1) { memset(bitD, 0, sizeof(*bitD)); return ERROR(srcSize_wrong); }
+
+    bitD->start = (const char*)srcBuffer;
+    bitD->limitPtr = bitD->start + sizeof(bitD->bitContainer);
+
+    if (srcSize >=  sizeof(bitD->bitContainer)) {  /* normal case */
+        bitD->ptr   = (const char*)srcBuffer + srcSize - sizeof(bitD->bitContainer);
+        bitD->bitContainer = MEM_readLEST(bitD->ptr);
+        { BYTE const lastByte = ((const BYTE*)srcBuffer)[srcSize-1];
+          bitD->bitsConsumed = lastByte ? 8 - BIT_highbit32(lastByte) : 0;  /* ensures bitsConsumed is always set */
+          if (lastByte == 0) return ERROR(GENERIC); /* endMark not present */ }
+    } else {
+        bitD->ptr   = bitD->start;
+        bitD->bitContainer = *(const BYTE*)(bitD->start);
+        switch(srcSize)
+        {
+        case 7: bitD->bitContainer += (size_t)(((const BYTE*)(srcBuffer))[6]) << (sizeof(bitD->bitContainer)*8 - 16);
+                /* fall-through */
+
+        case 6: bitD->bitContainer += (size_t)(((const BYTE*)(srcBuffer))[5]) << (sizeof(bitD->bitContainer)*8 - 24);
+                /* fall-through */
+
+        case 5: bitD->bitContainer += (size_t)(((const BYTE*)(srcBuffer))[4]) << (sizeof(bitD->bitContainer)*8 - 32);
+                /* fall-through */
+
+        case 4: bitD->bitContainer += (size_t)(((const BYTE*)(srcBuffer))[3]) << 24;
+                /* fall-through */
+
+        case 3: bitD->bitContainer += (size_t)(((const BYTE*)(srcBuffer))[2]) << 16;
+                /* fall-through */
+
+        case 2: bitD->bitContainer += (size_t)(((const BYTE*)(srcBuffer))[1]) <<  8;
+                /* fall-through */
+
+        default: break;
+        }
+        {   BYTE const lastByte = ((const BYTE*)srcBuffer)[srcSize-1];
+            bitD->bitsConsumed = lastByte ? 8 - BIT_highbit32(lastByte) : 0;
+            if (lastByte == 0) return ERROR(corruption_detected);  /* endMark not present */
+        }
+        bitD->bitsConsumed += (U32)(sizeof(bitD->bitContainer) - srcSize)*8;
+    }
+
+    return srcSize;
+}
+
+MEM_STATIC size_t BIT_getUpperBits(size_t bitContainer, U32 const start)
+{
+    return bitContainer >> start;
+}
+
+MEM_STATIC size_t BIT_getMiddleBits(size_t bitContainer, U32 const start, U32 const nbBits)
+{
+#if defined(__BMI__) && defined(__GNUC__) && __GNUC__*1000+__GNUC_MINOR__ >= 4008  /* experimental */
+#  if defined(__x86_64__)
+    if (sizeof(bitContainer)==8)
+        return _bextr_u64(bitContainer, start, nbBits);
+    else
+#  endif
+        return _bextr_u32(bitContainer, start, nbBits);
+#else
+    assert(nbBits < BIT_MASK_SIZE);
+    return (bitContainer >> start) & BIT_mask[nbBits];
+#endif
+}
+
+MEM_STATIC size_t BIT_getLowerBits(size_t bitContainer, U32 const nbBits)
+{
+    assert(nbBits < BIT_MASK_SIZE);
+    return bitContainer & BIT_mask[nbBits];
+}
+
+/*! BIT_lookBits() :
+ *  Provides next n bits from local register.
+ *  local register is not modified.
+ *  On 32-bits, maxNbBits==24.
+ *  On 64-bits, maxNbBits==56.
+ * @return : value extracted */
+MEM_STATIC size_t BIT_lookBits(const BIT_DStream_t* bitD, U32 nbBits)
+{
+#if defined(__BMI__) && defined(__GNUC__)   /* experimental; fails if bitD->bitsConsumed + nbBits > sizeof(bitD->bitContainer)*8 */
+    return BIT_getMiddleBits(bitD->bitContainer, (sizeof(bitD->bitContainer)*8) - bitD->bitsConsumed - nbBits, nbBits);
+#else
+    U32 const regMask = sizeof(bitD->bitContainer)*8 - 1;
+    return ((bitD->bitContainer << (bitD->bitsConsumed & regMask)) >> 1) >> ((regMask-nbBits) & regMask);
+#endif
+}
+
+/*! BIT_lookBitsFast() :
+ *  unsafe version; only works if nbBits >= 1 */
+MEM_STATIC size_t BIT_lookBitsFast(const BIT_DStream_t* bitD, U32 nbBits)
+{
+    U32 const regMask = sizeof(bitD->bitContainer)*8 - 1;
+    assert(nbBits >= 1);
+    return (bitD->bitContainer << (bitD->bitsConsumed & regMask)) >> (((regMask+1)-nbBits) & regMask);
+}
+
+MEM_STATIC void BIT_skipBits(BIT_DStream_t* bitD, U32 nbBits)
+{
+    bitD->bitsConsumed += nbBits;
+}
+
+/*! BIT_readBits() :
+ *  Read (consume) next n bits from local register and update.
+ *  Pay attention to not read more than nbBits contained into local register.
+ * @return : extracted value. */
+MEM_STATIC size_t BIT_readBits(BIT_DStream_t* bitD, U32 nbBits)
+{
+    size_t const value = BIT_lookBits(bitD, nbBits);
+    BIT_skipBits(bitD, nbBits);
+    return value;
+}
+
+/*! BIT_readBitsFast() :
+ *  unsafe version; only works only if nbBits >= 1 */
+MEM_STATIC size_t BIT_readBitsFast(BIT_DStream_t* bitD, U32 nbBits)
+{
+    size_t const value = BIT_lookBitsFast(bitD, nbBits);
+    assert(nbBits >= 1);
+    BIT_skipBits(bitD, nbBits);
+    return value;
+}
+
+/*! BIT_reloadDStream() :
+ *  Refill `bitD` from buffer previously set in BIT_initDStream() .
+ *  This function is safe, it guarantees it will not read beyond src buffer.
+ * @return : status of `BIT_DStream_t` internal register.
+ *           when status == BIT_DStream_unfinished, internal register is filled with at least 25 or 57 bits */
+MEM_STATIC BIT_DStream_status BIT_reloadDStream(BIT_DStream_t* bitD)
+{
+    if (bitD->bitsConsumed > (sizeof(bitD->bitContainer)*8))  /* overflow detected, like end of stream */
+        return BIT_DStream_overflow;
+
+    if (bitD->ptr >= bitD->limitPtr) {
+        bitD->ptr -= bitD->bitsConsumed >> 3;
+        bitD->bitsConsumed &= 7;
+        bitD->bitContainer = MEM_readLEST(bitD->ptr);
+        return BIT_DStream_unfinished;
+    }
+    if (bitD->ptr == bitD->start) {
+        if (bitD->bitsConsumed < sizeof(bitD->bitContainer)*8) return BIT_DStream_endOfBuffer;
+        return BIT_DStream_completed;
+    }
+    /* start < ptr < limitPtr */
+    {   U32 nbBytes = bitD->bitsConsumed >> 3;
+        BIT_DStream_status result = BIT_DStream_unfinished;
+        if (bitD->ptr - nbBytes < bitD->start) {
+            nbBytes = (U32)(bitD->ptr - bitD->start);  /* ptr > start */
+            result = BIT_DStream_endOfBuffer;
+        }
+        bitD->ptr -= nbBytes;
+        bitD->bitsConsumed -= nbBytes*8;
+        bitD->bitContainer = MEM_readLEST(bitD->ptr);   /* reminder : srcSize > sizeof(bitD->bitContainer), otherwise bitD->ptr == bitD->start */
+        return result;
+    }
+}
+
+/*! BIT_endOfDStream() :
+ * @return : 1 if DStream has _exactly_ reached its end (all bits consumed).
+ */
+MEM_STATIC unsigned BIT_endOfDStream(const BIT_DStream_t* DStream)
+{
+    return ((DStream->ptr == DStream->start) && (DStream->bitsConsumed == sizeof(DStream->bitContainer)*8));
+}
+
+#if defined (__cplusplus)
+}
+#endif
+
+#endif /* BITSTREAM_H_MODULE */
grub-core/lib/zstd/compiler.h new file +133 -0

133 lines not on this page · raw message

grub-core/lib/zstd/cpu.h new file +215 -0

215 lines not on this page · raw message

grub-core/lib/zstd/debug.c new file +44 -0

44 lines not on this page · raw message

grub-core/lib/zstd/debug.h new file +123 -0

123 lines not on this page · raw message

grub-core/lib/zstd/entropy_common.c new file +236 -0

236 lines not on this page · raw message

grub-core/lib/zstd/error_private.c new file +48 -0

48 lines not on this page · raw message

grub-core/lib/zstd/error_private.h new file +76 -0

76 lines not on this page · raw message

grub-core/lib/zstd/fse.h new file +708 -0

708 lines not on this page · raw message

grub-core/lib/zstd/fse_decompress.c new file +309 -0

309 lines not on this page · raw message

grub-core/lib/zstd/huf.h new file +334 -0

334 lines not on this page · raw message

grub-core/lib/zstd/huf_decompress.c new file +1096 -0

316 lines not on this page · raw message

grub-core/lib/zstd/mem.h new file +374 -0

374 lines not on this page · raw message

grub-core/lib/zstd/xxhash.c new file +876 -0

876 lines not on this page · raw message

grub-core/lib/zstd/xxhash.h new file +305 -0

305 lines not on this page · raw message

grub-core/lib/zstd/zstd.h new file +1516 -0

1,516 lines not on this page · raw message

grub-core/lib/zstd/zstd_common.c new file +81 -0

81 lines not on this page · raw message

grub-core/lib/zstd/zstd_decompress.c new file +3108 -0

3,108 lines not on this page · raw message

grub-core/lib/zstd/zstd_errors.h new file +92 -0

92 lines not on this page · raw message

grub-core/lib/zstd/zstd_internal.h new file +257 -0

257 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

--
2.17.1

Part of the diff is not shown here. View the raw message.

Reviewed-by on v3 1/2
#2 lore raw

Re: [PATCH v3 1/2] Import upstream zstd-1.3.6

Quoted 9 lines · Nick Terrell

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

Quoted 6 lines

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.

Quoted 7 lines

#!/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

Quoted 11 lines

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.

Quoted 1 line

If you fix things above you can add

Daniel

author maintainer
#3 lore raw

PATCH v3 2/2 btrfs: Add zstd support to grub btrfs

In reply to Nick Terrell

Adds zstd support to the btrfs module.

Tested on Ubuntu-18.04 with a btrfs /boot partition with and without zstd compression. A test case was also added to the test suite that fails before the patch, and passes after.

---
v1 -> v2:
- Fix comments from Daniel Kiper.

v2 -> v3:
- Use grub_error() to set grub_errno in grub_btrfs_zstd_decompress().
- Fix style and formatting comments.

 Makefile.util.def            |  10 +++-
 grub-core/Makefile.core.def  |  10 +++-
 grub-core/fs/btrfs.c         | 112 ++++++++++++++++++++++++++++++++++-
 tests/btrfs_test.in          |   1 +
 tests/util/grub-fs-tester.in |   2 +-
 5 files changed, 131 insertions(+), 4 deletions(-)
Makefile.util.def +9 -1
index f9caccb97..27c5e9903 100644
@@ -54,7 +54,7 @@ library = {
 library = {
   name = libgrubmods.a;
   cflags = '-fno-builtin -Wno-undef';
-  cppflags = '-I$(top_srcdir)/grub-core/lib/minilzo -I$(srcdir)/grub-core/lib/xzembed -DMINILZO_HAVE_CONFIG_H';
+  cppflags = '-I$(srcdir)/grub-core/lib/minilzo -I$(srcdir)/grub-core/lib/xzembed -I$(srcdir)/grub-core/lib/zstd -DMINILZO_HAVE_CONFIG_H';
 
   common_nodist = grub_script.tab.c;
   common_nodist = grub_script.yy.c;
@@ -164,6 +164,14 @@ library = {
   common = grub-core/lib/xzembed/xz_dec_bcj.c;
   common = grub-core/lib/xzembed/xz_dec_lzma2.c;
   common = grub-core/lib/xzembed/xz_dec_stream.c;
+  common = grub-core/lib/zstd/debug.c;
+  common = grub-core/lib/zstd/entropy_common.c;
+  common = grub-core/lib/zstd/error_private.c;
+  common = grub-core/lib/zstd/fse_decompress.c;
+  common = grub-core/lib/zstd/huf_decompress.c;
+  common = grub-core/lib/zstd/xxhash.c;
+  common = grub-core/lib/zstd/zstd_common.c;
+  common = grub-core/lib/zstd/zstd_decompress.c;
 };
 
 program = {
grub-core/Makefile.core.def +9 -1
index 2c1d62cee..f818f58bc 100644
@@ -1265,8 +1265,16 @@ module = {
   name = btrfs;
   common = fs/btrfs.c;
   common = lib/crc.c;
+  common = lib/zstd/debug.c;
+  common = lib/zstd/entropy_common.c;
+  common = lib/zstd/error_private.c;
+  common = lib/zstd/fse_decompress.c;
+  common = lib/zstd/huf_decompress.c;
+  common = lib/zstd/xxhash.c;
+  common = lib/zstd/zstd_common.c;
+  common = lib/zstd/zstd_decompress.c;
   cflags = '$(CFLAGS_POSIX) -Wno-undef';
-  cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo -DMINILZO_HAVE_CONFIG_H';
+  cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo -I$(srcdir)/lib/zstd -DMINILZO_HAVE_CONFIG_H';
 };
 
 module = {
grub-core/fs/btrfs.c +111 -1

160 lines not on this page · raw message

tests/btrfs_test.in +1 -0
index 2b37ddd33..0c9bf3a68 100644
@@ -18,6 +18,7 @@ fi
 "@builddir@/grub-fs-tester" btrfs
 "@builddir@/grub-fs-tester" btrfs_zlib
 "@builddir@/grub-fs-tester" btrfs_lzo
+"@builddir@/grub-fs-tester" btrfs_zstd
 "@builddir@/grub-fs-tester" btrfs_raid0
 "@builddir@/grub-fs-tester" btrfs_raid1
 "@builddir@/grub-fs-tester" btrfs_single
tests/util/grub-fs-tester.in +1 -1
index 15969d796..3470c89e3 100644
@@ -626,7 +626,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    ;;
 		x"btrfs")
 		    "mkfs.btrfs" -s $SECSIZE -L "$FSLABEL" "${MOUNTDEVICE}" ;;
-		x"btrfs_zlib" | x"btrfs_lzo")
+		x"btrfs_zlib" | x"btrfs_lzo" | x"btrfs_zstd")
 		    "mkfs.btrfs" -s $SECSIZE -L "$FSLABEL" "${MOUNTDEVICE}"
 		    MOUNTOPTS="compress=${fs/btrfs_/},"
 		    MOUNTFS="btrfs"

--
2.17.1

#4 lore raw
Quoted 60 lines · Nick Terrell

Adds zstd support to the btrfs module.

Tested on Ubuntu-18.04 with a btrfs /boot partition with and without zstd compression. A test case was also added to the test suite that fails before the patch, and passes after.

---
v1 -> v2:
- Fix comments from Daniel Kiper.

v2 -> v3:
- Use grub_error() to set grub_errno in grub_btrfs_zstd_decompress().
- Fix style and formatting comments.

 Makefile.util.def            |  10 +++-
 grub-core/Makefile.core.def  |  10 +++-
 grub-core/fs/btrfs.c         | 112 ++++++++++++++++++++++++++++++++++-
 tests/btrfs_test.in          |   1 +
 tests/util/grub-fs-tester.in |   2 +-
 5 files changed, 131 insertions(+), 4 deletions(-)
Makefile.util.def +9 -1
index f9caccb97..27c5e9903 100644
@@ -54,7 +54,7 @@ library = {
 library = {
   name = libgrubmods.a;
   cflags = '-fno-builtin -Wno-undef';
-  cppflags = '-I$(top_srcdir)/grub-core/lib/minilzo -I$(srcdir)/grub-core/lib/xzembed -DMINILZO_HAVE_CONFIG_H';
+  cppflags = '-I$(srcdir)/grub-core/lib/minilzo -I$(srcdir)/grub-core/lib/xzembed -I$(srcdir)/grub-core/lib/zstd -DMINILZO_HAVE_CONFIG_H';
 
   common_nodist = grub_script.tab.c;
   common_nodist = grub_script.yy.c;
@@ -164,6 +164,14 @@ library = {
   common = grub-core/lib/xzembed/xz_dec_bcj.c;
   common = grub-core/lib/xzembed/xz_dec_lzma2.c;
   common = grub-core/lib/xzembed/xz_dec_stream.c;
+  common = grub-core/lib/zstd/debug.c;
+  common = grub-core/lib/zstd/entropy_common.c;
+  common = grub-core/lib/zstd/error_private.c;
+  common = grub-core/lib/zstd/fse_decompress.c;
+  common = grub-core/lib/zstd/huf_decompress.c;
+  common = grub-core/lib/zstd/xxhash.c;
+  common = grub-core/lib/zstd/zstd_common.c;
+  common = grub-core/lib/zstd/zstd_decompress.c;
 };
 
 program = {
grub-core/Makefile.core.def +2 -0
index 2c1d62cee..f818f58bc 100644
@@ -1265,8 +1265,16 @@ module = {
   name = btrfs;
   common = fs/btrfs.c;
   common = lib/crc.c;
+  common = lib/zstd/debug.c;
+  common = lib/zstd/entropy_common.c;

grub-core/Makefile.core.def module = { · line 1278

Quoted 10 lines
grub-core/Makefile.core.def +7 -1
+  common = lib/zstd/error_private.c;
+  common = lib/zstd/fse_decompress.c;
+  common = lib/zstd/huf_decompress.c;
+  common = lib/zstd/xxhash.c;
+  common = lib/zstd/zstd_common.c;
+  common = lib/zstd/zstd_decompress.c;
   cflags = '$(CFLAGS_POSIX) -Wno-undef';
-  cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo -DMINILZO_HAVE_CONFIG_H';
+  cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo -I$(srcdir)/lib/zstd -DMINILZO_HAVE_CONFIG_H';
 };

Please do not embed zstd lib into btrfs module here. I would like to see zstd lib as separate module if possible.

Quoted 41 lines
grub-core/Makefile.core.def +0 -0
 module = {
grub-core/fs/btrfs.c +7 -0
index 4849c1ceb..c44fe8029 100644
@@ -17,6 +17,8 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#define ZSTD_STATIC_LINKING_ONLY
+
 #include <grub/err.h>
 #include <grub/file.h>
 #include <grub/mm.h>
@@ -27,6 +29,7 @@
 #include <grub/lib/crc.h>
 #include <grub/deflate.h>
 #include <minilzo.h>
+#include <zstd.h>
 #include <grub/i18n.h>
 #include <grub/btrfs.h>
 
@@ -45,6 +48,9 @@ GRUB_MOD_LICENSE ("GPLv3+");
 #define GRUB_BTRFS_LZO_BLOCK_MAX_CSIZE (GRUB_BTRFS_LZO_BLOCK_SIZE + \
 				     (GRUB_BTRFS_LZO_BLOCK_SIZE / 16) + 64 + 3)
 
+#define ZSTD_BTRFS_MAX_WINDOWLOG 17
+#define ZSTD_BTRFS_MAX_INPUT     (1 << ZSTD_BTRFS_MAX_WINDOWLOG)
+
 typedef grub_uint8_t grub_btrfs_checksum_t[0x20];
 typedef grub_uint16_t grub_btrfs_uuid_t[8];
 
@@ -212,6 +218,7 @@ struct grub_btrfs_extent_data
 #define GRUB_BTRFS_COMPRESSION_NONE 0
 #define GRUB_BTRFS_COMPRESSION_ZLIB 1
 #define GRUB_BTRFS_COMPRESSION_LZO  2
+#define GRUB_BTRFS_COMPRESSION_ZSTD 3
 
 #define GRUB_BTRFS_OBJECT_ID_CHUNK 0x100
 

grub-core/fs/btrfs.c grub_btrfs_read_inode (struct grub_btrfs_data *data, · line 924

Quoted 6 lines
grub-core/fs/btrfs.c +3 -0
@@ -912,6 +919,95 @@ grub_btrfs_read_inode (struct grub_btrfs_data *data,
   return grub_btrfs_read_logical (data, elemaddr, inode, sizeof (*inode), 0);
 }
 
+static void* grub_zstd_malloc (void* state, size_t size)
+{
+	(void)state;

Please drop this and use "__attribute__ ((unused))" in function prototype.

grub-core/fs/btrfs.c grub_btrfs_read_inode (struct grub_btrfs_data *data, · line 931

Quoted 6 lines
grub-core/fs/btrfs.c +6 -0
+	return grub_malloc (size);
+}
+
+static void grub_zstd_free (void* state, void* address)
+{
+	(void)state;

Ditto.

Quoted 22 lines
grub-core/fs/btrfs.c +22 -0
+	return grub_free (address);
+}
+
+static ZSTD_customMem grub_zstd_allocator (void)
+{
+	ZSTD_customMem allocator;
+
+	allocator.customAlloc = &grub_zstd_malloc;
+	allocator.customFree = &grub_zstd_free;
+	allocator.opaque = NULL;
+
+	return allocator;
+}
+
+static grub_ssize_t
+grub_btrfs_zstd_decompress (char *ibuf, grub_size_t isize, grub_off_t off,
+			    char *obuf, grub_size_t osize)
+{
+	void *allocated = NULL;
+	char *otmpbuf = obuf;
+	grub_size_t otmpsize = osize;
+	ZSTD_DCtx *dctx = NULL;

grub-core/fs/btrfs.c grub_btrfs_read_inode (struct grub_btrfs_data *data, · line 964

Quoted 10 lines
grub-core/fs/btrfs.c +10 -0
+	grub_size_t zstd_ret;
+	grub_ssize_t ret = -1;
+
+	/*
+	 * Zstd will fail if it can't fit the entire output in the destination
+	 * buffer, so if osize isn't large enough, allocate a temporary buffer.
+	 */
+	if (otmpsize < ZSTD_BTRFS_MAX_INPUT) {
+		allocated = grub_malloc (ZSTD_BTRFS_MAX_INPUT);
+		if (!allocated) {

Hmmm... Should not we say something here? Like grub_error() call below? It seems to me that failing silently is bad idea here.

grub-core/fs/btrfs.c grub_btrfs_read_inode (struct grub_btrfs_data *data, · line 976

Quoted 12 lines
grub-core/fs/btrfs.c +12 -0
+			goto err;
+		}
+		otmpbuf = (char*)allocated;
+		otmpsize = ZSTD_BTRFS_MAX_INPUT;
+	}
+
+	/* Create the ZSTD_DCtx. */
+	dctx = ZSTD_createDCtx_advanced (grub_zstd_allocator ());
+	if (!dctx) {
+		grub_error (GRUB_ERR_OUT_OF_MEMORY, "zstd out of memory");
+		goto err;
+	}

Daniel

author maintainer
#5 lore raw
Quoted 70 lines · Daniel Kiper
Quoted 65 lines · Nick Terrell

Adds zstd support to the btrfs module.

Tested on Ubuntu-18.04 with a btrfs /boot partition with and without zstd compression. A test case was also added to the test suite that fails before the patch, and passes after.

---
v1 -> v2:
- Fix comments from Daniel Kiper.

v2 -> v3:
- Use grub_error() to set grub_errno in grub_btrfs_zstd_decompress().
- Fix style and formatting comments.

Makefile.util.def            |  10 +++-
grub-core/Makefile.core.def  |  10 +++-
grub-core/fs/btrfs.c         | 112 ++++++++++++++++++++++++++++++++++-
tests/btrfs_test.in          |   1 +
tests/util/grub-fs-tester.in |   2 +-
5 files changed, 131 insertions(+), 4 deletions(-)
Makefile.util.def +9 -1
index f9caccb97..27c5e9903 100644
@@ -54,7 +54,7 @@ library = {
 library = {
  name = libgrubmods.a;
  cflags = '-fno-builtin -Wno-undef';
-  cppflags = '-I$(top_srcdir)/grub-core/lib/minilzo -I$(srcdir)/grub-core/lib/xzembed -DMINILZO_HAVE_CONFIG_H';
+  cppflags = '-I$(srcdir)/grub-core/lib/minilzo -I$(srcdir)/grub-core/lib/xzembed -I$(srcdir)/grub-core/lib/zstd -DMINILZO_HAVE_CONFIG_H';
 
  common_nodist = grub_script.tab.c;
  common_nodist = grub_script.yy.c;
@@ -164,6 +164,14 @@ library = {
  common = grub-core/lib/xzembed/xz_dec_bcj.c;
  common = grub-core/lib/xzembed/xz_dec_lzma2.c;
  common = grub-core/lib/xzembed/xz_dec_stream.c;
+  common = grub-core/lib/zstd/debug.c;
+  common = grub-core/lib/zstd/entropy_common.c;
+  common = grub-core/lib/zstd/error_private.c;
+  common = grub-core/lib/zstd/fse_decompress.c;
+  common = grub-core/lib/zstd/huf_decompress.c;
+  common = grub-core/lib/zstd/xxhash.c;
+  common = grub-core/lib/zstd/zstd_common.c;
+  common = grub-core/lib/zstd/zstd_decompress.c;
 };
 
 program = {
grub-core/Makefile.core.def +7 -0
index 2c1d62cee..f818f58bc 100644
@@ -1265,8 +1265,16 @@ module = {
  name = btrfs;
  common = fs/btrfs.c;
  common = lib/crc.c;
+  common = lib/zstd/debug.c;
+  common = lib/zstd/entropy_common.c;
+  common = lib/zstd/error_private.c;
+  common = lib/zstd/fse_decompress.c;
+  common = lib/zstd/huf_decompress.c;
+  common = lib/zstd/xxhash.c;
+  common = lib/zstd/zstd_common.c;
Quoted 8 lines

grub-core/Makefile.core.def module = { · line 1278

Quoted 5 lines
grub-core/Makefile.core.def +2 -1
+  common = lib/zstd/zstd_decompress.c;
  cflags = '$(CFLAGS_POSIX) -Wno-undef';
-  cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo -DMINILZO_HAVE_CONFIG_H';
+  cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo -I$(srcdir)/lib/zstd -DMINILZO_HAVE_CONFIG_H';
 };

Please do not embed zstd lib into btrfs module here. I would like to see zstd lib as separate module if possible.

Sure, I'm not exactly sure how the build system works. I haven't been able to find any documentation, if there is some can you point me to it? I think the zstd module should look like:

module = {
name = zstd;
common = lib/zstd/debug.c;
common = lib/zstd/entropy_common.c;
common = lib/zstd/error_private.c;
common = lib/zstd/fse_decompress.c;
common = lib/zstd/huf_decompress.c;
common = lib/zstd/xxhash.c;
common = lib/zstd/zstd_common.c;
common = lib/zstd/zstd_decompress.c;
cflags = '$(CFLAGS_POSIX) -Wno-undef';
cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/zstd';
};

Then how do I add a dependency on it in btrfs?

Quoted 13 lines
Quoted 13 lines
grub-core/Makefile.core.def +13 -0
+static grub_ssize_t
+grub_btrfs_zstd_decompress (char *ibuf, grub_size_t isize, grub_off_t off,
+			    char *obuf, grub_size_t osize)
+{
+	void *allocated = NULL;
+	char *otmpbuf = obuf;
+	grub_size_t otmpsize = osize;
+	ZSTD_DCtx *dctx = NULL;
+	grub_size_t zstd_ret;
+	grub_ssize_t ret = -1;
+
+	/*
+	 * Zstd will fail if it can't fit the entire output in the destination
Quoted 8 lines

grub-core/Makefile.core.def

Quoted 5 lines
grub-core/Makefile.core.def +5 -0
+	 * buffer, so if osize isn't large enough, allocate a temporary buffer.
+	 */
+	if (otmpsize < ZSTD_BTRFS_MAX_INPUT) {
+		allocated = grub_malloc (ZSTD_BTRFS_MAX_INPUT);
+		if (!allocated) {

Hmmm... Should not we say something here? Like grub_error() call below? It seems to me that failing silently is bad idea here.

grub_malloc() already calls grub_error with the message "Out of memory".

Thanks,
Nick

#6 lore raw
In reply to Nick Terrell

Dear Nick,

Thank you very much for your patches.

Am 10.10.2018 um 01:21 schrieb Nick Terrell:

Quoted 19 lines

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

Quoted 11 lines

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

author maintainer
#7 lore raw
Quoted 7 lines · Paul Menzel

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

#8 lore raw

Dear Nick,

Am 10.10.2018 um 22:28 schrieb Nick Terrell:

Quoted 12 lines
Quoted 8 lines · Paul Menzel

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.

Quoted 6 lines

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.

Quoted 1 line

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

#9 lore raw
In reply to Nick Terrell
Quoted 8 lines · Nick Terrell
Quoted 5 lines · Paul Menzel

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.

Quoted 7 lines

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

#10 lore raw
In reply to Nick Terrell

Hi Nick,

CC-ing Goffredo.

Quoted 6 lines · Nick Terrell

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.

Quoted 6 lines

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

author maintainer
#11 lore raw
Quoted 22 lines · Daniel Kiper

Hi Nick,

CC-ing Goffredo.

Quoted 6 lines · Nick Terrell

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.

Quoted 6 lines

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

Quoted 7 lines

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!

Quoted 1 line

Daniel