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

Enforce request limits on zvols #6294

Closed
wants to merge 1 commit into from
Closed

Enforce request limits on zvols #6294

wants to merge 1 commit into from

Conversation

dinatale2
Copy link
Contributor

Description

Currently, zvols do not handle heavy random IO
workloads. zvols should limit the number of outstanding
in-flight IO requests. This should improve performance.

Motivation and Context

#6127
#6278

How Has This Been Tested?

Builds on my VM. Buildbot will help me test. Hoping to test on hardware soon.

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

@mention-bot
Copy link

@dinatale2, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @bprotopopov and @tuxoko to be potential reviewers.

@dinatale2 dinatale2 mentioned this pull request Jul 3, 2017
Closed
@behlendorf behlendorf requested a review from tuxoko July 3, 2017 21:52
@dinatale2 dinatale2 force-pushed the issue-6127 branch 3 times, most recently from 577cde4 to 5c3b6cb Compare July 7, 2017 03:35
@dinatale2
Copy link
Contributor Author

I will refactor this patch soon. I've realized that I've not covered all exit cases (if a write results in a zvol_discard, the write counter won't be decremented for example).

@dinatale2 dinatale2 force-pushed the issue-6127 branch 2 times, most recently from b6c112a to f61d640 Compare July 10, 2017 14:58
@dinatale2 dinatale2 force-pushed the issue-6127 branch 2 times, most recently from c44b039 to 6b88a45 Compare August 10, 2017 18:32
tuxoko
tuxoko approved these changes Aug 22, 2017
View reviewed changes
Copy link
Contributor

@tuxoko tuxoko left a comment

Choose a reason for hiding this comment

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

@dinatale2 @behlendorf
Sorry, I forgot about this one, can you rebase to master and let the buildbot run again, otherwise LGTM.

@dinatale2
Enforce request limits on zvols
f023cc3 
ZVOLs do not handle heavy random IO works loads. ZVOLs
should limit the number of outstanding in-flight IO requests.
This should improve performance.

Signed-off-by: Giuseppe Di Natale <[email protected]>
behlendorf
behlendorf approved these changes Aug 28, 2017
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.

This looks good, pending the results of some performance testing.

This was referenced Nov 20, 2017
Merged
Closed
@behlendorf
Copy link
Contributor

@dinatale2 can you please rebase this PR.

@dinatale2
Copy link
Contributor Author

@behlendorf I am going to close this PR for now. I currently don't have the cycles to revisit it and due to recent changes around or related to the ARC, I think we can wait and see if ZVOL performance is still a problem.

@dinatale2 dinatale2 closed this Feb 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@tuxoko tuxoko tuxoko 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.
4 participants
@dinatale2 @mention-bot @behlendorf @tuxoko