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