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.

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

off-by-1 error for zfs_arc_max #7417

Merged
merged 3 commits into from Apr 12, 2018

Conversation

Projects
3 participants
@beren12
Copy link
Contributor

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.

Chris added some commits Apr 9, 2018

Chris
off-by-1 error. Matters on tiny systems ;-)
Signed-off-by: Chris Zubrzycki <github@mid-earth.net>
@codecov

This comment has been minimized.

Copy link

commented Apr 10, 2018

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
Copy link
Member

left a comment

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Member

commented Apr 12, 2018

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

@behlendorf behlendorf merged commit 7403d07 into zfsonlinux:master Apr 12, 2018

23 of 26 checks passed

buildbot/Kernel.org Built-in x86_64 (BUILD) Build done.
Details
buildbot/Ubuntu 16.04 x86_64 (TEST) Build done.
Details
buildbot/Ubuntu 17.10 x86_64 (STYLE) Build done.
Details
buildbot/Amazon 2 x86_64 (BUILD) Build done.
Details
buildbot/Amazon 2 x86_64 Release (TEST) Build done.
Details
buildbot/CentOS 6 x86_64 (BUILD) Build done.
Details
buildbot/CentOS 6 x86_64 (TEST) Build done.
Details
buildbot/CentOS 7 x86_64 (BUILD) Build done.
Details
buildbot/CentOS 7 x86_64 (TEST) Build done.
Details
buildbot/CentOS 7 x86_64 Mainline (TEST) Build done.
Details
buildbot/CentOS 7 x86_64 Release (BUILD) Build done.
Details
buildbot/Debian 8 arm (BUILD) Build done.
Details
buildbot/Debian 8 ppc (BUILD) Build done.
Details
buildbot/Debian 8 ppc64 (BUILD) Build done.
Details
buildbot/Debian 8 x86_64 (BUILD) Build done.
Details
buildbot/Fedora 27 x86_64 (BUILD) Build done.
Details
buildbot/Fedora 27 x86_64 (TEST) Build done.
Details
buildbot/Fedora Rawhide x86_64 (TEST) Build done.
Details
buildbot/Ubuntu 14.04 x86_64 (BUILD) Build done.
Details
buildbot/Ubuntu 16.04 aarch64 (BUILD) Build done.
Details
buildbot/Ubuntu 16.04 i386 (BUILD) Build done.
Details
buildbot/Ubuntu 16.04 x86_64 (BUILD) Build done.
Details
buildbot/Ubuntu 17.04 x86_64 Coverage (TEST) Build done.
Details
buildbot/Ubuntu 17.10 x86_64 (TEST) Build done.
Details
codecov/patch 100% of diff hit (target 76.31%)
Details
codecov/project 76.37% (+0.05%) compared to 3b0d992
Details

@tonyhutter tonyhutter added this to TODO in 0.7.9 Apr 30, 2018

@tonyhutter tonyhutter moved this from TODO to In progress in 0.7.9 May 2, 2018

tonyhutter added a commit to tonyhutter/zfs that referenced this pull request May 3, 2018

Fix zfs_arc_max minimum tuning
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 zfsonlinux#7417

tonyhutter added a commit to tonyhutter/zfs that referenced this pull request May 3, 2018

Fix zfs_arc_max minimum tuning
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 zfsonlinux#7417

tonyhutter added a commit to tonyhutter/zfs that referenced this pull request May 4, 2018

Fix zfs_arc_max minimum tuning
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 zfsonlinux#7417

tonyhutter added a commit that referenced this pull request May 10, 2018

Fix zfs_arc_max minimum tuning
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
You can’t perform that action at this time.