-
Notifications
You must be signed in to change notification settings - Fork 139
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
Disk I/O optimization #1765
Merged
Merged
Disk I/O optimization #1765
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sanderssj
reviewed
Aug 19, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
wjhun
approved these changes
Aug 30, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
When contiguous pages have their backing memory areas adjacent to each other (which happens frequently if the cache for a node is filled sequentially starting from offset 0), they can be merged in a single scatter-gather buffer before doing a filesystem storage operation; this minimizes the number of SG buffers being used to transfer a given amount of data, which translates into less I/O requests to the storage device (especially when the I/O ring queue depth is limited) and thus helps parallelizing I/O. This commit implements such merging during both reading and writing of page cache nodes.
When file data is being written beyond the range of an existing file extent, if the storage space adjacent to the extent is free, the extent can be extended to cover additional storage space, instead of creating a new extent; this keeps the amount of file metadata down and minimizes the number of disk I/O requests being used to transfer a given amount of data. The existing maximum limit of 1 MB on extent creation is no longer necessary and is being removed. Since allocations from the storage space are aligned to the allocation size, requesting large extents can create large unallocated ranges in the storage space; in order to be able to fill these ranges when requesting a new extent that does not fit into a single contiguous storage area, the create_extent() function upon allocation failure retries an allocation with a smaller size (down to a 1MB limit); the code that calls this function has been amended to properly handle the cases where the size of a created extent is smaller than requested.
francescolavra
force-pushed
the
feature/disk-io
branch
from
August 31, 2022 06:36
dfa33ee
to
65434ad
Compare
francescolavra
added a commit
that referenced
this pull request
Jun 2, 2023
When a request to create an extent of a certain size cannot be fulfilled due to missing free contiguous storage space, and the extent is created with a smaller size than requested, the existing code is assigning the originally requested file range (instead of the range actually allocated) to the extent being created, which leads to filesystem corruption. Regression introduced in commit 65434ad (#1765).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change set aims at minimizing the number of I/O requests and scatter-gather buffers used to transfer a given amount of data to/from disk, which helps parallelizing I/O (especially when the I/O ring queue depth is limited) and minimizes the memory allocation needs for an I/O transfer.
The first commit implements merging of contiguous pagecache pages that have their backing memory areas adjacent (which happens frequently if the cache for a node is filled sequentially starting from offset 0) into a single scatter-gather buffer before doing a filesystem storage operation (both when reading and writing a page cache node).
The second commit implements extension of existing file extents to cover additional storage space beyond the initially allocated space, and removes the 1MB maximum extent size; this helps keeping the amount of file metadata down and minimizing fragmentation of file data into non-contiguous storage areas. Since allocations from the storage space are aligned to the allocation size, requesting large extents can create large unallocated ranges in the storage space; in order to be able to fill these ranges when requesting a new extent that does not fit into a single contiguous storage area, the create_extent() function upon allocation failure retries an allocation with a smaller size (down to a 1MB limit); the code that calls this function has been amended to properly handle the cases where the size of a created extent is smaller than requested.
#1165