On 1/15/22 00:46, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> 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@xxxxxxx>
> ---
> stat.c | 81 ++++++++++++++++++++--------------------------------------
> 1 file changed, 27 insertions(+), 54 deletions(-)
>
> diff --git a/stat.c b/stat.c
> index 2b3687bb..ff2393b0 100644
> --- a/stat.c
> +++ b/stat.c
> @@ -462,6 +462,30 @@ 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));
No failed allocation check ?
> + memset((void *)ts_lcl, 0, sizeof(struct thread_stat));
Nit: a blank line here would be nice, for readability.
> + /* 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);
> +
> + 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,24 +668,7 @@ 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);
> + struct thread_stat *ts_lcl = gen_mixed_ddir_stats_from_ts(ts);
>
If you add allocation failure check in gen_mixed_ddir_stats_from_ts(),
then a NULL check is needed here too, alnd in the following functions
below too.
> show_ddir_status(rs, ts_lcl, DDIR_READ, out);
> free(ts_lcl);
> @@ -1332,24 +1339,7 @@ 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;
> -
> - /*
> - * 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);
>
> show_ddir_status_terse(ts_lcl, rs, DDIR_READ, ver, out);
> free(ts_lcl);
> @@ -1529,24 +1519,7 @@ 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);
--
Damien Le Moal
Western Digital Research
