Skip to content

Add spa_get_worst_case_min_alloc() function #17758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Sep 25, 2025
Merged

Add spa_get_worst_case_min_alloc() function #17758

merged 1 commit into from
Sep 25, 2025
+52 −14

Conversation

behlendorf
Copy link
Contributor

@behlendorf behlendorf commented Sep 17, 2025 •
edited
Loading

Motivation and Context

Currently there's no mechanism for an external DMU consumer to determine the worst case minimum allocation. This can result in significant wasted capacity if a small (4k) record size is specified for certain pools layouts such as dRAID.

Description

Provide an interface to fetch the worst case minimum allocation size for a pool. This is mainly needed for dRAID pools which always pad to a full stripe width.

How Has This Been Tested?

Locally compiled.

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)

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Sep 17, 2025
amotin
amotin reviewed Sep 18, 2025
View reviewed changes
Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Providing a public interface makes sense. We currently have internal zio_roundup_alloc_size() etc, but it is a bit different. I wonder if we could use it to report some warnings for internal consumers, like setting recordsize/volblocksize too small.

About the implementation though, I wonder if it could be a part of vdev_spa_set_alloc(), similar to spa_min_alloc.

@behlendorf behlendorf marked this pull request as draft September 18, 2025 21:51
@github-actions github-actions bot added Status: Work in Progress Not yet ready for general review and removed Status: Code Review Needed Ready for review and testing labels Sep 18, 2025
@behlendorf
Copy link
Contributor Author

Good idea. I've reworked the change to add a spa_max_alloc field which is set in vdev_spa_set_alloc() in the same way as spa_min_alloc. So now we're tracking both a low and high min_alloc for the normal allocation class. It's a bit more confusing than I'd really like, but at least it's consistent with the similar min/max ashift tracking.

I also went ahead and wired up the existing volblocksize warning to use the spa_max_alloc size tracked by the pool.

I've marked this as a draft for comment since I've only thus far compiled it locally.

@behlendorf behlendorf requested a review from amotin September 23, 2025 23:17
@behlendorf behlendorf marked this pull request as ready for review September 24, 2025 17:36
@github-actions github-actions bot added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Sep 24, 2025
tonyhutter
tonyhutter reviewed Sep 24, 2025
View reviewed changes
tonyhutter
tonyhutter reviewed Sep 24, 2025
View reviewed changes
@behlendorf
Add interface to interface spa_get_worst_case_min_alloc() function
6bef327 
Provide an interface to retrieve the lowest and highest minimum
allocation size for the normal allocation class.  This can be used
by external consumers of the DMU to estimate potential wasted
capacity when setting the recordsize for an object.

The new "min_alloc" and "max_alloc" keys are added to the pool
configuration and used by default_volblocksize() to warn when
an ineffecient block size is requested.  For older kmods which
don't yet include the new keys fallback to the previous logic.

Signed-off-by: Brian D Behlendorf <[email protected]>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 25, 2025
@behlendorf behlendorf merged commit c722bf8 into openzfs:master Sep 25, 2025
21 of 25 checks passed
behlendorf added a commit to behlendorf/zfs that referenced this pull request Sep 25, 2025
@behlendorf
Add interface to interface spa_get_worst_case_min_alloc() function
954fe5e 
Provide an interface to retrieve the lowest and highest minimum
allocation size for the normal allocation class.  This can be used
by external consumers of the DMU to estimate potential wasted
capacity when setting the recordsize for an object.

The new "min_alloc" and "max_alloc" keys are added to the pool
configuration and used by default_volblocksize() to warn when
an ineffecient block size is requested.  For older kmods which
don't yet include the new keys fallback to the previous logic.

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#17758
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amotin amotin amotin approved these changes

@tonyhutter tonyhutter tonyhutter approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@behlendorf @amotin @tonyhutter