backend: initialize comp_time before the rate checks read it - #2130
Open
Vladyyy wants to merge 1 commit into
Open
backend: initialize comp_time before the rate checks read it#2130Vladyyy wants to merge 1 commit into
Vladyyy wants to merge 1 commit into
Conversation
do_io() declared comp_time uninitialized inside the IO loop. It is only stamped by wait_for_completions() (called when the queue is full or when polling) and by io_queue_event()'s inline-completion path. An async engine on a block device with a rate limit takes neither: submissions return FIO_Q_QUEUED and completions are reaped during the rate-throttle sleep via io_u_quiesce(), which does not touch comp_time. Once bytes_done is non-zero, check_min_rate() (enabled by rate_iops_min / ratemin) computes mtime_since(&td->start, &comp_time) from whatever bytes happen to be on the stack, and when the garbage tv_nsec is out of range rel_time_since() aborts: fio: gettime.c:530: rel_time_since: Assertion `0 <= nsec && nsec < 1000ULL * 1000 * 1000' failed. This crashed every run of a rate-limited randwrite latency workload (libaio, iodepth=16, rate_iops=500, rate_iops_min=500, write_lat_log) ~100 ms after start on our EC2 test fleet, deterministically per host: the stack leftovers at that slot depend on the preceding code paths, so the same binary can crash on one fleet and run clean on another. When the garbage is numerically benign there is no crash, but the min-rate settle arithmetic is still computed from garbage. Hoist comp_time to function scope and seed it with fio_gettime() at do_io() entry (the previous declaration was loop-scoped, so it was fresh garbage on every iteration). A seed at loop start is safe: until the first real completion stamp the settle check just stays inside its 2-second settle window, which is the intended behavior for the start of a job anyway. handle_thinktime() only ever writes the passed time, so check_min_rate() is the sole reader. Tested with the workload above against a loop device: runs complete cleanly with correct sample counts (5000 lat samples for a 10s run at 500 IOPS), and seeding comp_time with the exact garbage values recovered from a production core dump no longer has any code path to reach the rate checks. Fixes: axboe#2128 Signed-off-by: Vlad Tudose <tudosevt@amazon.com>
Vladyyy
force-pushed
the
do-io-init-comp-time
branch
from
August 6, 2026 23:47
3f2b727 to
3566475
Compare
Collaborator
|
The patch does prevent the assert but If I comment out the abort in |
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
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.
do_io() declared comp_time uninitialized inside the IO loop. It is
only stamped by wait_for_completions() (called when the queue is full
or when polling) and by io_queue_event()'s inline-completion path. An
async engine on a block device with a rate limit takes neither:
submissions return FIO_Q_QUEUED and completions are reaped during the
rate-throttle sleep via io_u_quiesce(), which does not touch
comp_time. Once bytes_done is non-zero, check_min_rate() (enabled by
rate_iops_min / ratemin) computes mtime_since(&td->start, &comp_time)
from whatever bytes happen to be on the stack, and when the garbage
tv_nsec is out of range rel_time_since() aborts:
fio: gettime.c:530: rel_time_since: Assertion `0 <= nsec && nsec < 1000ULL * 1000 * 1000' failed.
This crashed every run of a rate-limited randwrite latency workload
(libaio, iodepth=16, rate_iops=500, rate_iops_min=500, write_lat_log)
~100 ms after start on our EC2 test fleet, deterministically per host:
the stack leftovers at that slot depend on the preceding code paths,
so the same binary can crash on one fleet and run clean on another.
When the garbage is numerically benign there is no crash, but the
min-rate settle arithmetic is still computed from garbage.
Hoist comp_time to function scope and seed it with fio_gettime() at
do_io() entry (the previous declaration was loop-scoped, so it was
fresh garbage on every iteration). A seed at loop start is safe: until
the first real completion stamp the settle check just stays inside its
2-second settle window, which is the intended behavior for the start
of a job anyway. handle_thinktime() only ever writes the passed time,
so check_min_rate() is the sole reader.
Tested with the workload above against a loop device: runs complete
cleanly with correct sample counts (5000 lat samples for a 10s run at
500 IOPS), and seeding comp_time with the exact garbage values
recovered from a production core dump no longer has any code path to
reach the rate checks.
Fixes: #2128
Signed-off-by: Vlad Tudose tudosevt@amazon.com