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

Handle 32-bit overflows in disk utilization stats #1641

Merged
merged 2 commits into from
Oct 6, 2023
Merged

Handle 32-bit overflows in disk utilization stats #1641

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b3d7ff
Change memcpy() calls to assignments
stilor Oct 5, 2023
5f7bd35
Handle 32-bit overflows in disk utilization stats
stilor Oct 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions diskutil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,23 @@ static int get_io_ticks(struct disk_util *du, struct disk_util_stat *dus)
return ret != 10;
}

static uint64_t safe_32bit_diff(uint64_t nval, uint64_t oval)
{
/* Linux kernel prints some of the stat fields as 32-bit integers. It is
* possible that the value overflows, but since fio uses unsigned 64-bit
* arithmetic in update_io_tick_disk(), it instead results in a huge
* bogus value being added to the respective accumulating field. Just
* in case Linux starts reporting these metrics as 64-bit values in the
* future, check that overflow actually happens around the 32-bit
* unsigned boundary; assume overflow only happens once between
* successive polls.
*/
if (oval <= nval || oval >= (1ull << 32))
return nval - oval;
else
return (1ull << 32) + nval - oval;
}

static void update_io_tick_disk(struct disk_util *du)
{
struct disk_util_stat __dus, *dus, *ldus;
Expand All @@ -96,15 +113,16 @@ static void update_io_tick_disk(struct disk_util *du)
dus->s.ios[1] += (__dus.s.ios[1] - ldus->s.ios[1]);
dus->s.merges[0] += (__dus.s.merges[0] - ldus->s.merges[0]);
dus->s.merges[1] += (__dus.s.merges[1] - ldus->s.merges[1]);
dus->s.ticks[0] += (__dus.s.ticks[0] - ldus->s.ticks[0]);
dus->s.ticks[1] += (__dus.s.ticks[1] - ldus->s.ticks[1]);
dus->s.io_ticks += (__dus.s.io_ticks - ldus->s.io_ticks);
dus->s.time_in_queue += (__dus.s.time_in_queue - ldus->s.time_in_queue);
dus->s.ticks[0] += safe_32bit_diff(__dus.s.ticks[0], ldus->s.ticks[0]);
dus->s.ticks[1] += safe_32bit_diff(__dus.s.ticks[1], ldus->s.ticks[1]);
dus->s.io_ticks += safe_32bit_diff(__dus.s.io_ticks, ldus->s.io_ticks);
dus->s.time_in_queue +=
safe_32bit_diff(__dus.s.time_in_queue, ldus->s.time_in_queue);

fio_gettime(&t, NULL);
dus->s.msec += mtime_since(&du->time, &t);
memcpy(&du->time, &t, sizeof(t));
Copy link
Contributor

@bvanassche bvanassche Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Since this change is unrelated to the 32-bit overflow fix, please consider moving this change into a separate patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

memcpy(&ldus->s, &__dus.s, sizeof(__dus.s));
du->time = t;
ldus->s = __dus.s;
}

int update_io_ticks(void)
Expand Down