On Mon, Jan 17, 2022 at 09:45:36AM +0900, Damien Le Moal wrote:
> 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 ?
This patch simply creates a helper function.
The actual code should be identical to before.
If we want to add error handling to malloc() in fio, in all the places
where it is missing (which is a lot!), that is probably more suited for
a major cleanup series which addresses all the places where error handling
for malloc() is missing.
Right now, I'm not sure if fio is missing it because that is the chosen
design pattern (maybe because Linux memory overcommitment is enabled by
default?), or if it is by mistake, so I decided to keep the code as is.
>
> > + memset((void *)ts_lcl, 0, sizeof(struct thread_stat));
>
> Nit: a blank line here would be nice, for readability.
I will send out a V2 that adds a blank line, and use init_thread_stat()
instead (which does the memset() and calls init_thread_stat_min_vals(),
so that we can remove those two lines from this function).
Kind regards,
Niklas
