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

zvol_write() can use dmu_tx_hold_write_by_dnode() #10184

Merged
merged 1 commit into from
Apr 11, 2020

Conversation

ahrens
Copy link
Member

@ahrens ahrens commented Apr 6, 2020

Motivation and Context

We can improve the performance of writes to zvols by using
dmu_tx_hold_write_by_dnode() instead of dmu_tx_hold_write(). This
reduces lock contention on the first block of the dnode object, and also
reduces the amount of CPU needed. The benefit will be highest with
multi-threaded async writes (i.e. writes that don't call zil_commit()).

See https://github.com/openzfsonwindows/ZFSin/issues/238

Description

Make zvol_write() call dmu_tx_hold_write_by_dnode() instead of dmu_tx_hold_write().

How Has This Been Tested?

Minimal testing so far. Would like to measure performance improvement with async writes.

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.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

We can improve the performance of writes to zvols by using
dmu_tx_hold_write_by_dnode() instead of dmu_tx_hold_write().  This
reduces lock contention on the first block of the dnode object, and also
reduces the amount of CPU needed.  The benefit will be highest with
multi-threaded async writes (i.e. writes that don't call zil_commit()).

Signed-off-by: Matthew Ahrens <mahrens@delphix.com>
@ahrens ahrens added Component: ZVOL ZFS Volumes Type: Performance Performance improvement or performance problem Status: Code Review Needed Ready for review and testing labels Apr 6, 2020
@ahrens
Copy link
Member Author

ahrens commented Apr 6, 2020

cc @tonynguien @lundman

@behlendorf behlendorf requested a review from lundman April 9, 2020 23:30
Copy link
Contributor

@lundman lundman left a comment

Choose a reason for hiding this comment

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

I have tested this over at openzfsonwindows/ZFSin#240 where it does appear to make a difference.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 10, 2020
@behlendorf behlendorf merged commit 20f2878 into openzfs:master Apr 11, 2020
@matveevandrey matveevandrey mentioned this pull request Apr 16, 2020
12 tasks
tle211212 added a commit to tle211212/zfs that referenced this pull request May 19, 2020
@georgeyil georgeyil mentioned this pull request Nov 11, 2020
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
We can improve the performance of writes to zvols by using
dmu_tx_hold_write_by_dnode() instead of dmu_tx_hold_write().  This
reduces lock contention on the first block of the dnode object, and also
reduces the amount of CPU needed.  The benefit will be highest with
multi-threaded async writes (i.e. writes that don't call zil_commit()).

Reviewed-by: Jorgen Lundman <lundman@lundman.net>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tony Nguyen <tony.nguyen@delphix.com>
Signed-off-by: Matthew Ahrens <mahrens@delphix.com>
Closes openzfs#10184
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: ZVOL ZFS Volumes Status: Accepted Ready to integrate (reviewed, tested) Type: Performance Performance improvement or performance problem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants