On 2/1/22 06:13, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> In order to avoid allocating a clat_prio_stat array for threadstats that we
> know will never be able to contain more than a single priority, introduce a
> new member disable_prio_stat in struct thread_stat.
>
> The naming prefix is disable, since we want the default value to be 0
> (enabled). This is because in default case, we do want sum_thread_stats()
> to generate a per prio stat array. Only in the case where we know that we
> don't want per stats to be generated, should this member be set to 1.
s/per stats/per priority stats ?
>
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
> ---
> client.c | 1 +
> rate-submit.c | 9 +++++++++
> server.c | 1 +
> stat.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
> stat.h | 1 +
> 5 files changed, 66 insertions(+)
>
> diff --git a/client.c b/client.c
> index e51138ee..e5f6cfa7 100644
> --- a/client.c
> +++ b/client.c
> @@ -954,6 +954,7 @@ static void convert_ts(struct thread_stat *dst, struct thread_stat *src)
> dst->members = le32_to_cpu(src->members);
> dst->unified_rw_rep = le32_to_cpu(src->unified_rw_rep);
> dst->ioprio = le32_to_cpu(src->ioprio);
> + dst->disable_prio_stat = le32_to_cpu(src->disable_prio_stat);
>
> for (i = 0; i < DDIR_RWDIR_CNT; i++) {
> convert_io_stat(&dst->clat_stat[i], &src->clat_stat[i]);
> diff --git a/rate-submit.c b/rate-submit.c
> index 752c30a5..e3a71168 100644
> --- a/rate-submit.c
> +++ b/rate-submit.c
> @@ -195,6 +195,15 @@ static void io_workqueue_exit_worker_fn(struct submit_worker *sw,
> struct thread_data *td = sw->priv;
>
> (*sum_cnt)++;
> +
> + /*
> + * io_workqueue_update_acct_fn() doesn't support per prio stats, and
> + * even if it did, offload can't be used with all async IO engines.
> + * If group reporting is set in the parent td, the group result
> + * generated by __show_run_stats() can still contain multiple prios
> + * from different offloaded jobs.
> + */
> + sw->wq->td->ts.disable_prio_stat = 1;
> sum_thread_stats(&sw->wq->td->ts, &td->ts);
>
> fio_options_free(td);
> diff --git a/server.c b/server.c
> index 4ddf682b..d82c7e5b 100644
> --- a/server.c
> +++ b/server.c
> @@ -1486,6 +1486,7 @@ void fio_server_send_ts(struct thread_stat *ts, struct group_run_stats *rs)
> p.ts.members = cpu_to_le32(ts->members);
> p.ts.unified_rw_rep = cpu_to_le32(ts->unified_rw_rep);
> p.ts.ioprio = cpu_to_le32(ts->ioprio);
> + p.ts.disable_prio_stat = cpu_to_le32(ts->disable_prio_stat);
>
> for (i = 0; i < DDIR_RWDIR_CNT; i++) {
> convert_io_stat(&p.ts.clat_stat[i], &ts->clat_stat[i]);
> diff --git a/stat.c b/stat.c
> index 7f134d35..3345c7c4 100644
> --- a/stat.c
> +++ b/stat.c
> @@ -2171,6 +2171,58 @@ void init_thread_stat(struct thread_stat *ts)
> ts->groupid = -1;
> }
>
> +static void init_per_prio_stats(struct thread_stat *threadstats, int nr_ts)
> +{
> + struct thread_data *td;
> + struct thread_stat *ts;
> + int i, j, last_ts, idx;
> + enum fio_ddir ddir;
> +
> + j = 0;
> + last_ts = -1;
> + idx = 0;
> +
> + /*
> + * Loop through all tds, if a td requires per prio stats, temporarily
> + * store a 1 in ts->disable_prio_stat, and then do an additional
> + * loop at the end where we invert the ts->disable_prio_stat values.
> + */
> + for_each_td(td, i) {
> + if (!td->o.stats)
> + continue;
> + if (idx &&
> + (!td->o.group_reporting ||
> + (td->o.group_reporting && last_ts != td->groupid))) {
> + idx = 0;
> + j++;
> + }
> +
> + last_ts = td->groupid;
> + ts = &threadstats[j];
> +
> + /* idx == 0 means first td in group, or td is not in a group. */
> + if (idx == 0)
> + ts->ioprio = td->ioprio;
> + else if (td->ioprio != ts->ioprio)
> + ts->disable_prio_stat = 1;
> +
> + for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++) {
> + if (td->ts.clat_prio[ddir]) {
> + ts->disable_prio_stat = 1;
> + break;
> + }
> + }
> +
> + idx++;
> + }
> +
> + /* Loop through all dst threadstats and fixup the values. */
> + for (i = 0; i < nr_ts; i++) {
> + ts = &threadstats[i];
> + ts->disable_prio_stat = !ts->disable_prio_stat;
> + }
> +}
> +
> void __show_run_stats(void)
> {
> struct group_run_stats *runstats, *rs;
> @@ -2217,6 +2269,8 @@ void __show_run_stats(void)
> opt_lists[i] = NULL;
> }
>
> + init_per_prio_stats(threadstats, nr_ts);
> +
> j = 0;
> last_ts = -1;
> idx = 0;
> diff --git a/stat.h b/stat.h
> index 1e720ad8..4b1d4cb8 100644
> --- a/stat.h
> +++ b/stat.h
> @@ -174,6 +174,7 @@ struct thread_stat {
> char description[FIO_JOBDESC_SIZE];
> uint32_t members;
> uint32_t unified_rw_rep;
> + uint32_t disable_prio_stat;
>
> /*
> * bandwidth and latency stats
With the commit message typo addressed,
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
--
Damien Le Moal
Western Digital Research
