Skip to content
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

off-by-1 error for zfs_arc_max #7417

Merged
merged 3 commits into from
Apr 12, 2018
Merged

off-by-1 error for zfs_arc_max #7417

merged 3 commits into from
Apr 12, 2018

Conversation

beren12
Copy link
Contributor

@beren12 beren12 commented Apr 9, 2018

Description

  • 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.
  • Change has been approved by a ZFS on Linux member.

@codecov
Copy link

codecov bot commented Apr 10, 2018 •
edited
Loading

Codecov Report

Merging #7417 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7417      +/-   ##
==========================================
+ Coverage   76.31%   76.37%   +0.05%     
==========================================
  Files         330      330              
  Lines      104279   104279              
==========================================
+ Hits        79583    79642      +59     
+ Misses      24696    24637      -59
Flag Coverage Δ
#kernel 76.38% <0%> (+0.51%) ⬆️
#user 65.6% <100%> (-0.02%) ⬇️

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 3b0d992...346b804. Read the comment docs.

behlendorf
behlendorf requested changes Apr 11, 2018
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks! Yes indeed. Just a few things.

  • Please address the style warnings. You can check them locally before updating the PR by running make checkstyle in tree. You'll need to add a clear commit message description and summary, and your SoB git commit -s.

error: commit subject over 50 characters
error: missing "Signed-off-by"

  • Please also create a new development branch with this change which is rebased on the latest master. It's fine to close this PR and open a new one with the needed change.

@beren12
Copy link
Contributor Author

beren12 commented Apr 11, 2018

The 2nd commit has the signed off by, but not sure how the commit subject is over 50 chars. I'll ask on irc i guess. I miss days of just sending a .patch.

@behlendorf
Copy link
Contributor

@beren12 ahh OK, I missed that. Then nevermind, I can squash this in the merge.

@behlendorf behlendorf merged commit 7403d07 into openzfs:master Apr 12, 2018
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request May 3, 2018
@beren12 @tonyhutter
Fix zfs_arc_max minimum tuning
f0fd458 
When setting `zfs_arc_max` its minimum value is allowed
to be 64 MiB.  There was an off-by-1 error which can matter
on tiny systems.

Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Zubrzycki <github@mid-earth.net>
Closes openzfs#7417
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request May 3, 2018
@beren12 @tonyhutter
Fix zfs_arc_max minimum tuning
99a937b 
When setting `zfs_arc_max` its minimum value is allowed
to be 64 MiB.  There was an off-by-1 error which can matter
on tiny systems.

Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Zubrzycki <github@mid-earth.net>
Closes openzfs#7417
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request May 4, 2018
@beren12 @tonyhutter
Fix zfs_arc_max minimum tuning
8157492 
When setting `zfs_arc_max` its minimum value is allowed
to be 64 MiB.  There was an off-by-1 error which can matter
on tiny systems.

Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Zubrzycki <github@mid-earth.net>
Closes openzfs#7417
tonyhutter pushed a commit that referenced this pull request May 10, 2018
@beren12 @tonyhutter
Fix zfs_arc_max minimum tuning
cca220d 
When setting `zfs_arc_max` its minimum value is allowed
to be 64 MiB.  There was an off-by-1 error which can matter
on tiny systems.

Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Zubrzycki <github@mid-earth.net>
Closes #7417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dinatale2 dinatale2 dinatale2 approved these changes

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@beren12 @behlendorf @dinatale2