On Sun, Nov 14, 2021 at 11:11 AM Geert Uytterhoeven
[off-list ref] wrote:

On Sat, Nov 13, 2021 at 10:12 PM Sedat Dilek [off-list ref] wrote:

quoted

On Tue, Nov 9, 2021 at 2:24 AM Nick Terrell [off-list ref] wrote:

quoted

I am sending you a pull request to add myself as the maintainer of zstd and
update the zstd version in the kernel, which is now 4 years out of date,
to the latest zstd release. This includes bug fixes, much more extensive fuzzing,
and performance improvements. And generates the kernel zstd automatically
from upstream zstd, so it is easier to keep the zstd verison up to date, and we
don't fall so far out of date again.

quoted

is it possible to have an adapted version of your work for Linux
v5.15.y which is a new kernel with LongTerm Support (see [1])?

Let's wait a bit before porting this to stable...

bloat-o-meter output for an atari_defconfig build with the old/new zstd
code (i.e. before/after commit e0c1b49f5b674cca ("lib: zstd: Upgrade to
latest upstream zstd version 1.4.10"):

vmlinux:

    add/remove: 96/28 grow/shrink: 28/29 up/down: 51766/-38162 (13604)
    CONFIG_ZSTD_DECOMPRESS=y due to CONFIG_RD_ZSTD=y (which is the default)

    Not a small increase, but acceptable, I guess?

lib/zstd/zstd_compress.ko:

    CONFIG_ZSTD_COMPRESS=m

    add/remove: 183/38 grow/shrink: 58/37 up/down: 346620/-51074 (295546)
    Function                                     old     new   delta
    ZSTD_compressBlock_btultra_dictMatchState       -   27802  +27802
    ZSTD_compressBlock_btopt_dictMatchState        -   27614  +27614
    ZSTD_compressBlock_doubleFast_dictMatchState       -   24420  +24420
    ZSTD_compressBlock_btultra_extDict             -   24376  +24376
    ZSTD_compressBlock_fast_dictMatchState         -   16712  +16712
    ZSTD_compressBlock_btultra2                    -   15432  +15432
    ZSTD_compressBlock_btopt_extDict            9052   24096  +15044
    ZSTD_initStats_ultra                           -   15040  +15040
    ZSTD_compressBlock_btultra                     -   14802  +14802
    ZSTD_compressBlock_doubleFast_extDict_generic    2432   12216   +9784
    ZSTD_compressBlock_doubleFast               8846   16342   +7496
    ZSTD_compressBlock_fast_extDict_generic     1254    8556   +7302
    ZSTD_compressBlock_btopt                    8826   15184   +6358
    ZSTD_compressBlock_fast                     3896    9532   +5636
    ZSTD_compressBlock_lazy2_extDict            6940   11578   +4638
    ZSTD_compressSuperBlock                        -    4440   +4440
    ZSTD_resetCCtx_internal                        -    3736   +3736
    ZSTD_HcFindBestMatch_dedicatedDictSearch_selectMLS.constprop
-    3706   +3706
    ...

    An increase of 288 KiB?
    My first thought was bloat-a-meter doesn't handle modules correctly.
    So I enabled CONFIG_CRYPTO_ZSTD=y, which made CONFIG_ZSTD_COMPRESS=y,
    and the impact on vmlinux is:

        add/remove: 288/0 grow/shrink: 5/0 up/down: 432712/0 (432712)

    Whoops...

    All of the top functions above just call ZSTD_compressBlock_opt_generic()
    with different parameters. Looks like the forced inlining

        FORCE_INLINE_TEMPLATE size_t
        ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms,
                                       seqStore_t* seqStore,
                                       U32 rep[ZSTD_REP_NUM],
                                 const void* src, size_t srcSize,
                                 const int optLevel,
                                 const ZSTD_dictMode_e dictMode)

    is not that suitable for the kernel...

Thanks for pointing that out! Code size wasn't something I was measuring in my
tests. I'll put up a patch to fix it.

That function is used by the highest compression level, so there
should be little
usage in the kernel. And what usage there is shouldn't be very speed sensitive.
So we should just be able to disable inlining for that file.

Longer term, we have noticed upstream that we had some code size bloat in
the compressor. We aggressively inlined to get better speed, but that tradeoff
went too far in some cases. So we're working on reducing the code size of
our largest translation units for the next release. All that to say
that we can land
a shorter term fix of disabling inlining for
lib/zstd/compress/zstd_opt.c for the
v5.16 kernel, and handle the problem thoroughly upstream in our next release.

Best,
Nick Terrell

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds