On 2/1/22 06:13, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> lat_percentiles is used to control if the high/low latency statistics
> (which are saved in ts->io_u_plat_high_prio/ts->io_u_plat_high_prio)
missing io_u_plat_low_prio (io_u_plat_high_prio twice)
> should collect and display total latencies instead of completion latencies.
>
> When doing group reporting, stat.c:__show_run_stats() happily overwrites
> the dst ts with the setting of each job, which means that the summing can
> take total lat samples for some jobs, and clat samples for some jobs, while
> adding samples into the same group result.
>
> The output summary will claim that the results are of whatever type the
> final job in the group is set to.
>
> To make sure that this cannot happen, verify that the option
> lat_percentiles is consistent for all jobs in group.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
With the nit above fixed, this looks good to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> ---
> init.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/init.c b/init.c
> index 07daaa84..7d5f2221 100644
> --- a/init.c
> +++ b/init.c
> @@ -1445,6 +1445,26 @@ static bool wait_for_ok(const char *jobname, struct thread_options *o)
> return true;
> }
>
> +static int verify_per_group_options(struct thread_data *td, const char *jobname)
> +{
> + struct thread_data *td2;
> + int i;
> +
> + for_each_td(td2, i) {
> + if (td->groupid != td2->groupid)
> + continue;
> +
> + if (td->o.stats &&
> + td->o.lat_percentiles != td2->o.lat_percentiles) {
> + log_err("fio: lat_percentiles in job: %s differs from group\n",
> + jobname);
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> /*
> * Treat an empty log file name the same as a one not given
> */
> @@ -1563,6 +1583,10 @@ static int add_job(struct thread_data *td, const char *jobname, int job_add_num,
> td->groupid = groupid;
> prev_group_jobs++;
>
> + if (td->o.group_reporting && prev_group_jobs > 1 &&
> + verify_per_group_options(td, jobname))
> + goto err;
> +
> if (setup_rate(td))
> goto err;
>
--
Damien Le Moal
Western Digital Research
