All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Terrell <nickrterrell@gmail.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Sedat Dilek" <sedat.dilek@gmail.com>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Stephen Rothwell" <sfr@canb.auug.org.au>,
	"Linux Crypto Mailing List" <linux-crypto@vger.kernel.org>,
	linux-btrfs <linux-btrfs@vger.kernel.org>,
	squashfs-devel@lists.sourceforge.net,
	linux-f2fs-devel@lists.sourceforge.net,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Kernel Team" <Kernel-team@fb.com>,
	"Nick Terrell" <terrelln@fb.com>, "Chris Mason" <clm@fb.com>,
	"Petr Malat" <oss@malat.biz>, "Yann Collet" <cyan@fb.com>,
	"Christoph Hellwig" <hch@infradead.org>,
	"Michał Mirosław" <mirq-linux@rere.qmqm.pl>,
	"David Sterba" <dsterba@suse.cz>,
	"Oleksandr Natalenko" <oleksandr@natalenko.name>,
	"Felix Handte" <felixh@fb.com>,
	"Eric Biggers" <ebiggers@kernel.org>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"Paul Jones" <paul@pauljones.id.au>,
	"Tom Seewald" <tseewald@gmail.com>,
	"Jean-Denis Girard" <jd.girard@sysnux.pf>
Subject: Re: [GIT PULL] zstd changes for v5.16
Date: Sun, 14 Nov 2021 12:33:41 -0800	[thread overview]
Message-ID: <CANr2DbfSu9RWv+c8jzj=6r0cRC-R0f_Z2v3gSkJm2dPR8MJi4A@mail.gmail.com> (raw)
In-Reply-To: <CAMuHMdWqBAwVnfuwmonT1hESB4P+EH0p0dtszdrZLJGxBbU2gw@mail.gmail.com>

On Sun, Nov 14, 2021 at 11:11 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> On Sat, Nov 13, 2021 at 10:12 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > On Tue, Nov 9, 2021 at 2:24 AM Nick Terrell <nickrterrell@gmail.com> wrote:
> > > 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.
>
> > 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

WARNING: multiple messages have this Message-ID (diff)
From: Nick Terrell <nickrterrell@gmail.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Oleksandr Natalenko" <oleksandr@natalenko.name>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Tom Seewald" <tseewald@gmail.com>, "Chris Mason" <clm@fb.com>,
	"Jean-Denis Girard" <jd.girard@sysnux.pf>,
	"Stephen Rothwell" <sfr@canb.auug.org.au>,
	"Paul Jones" <paul@pauljones.id.au>, "Yann Collet" <cyan@fb.com>,
	"Christoph Hellwig" <hch@infradead.org>,
	"Kernel Team" <Kernel-team@fb.com>,
	"Eric Biggers" <ebiggers@kernel.org>,
	squashfs-devel@lists.sourceforge.net,
	"Michał Mirosław" <mirq-linux@rere.qmqm.pl>,
	"Nick Terrell" <terrelln@fb.com>,
	"Sedat Dilek" <sedat.dilek@gmail.com>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"David Sterba" <dsterba@suse.cz>,
	linux-f2fs-devel@lists.sourceforge.net,
	"Petr Malat" <oss@malat.biz>,
	"Linux Crypto Mailing List" <linux-crypto@vger.kernel.org>,
	"Felix Handte" <felixh@fb.com>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	linux-btrfs <linux-btrfs@vger.kernel.org>
Subject: Re: [f2fs-dev] [GIT PULL] zstd changes for v5.16
Date: Sun, 14 Nov 2021 12:33:41 -0800	[thread overview]
Message-ID: <CANr2DbfSu9RWv+c8jzj=6r0cRC-R0f_Z2v3gSkJm2dPR8MJi4A@mail.gmail.com> (raw)
In-Reply-To: <CAMuHMdWqBAwVnfuwmonT1hESB4P+EH0p0dtszdrZLJGxBbU2gw@mail.gmail.com>

On Sun, Nov 14, 2021 at 11:11 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> On Sat, Nov 13, 2021 at 10:12 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > On Tue, Nov 9, 2021 at 2:24 AM Nick Terrell <nickrterrell@gmail.com> wrote:
> > > 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.
>
> > 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


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2021-11-14 20:34 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-09  1:30 [f2fs-dev] [GIT PULL] zstd changes for v5.16 Nick Terrell
2021-11-09  1:30 ` Nick Terrell
2021-11-10 18:46 ` [f2fs-dev] " Nick Terrell via Linux-f2fs-devel
2021-11-10 18:46   ` Nick Terrell
2021-11-11 22:59   ` Linus Torvalds
2021-11-11 22:59     ` [f2fs-dev] " Linus Torvalds
2021-11-12  0:08     ` Nick Terrell via Linux-f2fs-devel
2021-11-12  0:08       ` Nick Terrell
2021-11-11 20:11 ` [f2fs-dev] " Nick Terrell via Linux-f2fs-devel
2021-11-11 20:11   ` Nick Terrell
2021-11-13  5:41   ` Herbert Xu
2021-11-13  5:41     ` [f2fs-dev] " Herbert Xu
2021-11-13 21:07 ` Sedat Dilek
2021-11-13 21:07   ` [f2fs-dev] " Sedat Dilek
2021-11-14 19:11   ` Geert Uytterhoeven
2021-11-14 19:11     ` [f2fs-dev] " Geert Uytterhoeven
2021-11-14 20:33     ` Nick Terrell [this message]
2021-11-14 20:33       ` Nick Terrell
2021-11-14  0:43 ` pr-tracker-bot
2021-11-14  0:43   ` [f2fs-dev] " pr-tracker-bot
2022-02-20  9:45 ` Sedat Dilek
2022-02-20  9:45   ` [f2fs-dev] " Sedat Dilek
2022-03-01 19:09   ` Nick Terrell
2022-03-01 19:09     ` [f2fs-dev] " Nick Terrell via Linux-f2fs-devel
2022-03-02 22:31     ` Sedat Dilek
2022-03-02 22:31       ` [f2fs-dev] " Sedat Dilek
2022-04-03 11:13       ` Sedat Dilek
2022-04-03 11:13         ` [f2fs-dev] " Sedat Dilek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANr2DbfSu9RWv+c8jzj=6r0cRC-R0f_Z2v3gSkJm2dPR8MJi4A@mail.gmail.com' \
    --to=nickrterrell@gmail.com \
    --cc=Kernel-team@fb.com \
    --cc=clm@fb.com \
    --cc=cyan@fb.com \
    --cc=dsterba@suse.cz \
    --cc=ebiggers@kernel.org \
    --cc=felixh@fb.com \
    --cc=geert@linux-m68k.org \
    --cc=hch@infradead.org \
    --cc=jd.girard@sysnux.pf \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mirq-linux@rere.qmqm.pl \
    --cc=oleksandr@natalenko.name \
    --cc=oss@malat.biz \
    --cc=paul@pauljones.id.au \
    --cc=rdunlap@infradead.org \
    --cc=sedat.dilek@gmail.com \
    --cc=sfr@canb.auug.org.au \
    --cc=squashfs-devel@lists.sourceforge.net \
    --cc=terrelln@fb.com \
    --cc=torvalds@linux-foundation.org \
    --cc=tseewald@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.