[PATCH 1/6] stat: change json+ output format so that instead of printing the raw clat data structure, use actual durations instead of array indices and print only bins with nonzero counts

[PATCH 1/6] stat: change json+ output format so that instead of printing the raw clat data structure, use actual durations instead of array indices and print only bins with nonzero counts

[Vincent Fu]

From: Vincent Fu <Vincent.Fu@sandisk.com>

---
 stat.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/stat.c b/stat.c
index fde7af2..4a6e1b9 100644
--- a/stat.c
+++ b/stat.c
@@ -98,7 +98,7 @@ static unsigned int plat_val_to_idx(unsigned int val)
  * Convert the given index of the bucket array to the value
  * represented by the bucket
  */
-static unsigned int plat_idx_to_val(unsigned int idx)
+static unsigned long long plat_idx_to_val(unsigned int idx)
 {
 	unsigned int error_bits, k, base;
 
@@ -972,12 +972,11 @@ static void add_ddir_status_json(struct thread_stat *ts,
 		clat_bins_object = json_create_object();
 		json_object_add_value_object(tmp_object, "bins", clat_bins_object);
 		for(i = 0; i < FIO_IO_U_PLAT_NR; i++) {
-			snprintf(buf, sizeof(buf), "%d", i);
-			json_object_add_value_int(clat_bins_object, (const char *)buf, ts->io_u_plat[ddir][i]);
+			if (ts->io_u_plat[ddir][i]) {
+				snprintf(buf, sizeof(buf), "%llu", plat_idx_to_val(i));
+				json_object_add_value_int(clat_bins_object, (const char *)buf, ts->io_u_plat[ddir][i]);
+			}
 		}
-		json_object_add_value_int(clat_bins_object, "FIO_IO_U_PLAT_BITS", FIO_IO_U_PLAT_BITS);
-		json_object_add_value_int(clat_bins_object, "FIO_IO_U_PLAT_VAL", FIO_IO_U_PLAT_VAL);
-		json_object_add_value_int(clat_bins_object, "FIO_IO_U_PLAT_NR", FIO_IO_U_PLAT_NR);
 	}
 
 	if (!calc_lat(&ts->lat_stat[ddir], &min, &max, &mean, &dev)) {
-- 
2.7.4

[PATCH 2/6] Revert "tools/fio_latency2csv.py: add tool that converts json+ to CSV"

[Vincent Fu]

From: Vincent Fu <Vincent.Fu@sandisk.com>

This reverts commit 6cd4d583c75042bf1edd3d2d5c6626883755bfe4.
The script is no longer needed because the json+ output format now
automatically does the transformation carried out by the script.
---
 Makefile                 |   2 +-
 tools/fio_latency2csv.py | 101 -----------------------------------------------
 2 files changed, 1 insertion(+), 102 deletions(-)
 delete mode 100755 tools/fio_latency2csv.py

diff --git a/Makefile b/Makefile
index 66083ff..1f0f5d0 100644
--- a/Makefile
+++ b/Makefile
@@ -26,7 +26,7 @@ OPTFLAGS= -g -ffast-math
 CFLAGS	= -std=gnu99 -Wwrite-strings -Wall -Wdeclaration-after-statement $(OPTFLAGS) $(EXTFLAGS) $(BUILD_CFLAGS) -I. -I$(SRCDIR)
 LIBS	+= -lm $(EXTLIBS)
 PROGS	= fio
-SCRIPTS = $(addprefix $(SRCDIR)/,tools/fio_generate_plots tools/plot/fio2gnuplot tools/genfio tools/fiologparser.py tools/fio_latency2csv.py tools/hist/fiologparser_hist.py)
+SCRIPTS = $(addprefix $(SRCDIR)/,tools/fio_generate_plots tools/plot/fio2gnuplot tools/genfio tools/fiologparser.py tools/hist/fiologparser_hist.py)
 
 ifndef CONFIG_FIO_NO_OPT
   CFLAGS += -O3 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
diff --git a/tools/fio_latency2csv.py b/tools/fio_latency2csv.py
deleted file mode 100755
index 93586d2..0000000
--- a/tools/fio_latency2csv.py
+++ /dev/null
@@ -1,101 +0,0 @@
-#!/usr/bin/python
-#
-# fio_latency2csv.py
-#
-# This tool converts fio's json+ completion latency data to CSV format.
-# For example:
-#
-# fio_latency2csv.py fio-jsonplus.output fio-latency.csv
-#
-
-import os
-import json
-import argparse
-
-
-def parse_args():
-    parser = argparse.ArgumentParser()
-    parser.add_argument('source',
-                        help='fio json+ output file containing completion '
-                             'latency data')
-    parser.add_argument('dest',
-                        help='destination file stub for latency data in CSV '
-                             'format. job number will be appended to filename')
-    args = parser.parse_args()
-
-    return args
-
-
-# from stat.c
-def plat_idx_to_val(idx, FIO_IO_U_PLAT_BITS=6, FIO_IO_U_PLAT_VAL=64):
-    # MSB <= (FIO_IO_U_PLAT_BITS-1), cannot be rounded off. Use
-    # all bits of the sample as index
-    if (idx < (FIO_IO_U_PLAT_VAL << 1)):
-        return idx
-
-    # Find the group and compute the minimum value of that group
-    error_bits = (idx >> FIO_IO_U_PLAT_BITS) - 1
-    base = 1 << (error_bits + FIO_IO_U_PLAT_BITS)
-
-    # Find its bucket number of the group
-    k = idx % FIO_IO_U_PLAT_VAL
-
-    # Return the mean of the range of the bucket
-    return (base + ((k + 0.5) * (1 << error_bits)))
-
-
-def percentile(idx, run_total):
-    total = run_total[len(run_total)-1]
-    if total == 0:
-        return 0
-
-    return float(run_total[x]) / total
-
-
-if __name__ == '__main__':
-    args = parse_args()
-
-    with open(args.source, 'r') as source:
-        jsondata = json.loads(source.read())
-
-    bins = {}
-    bin_const = {}
-    run_total = {}
-    ddir_list = ['read', 'write', 'trim']
-    const_list = ['FIO_IO_U_PLAT_NR', 'FIO_IO_U_PLAT_BITS',
-                  'FIO_IO_U_PLAT_VAL']
-
-    for jobnum in range(0,len(jsondata['jobs'])):
-        prev_ddir = None
-        for ddir in ddir_list:
-            bins[ddir] = jsondata['jobs'][jobnum][ddir]['clat']['bins']
-
-            bin_const[ddir] = {}
-            for const in const_list:
-                bin_const[ddir][const] = bins[ddir].pop(const)
-                if prev_ddir:
-                    assert bin_const[ddir][const] == bin_const[prev_ddir][const]
-            prev_ddir = ddir
-
-            run_total[ddir] = [0 for x in
-                               range(bin_const[ddir]['FIO_IO_U_PLAT_NR'])]
-            run_total[ddir][0] = bins[ddir]['0']
-            for x in range(1, bin_const[ddir]['FIO_IO_U_PLAT_NR']):
-                run_total[ddir][x] = run_total[ddir][x-1] + bins[ddir][str(x)]
-        
-        stub, ext = os.path.splitext(args.dest)
-        outfile = stub + '_job' + str(jobnum) + ext
-
-        with open(outfile, 'w') as output:
-            output.write("clat (usec),")
-            for ddir in ddir_list:
-                output.write("{0},".format(ddir))
-            output.write("\n")
-
-            for x in range(bin_const['read']['FIO_IO_U_PLAT_NR']):
-                output.write("{0},".format(plat_idx_to_val(x,
-                                          bin_const['read']['FIO_IO_U_PLAT_BITS'],
-                                          bin_const['read']['FIO_IO_U_PLAT_VAL'])))
-                for ddir in ddir_list:
-                    output.write("{0},".format(percentile(x, run_total[ddir])))
-                output.write("\n")
-- 
2.7.4

[PATCH 3/6] server: update server version for the addition of the --stats option

[Vincent Fu]

From: Vincent Fu <Vincent.Fu@sandisk.com>

---
 server.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server.h b/server.h
index 798d5a8..5c720d4 100644
--- a/server.h
+++ b/server.h
@@ -49,7 +49,7 @@ struct fio_net_cmd_reply {
 };
 
 enum {
-	FIO_SERVER_VER			= 60,
+	FIO_SERVER_VER			= 61,
 
 	FIO_SERVER_MAX_FRAGMENT_PDU	= 1024,
 	FIO_SERVER_MAX_CMD_MB		= 2048,
-- 
2.7.4

[PATCH 4/6] stat: reset_io_stats: fix a problem, rearrange some code

[Vincent Fu]

From: Vincent Fu <Vincent.Fu@sandisk.com>

1) Make sure all the io_u_lat_u/m are reset
2) Only need to zero out total_submit/complete once
3) Reset total/short/drop_io_u inside the other loop that iterates over data directions
---
 stat.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/stat.c b/stat.c
index 4a6e1b9..5061bea 100644
--- a/stat.c
+++ b/stat.c
@@ -2171,6 +2171,9 @@ void reset_io_stats(struct thread_data *td)
 
 		ts->io_bytes[i] = 0;
 		ts->runtime[i] = 0;
+		ts->total_io_u[i] = 0;
+		ts->short_io_u[i] = 0;
+		ts->drop_io_u[i] = 0;
 
 		for (j = 0; j < FIO_IO_U_PLAT_NR; j++)
 			ts->io_u_plat[i][j] = 0;
@@ -2180,17 +2183,15 @@ void reset_io_stats(struct thread_data *td)
 		ts->io_u_map[i] = 0;
 		ts->io_u_submit[i] = 0;
 		ts->io_u_complete[i] = 0;
+	}
+
+	for (i = 0; i < FIO_IO_U_LAT_U_NR; i++)
 		ts->io_u_lat_u[i] = 0;
+	for (i = 0; i < FIO_IO_U_LAT_M_NR; i++)
 		ts->io_u_lat_m[i] = 0;
-		ts->total_submit = 0;
-		ts->total_complete = 0;
-	}
 
-	for (i = 0; i < 3; i++) {
-		ts->total_io_u[i] = 0;
-		ts->short_io_u[i] = 0;
-		ts->drop_io_u[i] = 0;
-	}
+	ts->total_submit = 0;
+	ts->total_complete = 0;
 }
 
 static void __add_stat_to_log(struct io_log *iolog, enum fio_ddir ddir,
-- 
2.7.4

[PATCH 5/6] client/server: make sure that all elements in io_u_lat_m[] are transferred and received

[Vincent Fu]

From: Vincent Fu <Vincent.Fu@sandisk.com>

FIO_IO_U_LAT_U_NR is 10 whereas FIO_IO_U_LAT_M_NR is 12. So we must iterate
over io_u_lat_u and io_u_lat_m separately.
---
 client.c | 4 ++--
 server.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/client.c b/client.c
index 7934661..80096bf 100644
--- a/client.c
+++ b/client.c
@@ -908,10 +908,10 @@ static void convert_ts(struct thread_stat *dst, struct thread_stat *src)
 		dst->io_u_complete[i]	= le32_to_cpu(src->io_u_complete[i]);
 	}
 
-	for (i = 0; i < FIO_IO_U_LAT_U_NR; i++) {
+	for (i = 0; i < FIO_IO_U_LAT_U_NR; i++)
 		dst->io_u_lat_u[i]	= le32_to_cpu(src->io_u_lat_u[i]);
+	for (i = 0; i < FIO_IO_U_LAT_M_NR; i++)
 		dst->io_u_lat_m[i]	= le32_to_cpu(src->io_u_lat_m[i]);
-	}
 
 	for (i = 0; i < DDIR_RWDIR_CNT; i++)
 		for (j = 0; j < FIO_IO_U_PLAT_NR; j++)
diff --git a/server.c b/server.c
index 1b3bc30..1e269c2 100644
--- a/server.c
+++ b/server.c
@@ -1497,10 +1497,10 @@ void fio_server_send_ts(struct thread_stat *ts, struct group_run_stats *rs)
 		p.ts.io_u_complete[i]	= cpu_to_le32(ts->io_u_complete[i]);
 	}
 
-	for (i = 0; i < FIO_IO_U_LAT_U_NR; i++) {
+	for (i = 0; i < FIO_IO_U_LAT_U_NR; i++)
 		p.ts.io_u_lat_u[i]	= cpu_to_le32(ts->io_u_lat_u[i]);
+	for (i = 0; i < FIO_IO_U_LAT_M_NR; i++)
 		p.ts.io_u_lat_m[i]	= cpu_to_le32(ts->io_u_lat_m[i]);
-	}
 
 	for (i = 0; i < DDIR_RWDIR_CNT; i++)
 		for (j = 0; j < FIO_IO_U_PLAT_NR; j++)
-- 
2.7.4

[PATCH 6/6] gettime: make utime_since_now and mtime_since_now consistent in how they record the caller and put this all behind FIO_DEBUG_TIME

[Vincent Fu]

From: Vincent Fu <Vincent.Fu@sandisk.com>

---
 gettime.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/gettime.c b/gettime.c
index 85ba7cb..628aad6 100644
--- a/gettime.c
+++ b/gettime.c
@@ -402,8 +402,14 @@ uint64_t utime_since(const struct timeval *s, const struct timeval *e)
 uint64_t utime_since_now(const struct timeval *s)
 {
 	struct timeval t;
+#ifdef FIO_DEBUG_TIME
+	void *p = __builtin_return_address(0);
 
+	fio_gettime(&t, p);
+#else
 	fio_gettime(&t, NULL);
+#endif
+
 	return utime_since(s, &t);
 }
 
@@ -429,9 +435,14 @@ uint64_t mtime_since(const struct timeval *s, const struct timeval *e)
 uint64_t mtime_since_now(const struct timeval *s)
 {
 	struct timeval t;
+#ifdef FIO_DEBUG_TIME
 	void *p = __builtin_return_address(0);
 
 	fio_gettime(&t, p);
+#else
+	fio_gettime(&t, NULL);
+#endif
+
 	return mtime_since(s, &t);
 }
 
-- 
2.7.4

Re: [PATCH 1/6] stat: change json+ output format so that instead of printing the raw clat data structure, use actual durations instead of array indices and print only bins with nonzero counts

[Vincent Fu]

Jens, can you review this patch series?

The json+ change (PATCH 1/6 and 2/6) was a request from a user and makes 
the output easier to use.

The other patches fix problems with resetting the latency counters when 
--latency-target is used (PATCH 4/6) and make sure some forgotten 
latency counters are transferred in client/server mode (PATCH 5/6). 
PATCH 6/6 is a cleanup. You can ignore PATCH 3/6.

Vincent

On 17-Apr-17 14:11, Vincent Fu wrote:
> From: Vincent Fu<Vincent.Fu@sandisk.com>
>
> ---
>   stat.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/stat.c b/stat.c
> index fde7af2..4a6e1b9 100644
> --- a/stat.c
> +++ b/stat.c
> @@ -98,7 +98,7 @@ static unsigned int plat_val_to_idx(unsigned int val)
>    * Convert the given index of the bucket array to the value
>    * represented by the bucket
>    */
> -static unsigned int plat_idx_to_val(unsigned int idx)
> +static unsigned long long plat_idx_to_val(unsigned int idx)
>   {
>   	unsigned int error_bits, k, base;
>   
> @@ -972,12 +972,11 @@ static void add_ddir_status_json(struct thread_stat *ts,
>   		clat_bins_object = json_create_object();
>   		json_object_add_value_object(tmp_object, "bins", clat_bins_object);
>   		for(i = 0; i < FIO_IO_U_PLAT_NR; i++) {
> -			snprintf(buf, sizeof(buf), "%d", i);
> -			json_object_add_value_int(clat_bins_object, (const char *)buf, ts->io_u_plat[ddir][i]);
> +			if (ts->io_u_plat[ddir][i]) {
> +				snprintf(buf, sizeof(buf), "%llu", plat_idx_to_val(i));
> +				json_object_add_value_int(clat_bins_object, (const char *)buf, ts->io_u_plat[ddir][i]);
> +			}
>   		}
> -		json_object_add_value_int(clat_bins_object, "FIO_IO_U_PLAT_BITS", FIO_IO_U_PLAT_BITS);
> -		json_object_add_value_int(clat_bins_object, "FIO_IO_U_PLAT_VAL", FIO_IO_U_PLAT_VAL);
> -		json_object_add_value_int(clat_bins_object, "FIO_IO_U_PLAT_NR", FIO_IO_U_PLAT_NR);
>   	}
>   
>   	if (!calc_lat(&ts->lat_stat[ddir], &min, &max, &mean, &dev)) {

Re: [PATCH 1/6] stat: change json+ output format so that instead of printing the raw clat data structure, use actual durations instead of array indices and print only bins with nonzero counts

[Jens Axboe]

On Wed, May 03 2017, Vincent Fu wrote:
> Jens, can you review this patch series?
> 
> The json+ change (PATCH 1/6 and 2/6) was a request from a user and makes the
> output easier to use.
> 
> The other patches fix problems with resetting the latency counters when
> --latency-target is used (PATCH 4/6) and make sure some forgotten latency
> counters are transferred in client/server mode (PATCH 5/6). PATCH 6/6 is a
> cleanup. You can ignore PATCH 3/6.

Looks good to me, this got lost since it arrived while I was away on
vacation. I applied the series, skipping patch #3. Thanks Vincent!

-- 
Jens Axboe