| Message ID | 20180828013654.1627080-1-terrelln@fb.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: Add zstd support to btrfs | expand |
On Mon, Aug 27, 2018 at 06:36:51PM -0700, Nick Terrell wrote: > Hi all, > > This patch set imports the upstream kernel zstd library, patches it to work > in grub, adds zstd support to the btrfs module, and adds a test case. I've > separated the importing and patching of the upstream kernel zstd library > for ease of review. > > Thanks to David Sterba for starting this project! I started my patch from > where he left off. The first patch did not make it to vger.kernel.org mailinglist, which suggests that it's big, and possibly too big for merging. I did only a prototype to make it work, I'm afraid the patch won't be accepted. Your follow patches clean it up and IMHO go in the right direction, but I think this needs to be done from the beginning.
> On Sep 11, 2018, at 3:23 AM, David Sterba <dsterba@suse.cz> wrote: > > On Mon, Aug 27, 2018 at 06:36:51PM -0700, Nick Terrell wrote: >> Hi all, >> >> This patch set imports the upstream kernel zstd library, patches it to work >> in grub, adds zstd support to the btrfs module, and adds a test case. I've >> separated the importing and patching of the upstream kernel zstd library >> for ease of review. >> >> Thanks to David Sterba for starting this project! I started my patch from >> where he left off. > > The first patch did not make it to vger.kernel.org mailinglist, which > suggests that it's big, and possibly too big for merging. I did only a > prototype to make it work, I'm afraid the patch won't be accepted. Hmm. We need to import zstd into grub to use it, so we will need a large patch somewhere in the stack. The first patch is a copy as-is from the linux kernel, and the second applies all the patches I need on top of it. I kept it that way for ease of review, so we don't lose the handful of lines changed in the sea of additions. Could we skip the first patch and replace it with a set of cp instructions to follow for the baseline zstd files? Or is there another way to go about this? I'd really appreciate any advice here. > Your follow patches clean it up and IMHO go in the right direction, but > I think this needs to be done from the beginning. Thanks, Nick
On Mon, Aug 27, 2018 at 06:36:52PM -0700, Nick Terrell wrote: > Import xxhash and zstd from the upstream kernel as-is. This will not > compile. The next patch in the series contains all the changes needed to > make zstd compile in grub. Only the files needed for decompression are > imported from zstd. > > The files are copied from these locations: > include/linux/{xxhash,zstd}.h > lib/xxhash.c > lib/zstd/ First of all, may I ask you to import zstd from original repository (https://github.com/facebook/zstd) instead of Linux kernel? Please do not forget to give a commit id and name of the latest patch in it. If they have stable releases please use the latest one. Daniel
> On Sep 21, 2018, at 11:10 AM, Daniel Kiper <dkiper@net-space.pl> wrote: > > On Mon, Aug 27, 2018 at 06:36:52PM -0700, Nick Terrell wrote: >> Import xxhash and zstd from the upstream kernel as-is. This will not >> compile. The next patch in the series contains all the changes needed to >> make zstd compile in grub. Only the files needed for decompression are >> imported from zstd. >> >> The files are copied from these locations: >> include/linux/{xxhash,zstd}.h >> lib/xxhash.c >> lib/zstd/ > > First of all, may I ask you to import zstd from original repository > (https://github.com/facebook/zstd) instead of Linux kernel? Sure, I'll rework the patch set to use upstream zstd, and incorporate your comments. I'll upstream any changes we need to reduce the noise when importing, like changing "byte" to "value" because of a typedef of the same name. Thanks for the review, Nick