Skip to content

ARC: Avoid overflows in arc_evict_adj() #17255

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
May 6, 2025
Merged

ARC: Avoid overflows in arc_evict_adj() #17255

merged 1 commit into from
May 6, 2025

Conversation

amotin
Copy link
Member

@amotin amotin commented Apr 18, 2025

With certain combinations of target ARC states balance and ghost hit rates it was possible to get the fractions outside of allowed range. This patch limits maximum balance adjustment speed, which should make it impossible, and also asserts it.

Fixes #17210

How Has This Been Tested?

It seems pretty hard to reproduce, but if ever happen again asserts should let us know.

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)
  • 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 Apr 18, 2025
@shodanshok shodanshok mentioned this pull request Apr 18, 2025
Merged
13 tasks
adamdmoss
adamdmoss reviewed Apr 21, 2025
View reviewed changes
ceedriic
ceedriic reviewed Apr 22, 2025
View reviewed changes
@amotin
ARC: Avoid overflows in arc_evict_adj()
575510c 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes openzfs#17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
@amotin
Copy link
Member Author

amotin commented Apr 23, 2025

Just a rebase.

robn
robn approved these changes May 5, 2025
View reviewed changes
Copy link
Member

@robn robn left a comment

Choose a reason for hiding this comment

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

Looks right. I reckon we could benefit from some macros to make it look a bit more like proper math, but eh, some other day!

@tonyhutter tonyhutter merged commit b1ccab1 into openzfs:master May 6, 2025
22 of 24 checks passed
ixhamza pushed a commit to truenas/zfs that referenced this pull request May 14, 2025
@amotin @ixhamza
ARC: Avoid overflows in arc_evict_adj() (openzfs#17255)
dd9b1b5 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes openzfs#17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
ixhamza pushed a commit to truenas/zfs that referenced this pull request May 14, 2025
@amotin @ixhamza
ARC: Avoid overflows in arc_evict_adj() (openzfs#17255)
0c65859 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes openzfs#17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
@shodanshok shodanshok mentioned this pull request May 20, 2025
Closed
13 tasks
robn pushed a commit to robn/zfs that referenced this pull request May 23, 2025
@amotin @robn
ARC: Avoid overflows in arc_evict_adj() (openzfs#17255)
81f2feb 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes openzfs#17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
(cherry picked from commit b1ccab1)
(cherry picked from commit 677cb91)
robn pushed a commit to robn/zfs that referenced this pull request May 23, 2025
@amotin @robn
ARC: Avoid overflows in arc_evict_adj() (openzfs#17255)
a6865d7 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes openzfs#17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
(cherry picked from commit b1ccab1)
(cherry picked from commit 677cb91ffee6d54501c96323c6308a024cb62268)
robn pushed a commit to robn/zfs that referenced this pull request May 23, 2025
@amotin @robn
ARC: Avoid overflows in arc_evict_adj() (openzfs#17255)
677cb91 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes openzfs#17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
(cherry picked from commit b1ccab1)
@robn robn mentioned this pull request May 23, 2025
Closed
14 tasks
robn pushed a commit to robn/zfs that referenced this pull request May 24, 2025
@amotin @robn
ARC: Avoid overflows in arc_evict_adj() (openzfs#17255)
ad65662 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes openzfs#17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
(cherry picked from commit b1ccab1)
tonyhutter pushed a commit that referenced this pull request May 27, 2025
@amotin @tonyhutter
ARC: Avoid overflows in arc_evict_adj() (#17255)
be72364 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes #17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
(cherry picked from commit b1ccab1)
(cherry picked from commit 677cb91)
tonyhutter pushed a commit that referenced this pull request May 28, 2025
@amotin @tonyhutter
ARC: Avoid overflows in arc_evict_adj() (#17255)
9b446fb 
With certain combinations of target ARC states balance and ghost
hit rates it was possible to get the fractions outside of allowed
range.  This patch limits maximum balance adjustment speed, which
should make it impossible, and also asserts it.

Fixes #17210
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
(cherry picked from commit b1ccab1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamdmoss adamdmoss adamdmoss left review comments

@ceedriic ceedriic ceedriic left review comments

@robn robn robn approved these changes

@tonyhutter tonyhutter tonyhutter approved these changes

Assignees
No one assigned
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

arc_summary shows negative value for MFU data target
5 participants
@amotin @robn @adamdmoss @ceedriic @tonyhutter