vol_volsize_to_reservation does not account for raidz skip blocks #8973

mgerdts · 2019-07-01T14:36:43Z

This is a port of illumos#9318. When a volume is created
in a pool with raidz vdevs and volblocksize != 128k, the volume can
reference more space than is reserved with the automatically
calculated refreservation. There are two deficiencies in
vol_volsize_to_reservation that contribute to this:

Skip blocks may be added to keep each allocation a multiple of
parity + 1. This is the dominating factor when volblocksize is close
to 2^ashift.
raidz deflation for 128 KB blocks is different for most other block
sizes.

See "The theory of raidz space accounting" comment in
libzfs_dataset.c for a full explanation.

Motivation and Context

As mentioned above, this is a port of illumos#9318 which was recently integrated into illumos-gate. I would like to integrate this into zfsonlinux to minimize divergence.

When virtual machines are created on top of zvols, they commonly use 4k or 8k volblocksize. With 4Kn disks becoming quite common, zvols on raidz end up consuming significantly more space than estimated. The common case is also the pathological case. A zvol with volblocksize=8k on raidz2 will allocate as much space as triple mirroring, but the automatically calculated refreservation will be be only a few percent larger than volsize.

The underlying issues are fully explained in a new theory comment and a recording of a Joyent Technical Discussion.

Description

zvol_volsize_to_reservation() is responsible for the calculation of the default refreservation. Originally, it assumed that the pool layout had no bearing on the amount of space that could be consumed by a volume. I've changed that so that if one or more raidz vdevs exist in the pool, the amount of reserved space will assume that all the writes to go the most inefficient vdev.

The files that his PR changes patched into zfsonlinux with the only conflicts being in copyrights.

How Has This Been Tested?

New tests were written to cover the areas of changed functionality. Results from testing on illumos are available in illumos#9318.

Small test changes were required when porting from illumos. Those include:

Use lsblock rathern than prtvtoc to figure out disk sector size
ksh on CentOS seems to have a different mixture of bugs from ksh on illumos, requiring additional checks when iterating associative array keys.
Adjust the /dev/zvol path to match the Linux way.

After those changes, scripts/zfs-tests.sh -v -d /data -T refreserv matches expectations (refreserv_004_pos fails, but that is expected). Results for running the full test suite are pending.

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)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.

This is a port of illumos/illumos-gate#9318. When a volume is created in a pool with raidz vdevs and volblocksize != 128k, the volume can reference more space than is reserved with the automatically calculated refreservation. There are two deficiencies in vol_volsize_to_reservation that contribute to this: 1) Skip blocks may be added to keep each allocation a multiple of parity + 1. This is the dominating factor when volblocksize is close to 2^ashift. 2) raidz deflation for 128 KB blocks is different for most other block sizes. See "The theory of raidz space accounting" comment in libzfs_dataset.c for a full explanation.

behlendorf · 2019-07-01T17:57:37Z

Thanks for porting this, I made a few small comments in #8974 before seeing this PR.

codecov · 2019-07-01T23:22:09Z

Codecov Report

Merging #8973 into master will decrease coverage by 0.19%.
The diff coverage is 97.36%.

@@            Coverage Diff            @@
##           master    #8973     +/-   ##
=========================================
- Coverage   78.74%   78.54%   -0.2%     
=========================================
  Files         388      388             
  Lines      119972   120003     +31     
=========================================
- Hits        94466    94255    -211     
- Misses      25506    25748    +242

Flag	Coverage Δ
#kernel	`79.32% <ø> (-0.19%)`	⬇️
#user	`66.18% <97.36%> (-0.51%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 681a85c...d6febf1. Read the comment docs.

tests/zfs-tests/tests/functional/refreserv/refreserv_raidz.ksh

Accidentally approved the wrong PR

behlendorf · 2019-07-05T22:51:28Z

@mgerdts merged, thank you for porting this!

…skip blocks When a volume is created in a pool with raidz vdevs and volblocksize != 128k, the volume can reference more space than is reserved with the automatically calculated refreservation. There are two deficiencies in vol_volsize_to_reservation that contribute to this: 1) Skip blocks may be added to keep each allocation a multiple of parity + 1. This is the dominating factor when volblocksize is close to 2^ashift. 2) raidz deflation for 128 KB blocks is different for most other block sizes. See "The theory of raidz space accounting" comment in libzfs_dataset.c for a full explanation. Authored by: Mike Gerdts <[email protected]> Reviewed by: Richard Elling <[email protected]> Reviewed by: Sanjay Nadkarni <[email protected]> Reviewed by: Jerry Jelinek <[email protected]> Reviewed by: Matt Ahrens <[email protected]> Reviewed by: Kody Kantor <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Approved by: Dan McDonald <[email protected]> Ported-by: Mike Gerdts <[email protected]> Porting Notes: * ZTS: wait for zvols to exist before writing * ZTS: use log_must_busy with {zpool|zfs} destroy OpenZFS-issue: https://www.illumos.org/issues/9318 OpenZFS-commit: illumos/illumos-gate@b73ccab0 Closes openzfs#8973

…skip blocks When a volume is created in a pool with raidz vdevs and volblocksize != 128k, the volume can reference more space than is reserved with the automatically calculated refreservation. There are two deficiencies in vol_volsize_to_reservation that contribute to this: 1) Skip blocks may be added to keep each allocation a multiple of parity + 1. This is the dominating factor when volblocksize is close to 2^ashift. 2) raidz deflation for 128 KB blocks is different for most other block sizes. See "The theory of raidz space accounting" comment in libzfs_dataset.c for a full explanation. Authored by: Mike Gerdts <[email protected]> Reviewed by: Richard Elling <[email protected]> Reviewed by: Sanjay Nadkarni <[email protected]> Reviewed by: Jerry Jelinek <[email protected]> Reviewed by: Matt Ahrens <[email protected]> Reviewed by: Kody Kantor <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Approved by: Dan McDonald <[email protected]> Ported-by: Mike Gerdts <[email protected]> Porting Notes: * ZTS: wait for zvols to exist before writing * ZTS: use log_must_busy with {zpool|zfs} destroy OpenZFS-issue: https://www.illumos.org/issues/9318 OpenZFS-commit: illumos/illumos-gate@b73ccab0 Closes #8973

mgerdts mentioned this pull request Jul 1, 2019

add zfs create dryrun #8974

Merged

12 tasks

behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 1, 2019

address feedback from behlendorf in pull/8974

be34cf7

behlendorf reviewed Jul 2, 2019

View reviewed changes

tests/zfs-tests/tests/functional/refreserv/refreserv_raidz.ksh Show resolved Hide resolved

behlendorf requested review from ahrens and jwk404 July 2, 2019 18:48

wait for zvols to exist before writing

611d0b1

behlendorf previously approved these changes Jul 3, 2019

View reviewed changes

use log_must_busy with {zpool|zfs} destroy

d6febf1

ahrens approved these changes Jul 5, 2019

View reviewed changes

behlendorf approved these changes Jul 5, 2019

View reviewed changes

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jul 5, 2019

behlendorf closed this in 341166c Jul 5, 2019

behlendorf mentioned this pull request Jan 21, 2022

IO error with overwriting when ZVOL space is full #11455

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vol_volsize_to_reservation does not account for raidz skip blocks #8973

vol_volsize_to_reservation does not account for raidz skip blocks #8973

mgerdts commented Jul 1, 2019

behlendorf commented Jul 1, 2019

codecov bot commented Jul 1, 2019 •

edited

Loading

behlendorf commented Jul 5, 2019

vol_volsize_to_reservation does not account for raidz skip blocks #8973

vol_volsize_to_reservation does not account for raidz skip blocks #8973

Conversation

mgerdts commented Jul 1, 2019

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

behlendorf commented Jul 1, 2019

codecov bot commented Jul 1, 2019 • edited Loading

Codecov Report

behlendorf commented Jul 5, 2019

codecov bot commented Jul 1, 2019 •

edited

Loading