Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1676385

Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit

Show all headers | View raw

> Please don't top post.

Sorry about that.

> Which function needs 1KB of stack space? That's quite a lot.

FSE_buildCTable_wksp(), FSE_compress_wksp(), and HUF_readDTableX4()
required over 1 KB of stack space.

> I can see in [1] that there are some on-stack buffers replaced by
> pointers to the workspace. That's good, but I would like to know if
> there's any hidden gem that grags the precious stack space.

I've been hunting down functions that use up the most stack trace and
replacing buffers with pointers to the workspace. I compiled the code
with -Wframe-larger-than=512 and reduced the stack usage of all offending
functions. In the next version of the patch, no function uses more than
400 B of stack space. We'll be porting the changes back upstream as well.

> Hm, I'd suggest to create a version optimized for kernel, eg. expecting
> that 4+ GB buffer will never be used and you can use the most fittin in
> type. This should affect only the function signatures, not the
> algorithm implementation, so porting future zstd changes should be
> straightforward.

If the functions were exposed, then I would agree 100%. However, since
these are internal functions, and the rest of zstd uses size_t to represent
buffer sizes, I think it would be awkward to change just FSE/HUF functions.
I also prefer size_t because it is friendlier to the optimizer, especially
the loop optimizer, since the compiler doesn't have to worry about unsigned
overflow.

On a related note, zstd performs automatic optimizations to improve
compression speed and reduce memory usage when given small sources, which
is the common case in the kernel.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 3/4] btrfs: Add zstd support Nick Terrell <terrelln@fb.com> - 2017-06-23 00:10 +0200
  Re: [PATCH 3/4] btrfs: Add zstd support Adam Borowski <kilobyte@angband.pl> - 2017-06-25 23:40 +0200
    Re: [PATCH 3/4] btrfs: Add zstd support David Sterba <dsterba@suse.cz> - 2017-06-26 14:30 +0200
      Re: [PATCH 3/4] btrfs: Add zstd support Nick Terrell <terrelln@fb.com> - 2017-06-26 19:00 +0200
      [PATCH] lib/zstd: use div_u64() to let it build on 32-bit Adam Borowski <kilobyte@angband.pl> - 2017-06-27 06:30 +0200
        Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit Nick Terrell <terrelln@fb.com> - 2017-06-27 07:30 +0200
          Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit David Sterba <dsterba@suse.cz> - 2017-06-27 15:00 +0200
            Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit Nick Terrell <terrelln@fb.com> - 2017-06-28 07:40 +0200
          Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit Adam Borowski <kilobyte@angband.pl> - 2017-06-29 03:10 +0200
            [PATCH] btrfs: Keep one more workspace around Nick Terrell <terrelln@fb.com> - 2017-06-29 05:10 +0200
              Re: [PATCH] btrfs: Keep one more workspace around David Sterba <dsterba@suse.cz> - 2017-06-29 16:10 +0200

csiph-web