* [PATCH v3 1/2] stat: remove duplicated code in show_mixed_ddir_status()
2022-01-17 15:50 [PATCH v3 0/2] cleanup mixed stat functions Niklas Cassel
@ 2022-01-17 15:50 ` Niklas Cassel
2022-01-17 15:50 ` [PATCH v3 2/2] stat: move unified=both mixed allocation and calculation to new helper Niklas Cassel
2022-01-17 23:21 ` [PATCH v3 0/2] cleanup mixed stat functions Jens Axboe
2 siblings, 0 replies; 4+ messages in thread
From: Niklas Cassel @ 2022-01-17 15:50 UTC (permalink / raw)
To: axboe; +Cc: fio, damien.lemoal, Niklas Cassel
From: Niklas Cassel <niklas.cassel@wdc.com>
When using unified_rw_reporting=mixed, show_ddir_status() is called,
and is solely responsible for printing the mixed stats.
When using unified_rw_reporting=both, show_ddir_status() is called
and prints the regular output, after that, show_mixed_ddir_status()
is called to print the mixed stats.
The way that show_mixed_ddir_status_terse() and
add_mixed_ddir_status_json() is implemented, is to alloc a new local ts
that will hold the mixed result, and then simply call the regular non-mixed
print function show_ddir_status_terse()/add_ddir_status_json() with this
local ts.
show_mixed_ddir_status() also allocates a new local ts, but fails to
initialize the lat percentiles and the percentile_list in the new local ts.
Therefore, show_mixed_ddir_status() has duplicated all the code from
show_ddir_status(), except that it uses the lat percentiles and the
percentile_list from the original ts.
Simplify show_mixed_ddir_status(), to behave in the same way as
show_mixed_ddir_status_terse() and add_mixed_ddir_status_json().
In other words, initialize the lat percentiles and the percentile_list in
the new local ts, and replace all the duplicated code with a simple call to
the regular non-mixed print function (show_ddir_status()).
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
stat.c | 184 +++++++++------------------------------------------------
1 file changed, 27 insertions(+), 157 deletions(-)
diff --git a/stat.c b/stat.c
index 36742a25..42be600d 100644
--- a/stat.c
+++ b/stat.c
@@ -474,163 +474,6 @@ static double convert_agg_kbytes_percent(struct group_run_stats *rs, int ddir, i
return p_of_agg;
}
-static void show_mixed_ddir_status(struct group_run_stats *rs,
- struct thread_stat *ts,
- struct buf_output *out)
-{
- unsigned long runt;
- unsigned long long min, max, bw, iops;
- double mean, dev;
- char *io_p, *bw_p, *bw_p_alt, *iops_p, *post_st = NULL;
- struct thread_stat *ts_lcl;
- int i2p;
- int ddir = 0;
-
- /*
- * Handle aggregation of Reads (ddir = 0), Writes (ddir = 1), and
- * Trims (ddir = 2) */
- ts_lcl = malloc(sizeof(struct thread_stat));
- memset((void *)ts_lcl, 0, sizeof(struct thread_stat));
- /* calculate mixed stats */
- ts_lcl->unified_rw_rep = UNIFIED_MIXED;
- init_thread_stat_min_vals(ts_lcl);
-
- sum_thread_stats(ts_lcl, ts);
-
- assert(ddir_rw(ddir));
-
- if (!ts_lcl->runtime[ddir]) {
- free(ts_lcl);
- return;
- }
-
- i2p = is_power_of_2(rs->kb_base);
- runt = ts_lcl->runtime[ddir];
-
- bw = (1000 * ts_lcl->io_bytes[ddir]) / runt;
- io_p = num2str(ts_lcl->io_bytes[ddir], ts->sig_figs, 1, i2p, N2S_BYTE);
- bw_p = num2str(bw, ts->sig_figs, 1, i2p, ts->unit_base);
- bw_p_alt = num2str(bw, ts->sig_figs, 1, !i2p, ts->unit_base);
-
- iops = (1000 * ts_lcl->total_io_u[ddir]) / runt;
- iops_p = num2str(iops, ts->sig_figs, 1, 0, N2S_NONE);
-
- log_buf(out, " mixed: IOPS=%s, BW=%s (%s)(%s/%llumsec)%s\n",
- iops_p, bw_p, bw_p_alt, io_p,
- (unsigned long long) ts_lcl->runtime[ddir],
- post_st ? : "");
-
- free(post_st);
- free(io_p);
- free(bw_p);
- free(bw_p_alt);
- free(iops_p);
-
- if (calc_lat(&ts_lcl->slat_stat[ddir], &min, &max, &mean, &dev))
- display_lat("slat", min, max, mean, dev, out);
- if (calc_lat(&ts_lcl->clat_stat[ddir], &min, &max, &mean, &dev))
- display_lat("clat", min, max, mean, dev, out);
- if (calc_lat(&ts_lcl->lat_stat[ddir], &min, &max, &mean, &dev))
- display_lat(" lat", min, max, mean, dev, out);
- if (calc_lat(&ts_lcl->clat_high_prio_stat[ddir], &min, &max, &mean, &dev)) {
- display_lat(ts_lcl->lat_percentiles ? "high prio_lat" : "high prio_clat",
- min, max, mean, dev, out);
- if (calc_lat(&ts_lcl->clat_low_prio_stat[ddir], &min, &max, &mean, &dev))
- display_lat(ts_lcl->lat_percentiles ? "low prio_lat" : "low prio_clat",
- min, max, mean, dev, out);
- }
-
- if (ts->slat_percentiles && ts_lcl->slat_stat[ddir].samples > 0)
- show_clat_percentiles(ts_lcl->io_u_plat[FIO_SLAT][ddir],
- ts_lcl->slat_stat[ddir].samples,
- ts->percentile_list,
- ts->percentile_precision, "slat", out);
- if (ts->clat_percentiles && ts_lcl->clat_stat[ddir].samples > 0)
- show_clat_percentiles(ts_lcl->io_u_plat[FIO_CLAT][ddir],
- ts_lcl->clat_stat[ddir].samples,
- ts->percentile_list,
- ts->percentile_precision, "clat", out);
- if (ts->lat_percentiles && ts_lcl->lat_stat[ddir].samples > 0)
- show_clat_percentiles(ts_lcl->io_u_plat[FIO_LAT][ddir],
- ts_lcl->lat_stat[ddir].samples,
- ts->percentile_list,
- ts->percentile_precision, "lat", out);
-
- if (ts->clat_percentiles || ts->lat_percentiles) {
- const char *name = ts->lat_percentiles ? "lat" : "clat";
- char prio_name[32];
- uint64_t samples;
-
- if (ts->lat_percentiles)
- samples = ts_lcl->lat_stat[ddir].samples;
- else
- samples = ts_lcl->clat_stat[ddir].samples;
-
- /* Only print if high and low priority stats were collected */
- if (ts_lcl->clat_high_prio_stat[ddir].samples > 0 &&
- ts_lcl->clat_low_prio_stat[ddir].samples > 0) {
- sprintf(prio_name, "high prio (%.2f%%) %s",
- 100. * (double) ts_lcl->clat_high_prio_stat[ddir].samples / (double) samples,
- name);
- show_clat_percentiles(ts_lcl->io_u_plat_high_prio[ddir],
- ts_lcl->clat_high_prio_stat[ddir].samples,
- ts->percentile_list,
- ts->percentile_precision, prio_name, out);
-
- sprintf(prio_name, "low prio (%.2f%%) %s",
- 100. * (double) ts_lcl->clat_low_prio_stat[ddir].samples / (double) samples,
- name);
- show_clat_percentiles(ts_lcl->io_u_plat_low_prio[ddir],
- ts_lcl->clat_low_prio_stat[ddir].samples,
- ts->percentile_list,
- ts->percentile_precision, prio_name, out);
- }
- }
-
- if (calc_lat(&ts_lcl->bw_stat[ddir], &min, &max, &mean, &dev)) {
- double p_of_agg = 100.0, fkb_base = (double)rs->kb_base;
- const char *bw_str;
-
- if ((rs->unit_base == 1) && i2p)
- bw_str = "Kibit";
- else if (rs->unit_base == 1)
- bw_str = "kbit";
- else if (i2p)
- bw_str = "KiB";
- else
- bw_str = "kB";
-
- p_of_agg = convert_agg_kbytes_percent(rs, ddir, mean);
-
- if (rs->unit_base == 1) {
- min *= 8.0;
- max *= 8.0;
- mean *= 8.0;
- dev *= 8.0;
- }
-
- if (mean > fkb_base * fkb_base) {
- min /= fkb_base;
- max /= fkb_base;
- mean /= fkb_base;
- dev /= fkb_base;
- bw_str = (rs->unit_base == 1 ? "Mibit" : "MiB");
- }
-
- log_buf(out, " bw (%5s/s): min=%5llu, max=%5llu, per=%3.2f%%, "
- "avg=%5.02f, stdev=%5.02f, samples=%" PRIu64 "\n",
- bw_str, min, max, p_of_agg, mean, dev,
- (&ts_lcl->bw_stat[ddir])->samples);
- }
- if (calc_lat(&ts_lcl->iops_stat[ddir], &min, &max, &mean, &dev)) {
- log_buf(out, " iops : min=%5llu, max=%5llu, "
- "avg=%5.02f, stdev=%5.02f, samples=%" PRIu64 "\n",
- min, max, mean, dev, (&ts_lcl->iops_stat[ddir])->samples);
- }
-
- free(ts_lcl);
-}
-
static void show_ddir_status(struct group_run_stats *rs, struct thread_stat *ts,
int ddir, struct buf_output *out)
{
@@ -797,6 +640,33 @@ static void show_ddir_status(struct group_run_stats *rs, struct thread_stat *ts,
}
}
+static void show_mixed_ddir_status(struct group_run_stats *rs,
+ struct thread_stat *ts,
+ struct buf_output *out)
+{
+ struct thread_stat *ts_lcl;
+
+ /*
+ * Handle aggregation of Reads (ddir = 0), Writes (ddir = 1), and
+ * Trims (ddir = 2)
+ */
+ ts_lcl = malloc(sizeof(struct thread_stat));
+ memset((void *)ts_lcl, 0, sizeof(struct thread_stat));
+ /* calculate mixed stats */
+ ts_lcl->unified_rw_rep = UNIFIED_MIXED;
+ init_thread_stat_min_vals(ts_lcl);
+ ts_lcl->lat_percentiles = ts->lat_percentiles;
+ ts_lcl->clat_percentiles = ts->clat_percentiles;
+ ts_lcl->slat_percentiles = ts->slat_percentiles;
+ ts_lcl->percentile_precision = ts->percentile_precision;
+ memcpy(ts_lcl->percentile_list, ts->percentile_list, sizeof(ts->percentile_list));
+
+ sum_thread_stats(ts_lcl, ts);
+
+ show_ddir_status(rs, ts_lcl, DDIR_READ, out);
+ free(ts_lcl);
+}
+
static bool show_lat(double *io_u_lat, int nr, const char **ranges,
const char *msg, struct buf_output *out)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v3 2/2] stat: move unified=both mixed allocation and calculation to new helper
2022-01-17 15:50 [PATCH v3 0/2] cleanup mixed stat functions Niklas Cassel
2022-01-17 15:50 ` [PATCH v3 1/2] stat: remove duplicated code in show_mixed_ddir_status() Niklas Cassel
@ 2022-01-17 15:50 ` Niklas Cassel
2022-01-17 23:21 ` [PATCH v3 0/2] cleanup mixed stat functions Jens Axboe
2 siblings, 0 replies; 4+ messages in thread
From: Niklas Cassel @ 2022-01-17 15:50 UTC (permalink / raw)
To: axboe; +Cc: fio, damien.lemoal, Niklas Cassel
From: Niklas Cassel <niklas.cassel@wdc.com>
When using unified_rw_reporting=both, we need to print both the
per ddir stats, as well as the mixed stats.
In order to print both, the regular printing functions are responsible
for printing the per ddir stats from the unmodified struct thread_stat,
and show_mixed_ddir_status(), show_mixed_ddir_status_terse()
or add_mixed_ddir_status_json() is responsible for calculating and
printing the mixed stats.
In order to keep the original struct thread_stat intact, these three
functions have to allocate a new local thread_stat, where the mixed ddir
result can be stored before printing.
Move the allocation and calculation of this new struct thread_stat to a
new helper function, so that the code is easier to follow.
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
stat.c | 95 ++++++++++++++++++++++++----------------------------------
1 file changed, 39 insertions(+), 56 deletions(-)
diff --git a/stat.c b/stat.c
index 42be600d..b08d2f25 100644
--- a/stat.c
+++ b/stat.c
@@ -462,6 +462,35 @@ static void display_lat(const char *name, unsigned long long min,
free(maxp);
}
+static struct thread_stat *gen_mixed_ddir_stats_from_ts(struct thread_stat *ts)
+{
+ struct thread_stat *ts_lcl;
+
+ /*
+ * Handle aggregation of Reads (ddir = 0), Writes (ddir = 1), and
+ * Trims (ddir = 2)
+ */
+ ts_lcl = malloc(sizeof(struct thread_stat));
+ if (!ts_lcl) {
+ log_err("fio: failed to allocate local thread stat\n");
+ return NULL;
+ }
+
+ init_thread_stat(ts_lcl);
+
+ /* calculate mixed stats */
+ ts_lcl->unified_rw_rep = UNIFIED_MIXED;
+ ts_lcl->lat_percentiles = ts->lat_percentiles;
+ ts_lcl->clat_percentiles = ts->clat_percentiles;
+ ts_lcl->slat_percentiles = ts->slat_percentiles;
+ ts_lcl->percentile_precision = ts->percentile_precision;
+ memcpy(ts_lcl->percentile_list, ts->percentile_list, sizeof(ts->percentile_list));
+
+ sum_thread_stats(ts_lcl, ts);
+
+ return ts_lcl;
+}
+
static double convert_agg_kbytes_percent(struct group_run_stats *rs, int ddir, int mean)
{
double p_of_agg = 100.0;
@@ -644,26 +673,11 @@ static void show_mixed_ddir_status(struct group_run_stats *rs,
struct thread_stat *ts,
struct buf_output *out)
{
- struct thread_stat *ts_lcl;
+ struct thread_stat *ts_lcl = gen_mixed_ddir_stats_from_ts(ts);
- /*
- * Handle aggregation of Reads (ddir = 0), Writes (ddir = 1), and
- * Trims (ddir = 2)
- */
- ts_lcl = malloc(sizeof(struct thread_stat));
- memset((void *)ts_lcl, 0, sizeof(struct thread_stat));
- /* calculate mixed stats */
- ts_lcl->unified_rw_rep = UNIFIED_MIXED;
- init_thread_stat_min_vals(ts_lcl);
- ts_lcl->lat_percentiles = ts->lat_percentiles;
- ts_lcl->clat_percentiles = ts->clat_percentiles;
- ts_lcl->slat_percentiles = ts->slat_percentiles;
- ts_lcl->percentile_precision = ts->percentile_precision;
- memcpy(ts_lcl->percentile_list, ts->percentile_list, sizeof(ts->percentile_list));
-
- sum_thread_stats(ts_lcl, ts);
+ if (ts_lcl)
+ show_ddir_status(rs, ts_lcl, DDIR_READ, out);
- show_ddir_status(rs, ts_lcl, DDIR_READ, out);
free(ts_lcl);
}
@@ -1332,27 +1346,11 @@ static void show_mixed_ddir_status_terse(struct thread_stat *ts,
struct group_run_stats *rs,
int ver, struct buf_output *out)
{
- struct thread_stat *ts_lcl;
+ struct thread_stat *ts_lcl = gen_mixed_ddir_stats_from_ts(ts);
- /*
- * Handle aggregation of Reads (ddir = 0), Writes (ddir = 1), and
- * Trims (ddir = 2)
- */
- ts_lcl = malloc(sizeof(struct thread_stat));
- memset((void *)ts_lcl, 0, sizeof(struct thread_stat));
- /* calculate mixed stats */
- ts_lcl->unified_rw_rep = UNIFIED_MIXED;
- init_thread_stat_min_vals(ts_lcl);
- ts_lcl->lat_percentiles = ts->lat_percentiles;
- ts_lcl->clat_percentiles = ts->clat_percentiles;
- ts_lcl->slat_percentiles = ts->slat_percentiles;
- ts_lcl->percentile_precision = ts->percentile_precision;
- memcpy(ts_lcl->percentile_list, ts->percentile_list, sizeof(ts->percentile_list));
-
- sum_thread_stats(ts_lcl, ts);
+ if (ts_lcl)
+ show_ddir_status_terse(ts_lcl, rs, DDIR_READ, ver, out);
- /* add the aggregated stats to json parent */
- show_ddir_status_terse(ts_lcl, rs, DDIR_READ, ver, out);
free(ts_lcl);
}
@@ -1530,27 +1528,12 @@ static void add_ddir_status_json(struct thread_stat *ts,
static void add_mixed_ddir_status_json(struct thread_stat *ts,
struct group_run_stats *rs, struct json_object *parent)
{
- struct thread_stat *ts_lcl;
-
- /*
- * Handle aggregation of Reads (ddir = 0), Writes (ddir = 1), and
- * Trims (ddir = 2)
- */
- ts_lcl = malloc(sizeof(struct thread_stat));
- memset((void *)ts_lcl, 0, sizeof(struct thread_stat));
- /* calculate mixed stats */
- ts_lcl->unified_rw_rep = UNIFIED_MIXED;
- init_thread_stat_min_vals(ts_lcl);
- ts_lcl->lat_percentiles = ts->lat_percentiles;
- ts_lcl->clat_percentiles = ts->clat_percentiles;
- ts_lcl->slat_percentiles = ts->slat_percentiles;
- ts_lcl->percentile_precision = ts->percentile_precision;
- memcpy(ts_lcl->percentile_list, ts->percentile_list, sizeof(ts->percentile_list));
-
- sum_thread_stats(ts_lcl, ts);
+ struct thread_stat *ts_lcl = gen_mixed_ddir_stats_from_ts(ts);
/* add the aggregated stats to json parent */
- add_ddir_status_json(ts_lcl, rs, DDIR_READ, parent);
+ if (ts_lcl)
+ add_ddir_status_json(ts_lcl, rs, DDIR_READ, parent);
+
free(ts_lcl);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread