From 4b3d7ff7216dfdb1708cc8ad1b267186440eac11 Mon Sep 17 00:00:00 2001 From: Alexey Neyman Date: Thu, 5 Oct 2023 18:43:17 +0000 Subject: [PATCH 1/2] Change memcpy() calls to assignments This is to avoid triggering a spurious warning caused by [1], which is triggered by the next commit in chain (unrelated change in update_io_tick_disk()). [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111696 Signed-off-by: Alexey Neyman --- diskutil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diskutil.c b/diskutil.c index cf4ede8577..40f2c26030 100644 --- a/diskutil.c +++ b/diskutil.c @@ -103,8 +103,8 @@ static void update_io_tick_disk(struct disk_util *du) fio_gettime(&t, NULL); dus->s.msec += mtime_since(&du->time, &t); - memcpy(&du->time, &t, sizeof(t)); - memcpy(&ldus->s, &__dus.s, sizeof(__dus.s)); + du->time = t; + ldus->s = __dus.s; } int update_io_ticks(void) From 5f7bd351630d02c223772ade57bf321a75fba020 Mon Sep 17 00:00:00 2001 From: Alexey Neyman Date: Tue, 3 Oct 2023 22:49:02 +0000 Subject: [PATCH 2/2] Handle 32-bit overflows in disk utilization stats Linux prints [1] some of the values reported in block device's `stat` sysfs file as 32-bit unsigned integers. fio interprets them as 64-bit integers when reading that sysfs file and performs further arithmetics on them in 64-bits. If the reported value overflows during fio run, a huge bogus value is reported in the "disk utilization" block instead. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/genhd.c#n962 Signed-off-by: Alexey Neyman --- diskutil.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/diskutil.c b/diskutil.c index 40f2c26030..69b3dd263f 100644 --- a/diskutil.c +++ b/diskutil.c @@ -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; @@ -96,10 +113,11 @@ 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);