New issue
Advanced search Search tips

Issue 892777 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

termina: disable raid6 benchmarking on boot

Project Member Reported by smbar...@chromium.org, Oct 5

Issue description

The raid6 subsystem is using a substantial amount of boot time to run benchmarks and select the best hardware acceleration mechanisms.

This takes about 25-30% of the time to boot the guest kernel. On eve, maitred starts in about 600ms, 200ms of which is raid6 benchmarking. On kevin, maitred starts in about 1000ms, 260ms of which is raid6 benchmarking.


We don't use raid6 for btrfs, so we should disable this to improve our boot speed.
 
Labels: -Pri-3 M-72 Pri-2
Owner: dverkamp@chromium.org
Status: Assigned (was: Untriaged)
It looks like the raid6 benchmarking is controlled by the top-level CONFIG_RAID6_PQ, which is pulled in by CONFIG_BTRFS_FS.  It may not be possible to disable this without applying a kernel patch (not just a config change).  The responsible source code in the kernel is in lib/raid6/algos.c, which has raid6_select_algo as an initcall.
I wonder if upstream would be willing to take a patch that made the dependency optional.
I think RAID6_PQ has a lot of other stuff that btrfs needs.

Upstream would probably be receptive to a config specifically to disable this benchmarking though.
This looks roughly like what we would want: https://github.com/clearlinux-pkgs/linux-hyperv/blob/master/0116-raid6-add-Kconfig-option-to-skip-raid6-benchmarking.patch

However, I don't know if it has been posted upstream anywhere.  We'd also probably want to extend it to allow picking the NEON variants on ARM (rather than the fallback C code) in case we ever actually want to use raid6.
Did it get posted to any of the upstream mailing lists? It does look rather useful.
I couldn't find the linked patch on any upstream mailing lists, and it seemed somewhat annoying to have to choose a particular instruction set at compile time, so I put together a different patch set to do something slightly different:

https://marc.info/?l=linux-raid&m=154206522104769&w=2
https://marc.info/?l=linux-raid&m=154206522304770&w=2

We'll see what upstream thinks - if it looks OK, we can cherry-pick these back into our 4.19 branch.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 29

Labels: merge-merged-chromeos-4.19
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/72d5afdf13fe5e05d7185a2ac223cbbd56ee7f8f

commit 72d5afdf13fe5e05d7185a2ac223cbbd56ee7f8f
Author: Daniel Verkamp <dverkamp@chromium.org>
Date: Thu Nov 29 00:32:38 2018

FROMLIST: lib/raid6: sort algos in rough performance order

Sort the list of RAID6 algorithms in roughly decreasing order of
expected performance: newer instruction sets first (within each
architecture) and wider unrollings first.

This doesn't make any difference right now, since all functions are
benchmarked; a follow-up change will make use of this by optionally
choosing the first valid function rather than testing all of them.

The Itanium raid6_intx{16,32} entries are also moved down to be near the
other raid6_intx entries for clarity.

Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>

(from https://marc.info/?l=linux-raid&m=154206522104769&w=2)

BUG= chromium:892777 
TEST=Boot Termina on nami and kevin

Change-Id: Ie464f6dfff0e5e374a9b28136e2e8bb743a54eec
Reviewed-on: https://chromium-review.googlesource.com/1343218
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/72d5afdf13fe5e05d7185a2ac223cbbd56ee7f8f/lib/raid6/algos.c

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/10093edd34018dc30ca82df4d0d0ebe831248587

commit 10093edd34018dc30ca82df4d0d0ebe831248587
Author: Daniel Verkamp <dverkamp@chromium.org>
Date: Thu Nov 29 00:32:39 2018

FROMLIST: lib/raid6: add option to skip algo benchmarking

This is helpful for systems where fast startup time is important.
It is especially nice to avoid benchmarking RAID functions that are
never used (for example, BTRFS selects RAID6_PQ even if the parity RAID
mode is not in use).

This saves 250+ milliseconds of boot time on modern x86 and ARM systems
with a dozen or more available implementations.

The new option is defaulted to 'y' to match the previous behavior of
always benchmarking on init.

Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>

(from https://marc.info/?l=linux-raid&m=154206522304770&w=2)

BUG= chromium:892777 
TEST=Boot Termina on nami and kevin

Change-Id: I14aebcc7d1ee70ae7bab2d979447b2b59ba85538
Reviewed-on: https://chromium-review.googlesource.com/1343219
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/10093edd34018dc30ca82df4d0d0ebe831248587/lib/Kconfig
[modify] https://crrev.com/10093edd34018dc30ca82df4d0d0ebe831248587/include/linux/raid/pq.h
[modify] https://crrev.com/10093edd34018dc30ca82df4d0d0ebe831248587/lib/raid6/algos.c

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7bc370e1a483b2b7abc659cd5f032d78bd0d4b15

commit 7bc370e1a483b2b7abc659cd5f032d78bd0d4b15
Author: Daniel Verkamp <dverkamp@chromium.org>
Date: Thu Nov 29 00:32:41 2018

CHROMIUM: configs: skip raid6 benchmark in VM kernel

This eliminates the boot time required for raid6 benchmarking (which is
pulled in by BTRFS and can't be disabled).

BUG= chromium:892777 
TEST=Boot Termina on nami and kevin; verify that boot time is reduced

Change-Id: I38734646c187a40281e2a91043c0a3f186488125
Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1343220
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/7bc370e1a483b2b7abc659cd5f032d78bd0d4b15/arch/arm64/configs/chromiumos-container-vm-arm64_defconfig
[modify] https://crrev.com/7bc370e1a483b2b7abc659cd5f032d78bd0d4b15/arch/x86/configs/chromiumos-container-vm-x86_64_defconfig

Status: Fixed (was: Assigned)

Sign in to add a comment