Skip to contentSkip to conversation

fio: few enhancements

PATCH In review Luis Chamberlain 2 participants 4 messages Aug 21, 2018 – Aug 31, 2018

lore
Share & tools
lore
#0 lore raw

cover fio: few enhancements

I've been toying around with fio's output for a bit, mostly because Intel's fiovisualizer [0] was the first graphing tool I found for fio, even though gfio exists. The way fiovisualizer works is it reads terse ouptut, parses that, and plots data. gfio on the other hand is a proper built-in client, where communication is exchanged between client/server and the client plots the data.

If you end up using scraper for terse data such as fiovisualizer with the prospect to scrape data you will end up with different results if you use a remote server as a backend.

While studying fio code I realized that the way fiovisualizer implements graphing is perhaps the wrong / least optimal approach, but also that fio terse output needed a bit of love for client/server relationship to keep fidelity in terms of the expected output.

These are just two small enhancements which I hope wil make things clearer.

[0] https://github.com/intel/fiovisualizer

Luis Chamberlain (2):
client: respect terse output on client <--> backend relationship
init: add semantics for all types of backends running

 backend.c  |  2 ++
 client.c   | 16 +++++++++++++---
 diskutil.c |  2 +-
 fio.h      |  8 ++++++++
 init.c     |  1 +
 stat.c     |  4 +++-
 6 files changed, 28 insertions(+), 5 deletions(-)
Signature · 1 line
2.18.0
#1 lore raw

PATCH 1/2 client: respect terse output on client <--> backend relationship

You end up with different results if you run this terse output on a local system which also runs its own backend Vs running a client to connect to a remote server which is running fio as a backend only. The reason is the client ops handle printing of threads / disk utils separately. The terse output created *by* the backend is the right and expected output, so just use that, and we can piggy back off of the fact that the server will send its own output via FIO_NET_CMD_TEXT.

Another solution is to address getting the disk util data sent to be cached locally, and then upon handle_ts() print that, but that would require significant re-architecturing.

Terse output is supposed to be just that, terse. This implies that it will not be clear from what backend data came from, but for this the best strategy is to *extend* the terse version with yet another field, the remote hostname/ip address.

---
 client.c | 16 +++++++++++++---
 stat.c   |  2 ++
 2 files changed, 15 insertions(+), 3 deletions(-)
client.c +13 -3
index e2525c81..bc0275b6 100644
@@ -1058,6 +1058,9 @@ static void handle_ts(struct fio_client *client, struct fio_net_cmd *cmd)
 	struct flist_head *opt_list = NULL;
 	struct json_object *tsobj;
 
+	if (output_format & FIO_OUTPUT_TERSE)
+		return;
+
 	if (client->opt_lists && p->ts.thread_number <= client->jobs)
 		opt_list = &client->opt_lists[p->ts.thread_number - 1];
 
@@ -1094,6 +1097,9 @@ static void handle_gs(struct fio_client *client, struct fio_net_cmd *cmd)
 {
 	struct group_run_stats *gs = (struct group_run_stats *) cmd->payload;
 
+	if (output_format & FIO_OUTPUT_TERSE)
+		return;
+
 	if (output_format & FIO_OUTPUT_NORMAL)
 		show_group_stats(gs, NULL);
 }
@@ -1140,7 +1146,7 @@ static void handle_text(struct fio_client *client, struct fio_net_cmd *cmd)
 
 	name = client->name ? client->name : client->hostname;
 
-	if (!client->skip_newline)
+	if (!client->skip_newline && !(output_format & FIO_OUTPUT_TERSE))
 		fprintf(f_out, "<%s> ", name);
 	ret = fwrite(buf, pdu->buf_len, 1, f_out);
 	fflush(f_out);
@@ -1184,6 +1190,9 @@ static void handle_du(struct fio_client *client, struct fio_net_cmd *cmd)
 {
 	struct cmd_du_pdu *du = (struct cmd_du_pdu *) cmd->payload;
 
+	if (output_format & FIO_OUTPUT_TERSE)
+		return;
+
 	if (!client->disk_stats_shown) {
 		client->disk_stats_shown = true;
 		log_info("\nDisk stats (read/write):\n");
@@ -1195,8 +1204,6 @@ static void handle_du(struct fio_client *client, struct fio_net_cmd *cmd)
 		duobj = json_array_last_value_object(du_array);
 		json_object_add_client_info(duobj, client);
 	}
-	if (output_format & FIO_OUTPUT_TERSE)
-		print_disk_util(&du->dus, &du->agg, 1, NULL);
 	if (output_format & FIO_OUTPUT_NORMAL)
 		print_disk_util(&du->dus, &du->agg, 0, NULL);
 }
@@ -1456,6 +1463,9 @@ static void handle_probe(struct fio_client *client, struct fio_net_cmd *cmd)
 	const char *os, *arch;
 	char bit[16];
 
+	if (output_format & FIO_OUTPUT_TERSE)
+		return;
+
 	os = fio_get_os_string(probe->os);
 	if (!os)
 		os = "unknown";
stat.c +2 -0
index 82e79dfc..edf9ecf6 100644
@@ -1922,6 +1922,8 @@ void __show_run_stats(void)
 		if (is_backend) {
 			fio_server_send_job_options(opt_lists[i], i);
 			fio_server_send_ts(ts, rs);
+			if (output_format & FIO_OUTPUT_TERSE)
+				show_thread_status_terse(ts, rs, &output[__FIO_OUTPUT_TERSE]);
 		} else {
 			if (output_format & FIO_OUTPUT_TERSE)
 				show_thread_status_terse(ts, rs, &output[__FIO_OUTPUT_TERSE]);
Signature · 1 line
2.18.0
#2 lore raw

Re: [PATCH 1/2] client: respect terse output on client <--> backend relationship

Quoted 19 lines · Luis Chamberlain

You end up with different results if you run this terse output on a local system which also runs its own backend Vs running a client to connect to a remote server which is running fio as a backend only. The reason is the client ops handle printing of threads / disk utils separately. The terse output created *by* the backend is the right and expected output, so just use that, and we can piggy back off of the fact that the server will send its own output via FIO_NET_CMD_TEXT.

Another solution is to address getting the disk util data sent to be cached locally, and then upon handle_ts() print that, but that would require significant re-architecturing.

Terse output is supposed to be just that, terse. This implies that it will not be clear from what backend data came from, but for this the best strategy is to *extend* the terse version with yet another field, the remote hostname/ip address.

[...]

stat.c void __show_run_stats(void) · line 1926

Quoted 10 lines
stat.c +2 -0
index 82e79dfc..edf9ecf6 100644
@@ -1922,6 +1922,8 @@ void __show_run_stats(void)
                if (is_backend) {
                        fio_server_send_job_options(opt_lists[i], i);
                        fio_server_send_ts(ts, rs);
+                       if (output_format & FIO_OUTPUT_TERSE)
+                               show_thread_status_terse(ts, rs, &output[__FIO_OUTPUT_TERSE]);

What stops the extra disk usage output being suppressed in the sub-calls from show_thread_status_terse? Were all the early returns just stop the client requesting things like disk stats periodically and to just let the servers send it at the end?

Signature · 1 line
Sitsofe | http://sucs.org/~sits/
#3 lore raw

PATCH 2/2 init: add semantics for all types of backends running

In reply to Luis Chamberlain

Commit 67a176fc7 ("Fix segfault with client/server and minimal output") addressed a segfault but it wasn't very clear *why* the fix was needed. The reason it was needed is that the disk util and its respective semaphore are only initialized when setup_disk_util() is called, this happens upon fio_backend() calls. That is, either we have a dedicated backend or have initiated a backend for the localhost due to some local work. And show_thread_status() is currently called from a complete client setup -- handle_ts() calls show_thread_status(), and a client does not collect any local disk data, it receives this from the backend. As such, the semaphore won't be setup in a client setup and this is why we segfault here.

We can enable show_thread_status() then only if any type of backend is running, however there is are no semantics currently which enable us to query for this. Add such semantics and replace the previous check with a check for if fio_backend() was ever called. This will make it clearer.

---
 backend.c  | 2 ++
 diskutil.c | 2 +-
 fio.h      | 8 ++++++++
 init.c     | 1 +
 stat.c     | 2 +-
 5 files changed, 13 insertions(+), 2 deletions(-)
backend.c +2 -0
index 36bde6a5..8fec1ce3 100644
@@ -2481,6 +2481,8 @@ int fio_backend(struct sk_out *sk_out)
 	}
 
 	startup_sem = fio_sem_init(FIO_SEM_LOCKED);
+	if (!sk_out)
+		is_local_backend = true;
 	if (startup_sem == NULL)
 		return 1;
 
diskutil.c +1 -1
index 5b4eb46d..7be4c022 100644
@@ -701,7 +701,7 @@ void show_disk_util(int terse, struct json_object *parent,
 	struct disk_util *du;
 	bool do_json;
 
-	if (!disk_util_sem)
+	if (!is_running_backend())
 		return;
 
 	fio_sem_down(disk_util_sem);
fio.h +8 -0
index b58057f7..ce3583cb 100644
@@ -522,6 +522,7 @@ extern int fio_clock_source_set;
 extern int warnings_fatal;
 extern int terse_version;
 extern int is_backend;
+extern int is_local_backend;
 extern int nr_clients;
 extern int log_syslog;
 extern int status_interval;
@@ -534,6 +535,13 @@ extern char *aux_path;
 
 extern struct thread_data *threads;
 
+static inline bool is_running_backend(void)
+{
+	if (is_backend || is_local_backend)
+		return true;
+	return false;
+}
+
 extern bool eta_time_within_slack(unsigned int time);
 
 static inline void fio_ro_check(const struct thread_data *td, struct io_u *io_u)
init.c +1 -0
index 06f69719..3ed57570 100644
@@ -63,6 +63,7 @@ char *exec_profile = NULL;
 int warnings_fatal = 0;
 int terse_version = 3;
 int is_backend = 0;
+int is_local_backend = 0;
 int nr_clients = 0;
 int log_syslog = 0;
 
stat.c +1 -1
index edf9ecf6..6cb704eb 100644
@@ -1205,7 +1205,7 @@ static void show_thread_status_terse_all(struct thread_stat *ts,
 		log_buf(out, ";%3.2f%%", io_u_lat_m[i]);
 
 	/* disk util stats, if any */
-	if (ver >= 3)
+	if (ver >= 3 && is_running_backend())
 		show_disk_util(1, NULL, out);
 
 	/* Additional output if continue_on_error set - default off*/
Signature · 1 line
2.18.0