-
Notifications
You must be signed in to change notification settings - Fork 69
Fixes for sequential I/O issues #11
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
Open
gregs1104
wants to merge
7
commits into
benschweizer:master
Choose a base branch
from
gregs1104:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or 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
The original text only mentioned random reads.
os.lseek needs the file descriptor number of the file handle
Cleanup some obsolete function text too. This is just good general paranoia to keep results on short runs from being inflated by failed reads. There's no path out of the code yet that might count a read without doing one.
A few code assumptions didn't hold for blocksize < 512. This avoids the potential exploits of negative sizes too.
Relying on a read to return no bytes was a fragile way to do wrap-around.
Sequential read wrap-around was depending the EOF behavior of read() to
return 0 bytes. That isn't the most robust approach possible. As one
example, in cases cases where files/devices were not an exact multiple
of block_size, this was counting the leftover small reads at the end
of the file/device as a full read. That could inflate results a good
bit at the end of runs where the block size became very large.
Counting blocks also allows the seq read logic to be refactored to
start at any position. The way all the sequential read threads were
synchronized to start at the same spot is itself a problem. This
fix was needed first.
New code was tested with some temporary logging and a test long enough
here to wrap around to the start:
dd if=/dev/zero of=test.bin bs=1M count=1000
./iops -n 1 -b 512 -p sequential -t 20 test.bin
1468848531.4 count=2047997 seq to pos 2047997 out of 2048000 blocks
1468848531.4 count=2047998 seq to pos 2047998 out of 2048000 blocks
1468848531.4 count=2047999 seq to pos 2047999 out of 2048000 blocks
1468848531.4 count=2048000 seq to pos 0 out of 2048000 blocks
1468848531.4 count=2048001 seq to pos 1 out of 2048000 blocks
1468848531.4 count=2048002 seq to pos 2 out of 2048000 blocks
Here's the temporary debug lines:
if pattern=='random':
pos = random.randint(0, mediasize - blocksize) # need at least one block left
pos &= ~(sectorsize-1) # sector alignment at blocksize
fh.seek(pos)
+ else:
+ print time.time(), "count=%d seq to pos %d out of %d blocks" % (count,seq_block,blocks)
ananich
reviewed
Jan 19, 2025
|
|
||
| iops is an IO benchmark tool that performs random reads on block devices. | ||
| If an exact block size is not specified using -b, the the size starts with | ||
| iops is an IO benachmark tool that reads block devices randomly (default). |
Contributor
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.
Misprint
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.
First round of bug fixes for new issue #10 and its related documentation.
I also made a small change renaming "exact" to "single" in the parameters. I think the documentation and even the code is easier to follow like this. I isolated that to its own commit, so it's possible to revert just that if you liked the original name better.