Skip to content

Conversation

mcmilk
Copy link
Contributor

@mcmilk mcmilk commented Jul 22, 2025

While booting, only the needed 256KiB benchmarks are done now.

The long delay for testing all checksums is done on request for them now:

Linux: cat /proc/spl/kstat/zfs/chksum_bench
FreeBSD: sysctl kstat.zfs.misc.chksum_bench

Closes #17560

Motivation and Context

The issue #17560 is the main reason for this PR.

Description

How Has This Been Tested?

I loaded the module :-)

Old load module time: 1,5s (Github runner)
New load module time: 0,25s (Github runner)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@amotin amotin added the Status: Code Review Needed Ready for review and testing label Jul 23, 2025
@mcmilk
Copy link
Contributor Author

mcmilk commented Jul 23, 2025

Sorry for this extra rebasing - had trouble with my handy :/

While booting, only the needed 256KiB benchmarks are done now.

The delay for checking all checksums occurs when requested via:
- Linux: cat /proc/spl/kstat/zfs/chksum_bench
- FreeBSD: sysctl kstat.zfs.misc.chksum_bench

Reported by: Lahiru Gunathilake <[email protected]>
Co-authored-by: Colin Percival <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Closes openzfs#17560
@mcmilk
Copy link
Contributor Author

mcmilk commented Jul 24, 2025

I reworked the commit message to give credits for the reporting:
Reported by: Lahiru Gunathilake [email protected]
Co-authored-by: Colin Percival [email protected]

Thanks @cperciva and @bllgg

@lundman
Copy link
Contributor

lundman commented Jul 24, 2025

🤔 Benchmarking is only used with fastest - if the tunable is set to something static, skip benchmarking entirely? 🤔

@mcmilk
Copy link
Contributor Author

mcmilk commented Jul 24, 2025

🤔 Benchmarking is only used with fastest - if the tunable is set to something static, skip benchmarking entirely? 🤔

It's not skipped. Only the 256KB blocksize is checked now.
If some admin wants to know other sizes, then he cat check it via: cat /proc/spl/kstat/zfs/chksum_bench ... this will then calculated when requested. So the first cat delays with ~one seconds now.

Edit: ah, I understand what you mean now :/ Sorry... it's early in the morning here ;-)
So when the user sets some implementation by hand, then the whole check could be skipped. -> hm... yes, this would be possible, but it's not done for fletcher and aes this way. So the documentation would be different then also... I think some micro benchmarking is okay in the beginning. But we could change it ... but I would prefer for all algos then.

@lundman
Copy link
Contributor

lundman commented Jul 24, 2025

Just a thought.. I can set values before ZFS loads on Windows, but not on macOS - so it'd only work on one platform for me. I don't know when Linux/FreeBSD would load the tunables.

@cperciva
Copy link
Contributor

Just a thought.. I can set values before ZFS loads on Windows, but not on macOS - so it'd only work on one platform for me. I don't know when Linux/FreeBSD would load the tunables.

Assuming that "tunable" here means the same thing as it normally does in the FreeBSD kernel: We process those long before the ZFS module initialization runs. Loader tunables can be used to control lots of ZFS behaviours.

@amotin amotin added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jul 25, 2025
@behlendorf behlendorf merged commit 10a78e2 into openzfs:master Jul 30, 2025
62 of 73 checks passed
amotin pushed a commit that referenced this pull request Aug 5, 2025
While booting, only the needed 256KiB benchmarks are done now.

The delay for checking all checksums occurs when requested via:
- Linux: cat /proc/spl/kstat/zfs/chksum_bench
- FreeBSD: sysctl kstat.zfs.misc.chksum_bench

Reported by: Lahiru Gunathilake <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Co-authored-by: Colin Percival <[email protected]>
Closes #17563
Closes #17560
ixhamza pushed a commit to truenas/zfs that referenced this pull request Aug 28, 2025
While booting, only the needed 256KiB benchmarks are done now.

The delay for checking all checksums occurs when requested via:
- Linux: cat /proc/spl/kstat/zfs/chksum_bench
- FreeBSD: sysctl kstat.zfs.misc.chksum_bench

Reported by: Lahiru Gunathilake <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Co-authored-by: Colin Percival <[email protected]>
Closes openzfs#17563
Closes openzfs#17560
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
While booting, only the needed 256KiB benchmarks are done now.

The delay for checking all checksums occurs when requested via:
- Linux: cat /proc/spl/kstat/zfs/chksum_bench
- FreeBSD: sysctl kstat.zfs.misc.chksum_bench

Reported by: Lahiru Gunathilake <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Co-authored-by: Colin Percival <[email protected]>
Closes openzfs#17563
Closes openzfs#17560
bugclerk pushed a commit to truenas/zfs that referenced this pull request Sep 8, 2025
While booting, only the needed 256KiB benchmarks are done now.

The delay for checking all checksums occurs when requested via:
- Linux: cat /proc/spl/kstat/zfs/chksum_bench
- FreeBSD: sysctl kstat.zfs.misc.chksum_bench

Reported by: Lahiru Gunathilake <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Co-authored-by: Colin Percival <[email protected]>
Closes openzfs#17563
Closes openzfs#17560
(cherry picked from commit 62169c9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delay in Boot Due to Checksum Benchmarking: Suggest Deferring to Idle Time
5 participants