All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/7] Seven fio patches
@ 2019-08-14 20:10 Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 1/7] zbd: Declare local functions 'static' Bart Van Assche
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 20:10 UTC (permalink / raw
  To: Jens Axboe; +Cc: fio, Bart Van Assche

Hi Jens,

The following changes are included in this patch series:
- Declare local functions 'static' in zbd.c.
- Make the zbd test scripts more robust.
- Use snprintf() instead of strncpy() + explicit null termination.
- Rework the approach for suppressing gcc 9 address of packed member warnings.
- Add a 'fulltest' target that runs the zbd tests.

Changes compared to v2:
- Changed BUILD_BUG_ON(x) into compiletime_assert(!x).

Changes compared to v1:
- Two new patches have been added and the patch for making Travis run the ZBD
  tests has been left out. That patch had been prepared considerable time ago
  and I have not yet figured out how to load the null_blk kernel module today
  in a Travis environment.

Please consider these patches for the official fio git repository.

Thanks,

Bart.

Bart Van Assche (7):
  zbd: Declare local functions 'static'
  zbd: Improve robustness of unit tests
  Optimize the code that copies strings
  Refine packed annotations in stat.h
  Verify the absence of holes in struct jobs_eta at compile time
  Restore type checking in calc_thread_status()
  Makefile: Add 'fulltest' target

 Makefile                            | 17 +++++++++
 cconv.c                             |  7 ++--
 client.c                            |  5 +--
 diskutil.c                          |  9 +++--
 engines/net.c                       |  6 ++--
 engines/sg.c                        |  4 +--
 eta.c                               | 12 +++----
 filesetup.c                         |  6 ++--
 gclient.c                           |  4 +--
 init.c                              | 19 +++-------
 ioengines.c                         |  3 +-
 options.c                           |  3 +-
 parse.c                             |  6 ++--
 server.c                            | 26 ++++++--------
 stat.c                              | 15 ++++----
 stat.h                              | 54 ++++++++++++++++-------------
 t/zbd/run-tests-against-zoned-nullb |  2 +-
 t/zbd/test-zbd-support              |  4 +--
 verify.c                            |  3 +-
 zbd.c                               |  6 ++--
 20 files changed, 105 insertions(+), 106 deletions(-)

-- 
2.22.0.rc1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/7] zbd: Declare local functions 'static'
  2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
@ 2019-08-14 20:10 ` Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 2/7] zbd: Improve robustness of unit tests Bart Van Assche
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 20:10 UTC (permalink / raw
  To: Jens Axboe; +Cc: fio, Bart Van Assche

This patch fixes two sparse warnings.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 zbd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/zbd.c b/zbd.c
index d7e91e37e010..2383c57dfc90 100644
--- a/zbd.c
+++ b/zbd.c
@@ -481,7 +481,7 @@ out:
  *
  * Returns 0 upon success and a negative error code upon failure.
  */
-int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
+static int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
 {
 	enum blk_zoned_model zbd_model;
 	int ret = 0;
@@ -933,8 +933,8 @@ static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
  * a multiple of the fio block size. The caller must neither hold z->mutex
  * nor f->zbd_info->mutex. Returns with z->mutex held upon success.
  */
-struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
-					       struct io_u *io_u)
+static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
+						      struct io_u *io_u)
 {
 	const uint32_t min_bs = td->o.min_bs[io_u->ddir];
 	const struct fio_file *f = io_u->file;
-- 
2.22.0.rc1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/7] zbd: Improve robustness of unit tests
  2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 1/7] zbd: Declare local functions 'static' Bart Van Assche
@ 2019-08-14 20:10 ` Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 3/7] Optimize the code that copies strings Bart Van Assche
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 20:10 UTC (permalink / raw
  To: Jens Axboe; +Cc: fio, Bart Van Assche

Give up if creation of the null_blk instance fails.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 t/zbd/run-tests-against-zoned-nullb | 2 +-
 t/zbd/test-zbd-support              | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/zbd/run-tests-against-zoned-nullb b/t/zbd/run-tests-against-zoned-nullb
index 9336716d4762..0952011c576e 100755
--- a/t/zbd/run-tests-against-zoned-nullb
+++ b/t/zbd/run-tests-against-zoned-nullb
@@ -24,6 +24,6 @@ modprobe null_blk nr_devices=0 &&
     echo 4096 > blocksize &&
     echo 1024 > size &&
     echo 1 > memory_backed &&
-    echo 1 > power
+    echo 1 > power || exit $?
 
 "${scriptdir}"/test-zbd-support "$@" /dev/nullb0
diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 6fb48ef091cf..ed54a0aa21fd 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -772,8 +772,8 @@ source "$(dirname "$0")/functions" || exit $?
 dev=$1
 realdev=$(readlink -f "$dev")
 basename=$(basename "$realdev")
-major=$((0x$(stat -L -c '%t' "$realdev")))
-minor=$((0x$(stat -L -c '%T' "$realdev")))
+major=$((0x$(stat -L -c '%t' "$realdev"))) || exit $?
+minor=$((0x$(stat -L -c '%T' "$realdev"))) || exit $?
 disk_size=$(($(<"/sys/dev/block/$major:$minor/size")*512))
 # When the target is a partition device, get basename of its holder device to
 # access sysfs path of the holder device
-- 
2.22.0.rc1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/7] Optimize the code that copies strings
  2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 1/7] zbd: Declare local functions 'static' Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 2/7] zbd: Improve robustness of unit tests Bart Van Assche
@ 2019-08-14 20:10 ` Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 4/7] Refine packed annotations in stat.h Bart Van Assche
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 20:10 UTC (permalink / raw
  To: Jens Axboe; +Cc: fio, Bart Van Assche, Damien Le Moal

Using strncpy() to copy strings is suboptimal because strncpy writes a
bunch of additional unnecessary null bytes. Use snprintf() instead of
strncpy(). An additional advantage of snprintf() is that it guarantees
that the output string is '\0'-terminated.

This patch is an improvement for commit 32e31c8c5f7b ("Fix string copy
compilation warnings").

Cc: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 cconv.c       |  7 +++----
 client.c      |  5 +++--
 diskutil.c    |  9 ++++-----
 engines/net.c |  6 ++----
 engines/sg.c  |  4 ++--
 filesetup.c   |  6 ++----
 gclient.c     |  4 ++--
 init.c        | 19 +++++--------------
 ioengines.c   |  3 +--
 options.c     |  3 +--
 parse.c       |  6 ++----
 server.c      | 26 +++++++++++---------------
 stat.c        | 15 ++++++++-------
 verify.c      |  3 +--
 14 files changed, 47 insertions(+), 69 deletions(-)

diff --git a/cconv.c b/cconv.c
index 50e45c63a636..0e6572462788 100644
--- a/cconv.c
+++ b/cconv.c
@@ -13,10 +13,9 @@ static void string_to_cpu(char **dst, const uint8_t *src)
 
 static void __string_to_net(uint8_t *dst, const char *src, size_t dst_size)
 {
-	if (src) {
-		dst[dst_size - 1] = '\0';
-		strncpy((char *) dst, src, dst_size - 1);
-	} else
+	if (src)
+		snprintf((char *) dst, dst_size, "%s", src);
+	else
 		dst[0] = '\0';
 }
 
diff --git a/client.c b/client.c
index 43cfbd43b9d3..e0047af06a3e 100644
--- a/client.c
+++ b/client.c
@@ -520,7 +520,7 @@ static void probe_client(struct fio_client *client)
 
 	sname = server_name(client, buf, sizeof(buf));
 	memset(pdu.server, 0, sizeof(pdu.server));
-	strncpy((char *) pdu.server, sname, sizeof(pdu.server) - 1);
+	snprintf((char *) pdu.server, sizeof(pdu.server), "%s", sname);
 
 	fio_net_send_cmd(client->fd, FIO_NET_CMD_PROBE, &pdu, sizeof(pdu), &tag, &client->cmd_list);
 }
@@ -574,7 +574,8 @@ static int fio_client_connect_sock(struct fio_client *client)
 
 	memset(addr, 0, sizeof(*addr));
 	addr->sun_family = AF_UNIX;
-	strncpy(addr->sun_path, client->hostname, sizeof(addr->sun_path) - 1);
+	snprintf(addr->sun_path, sizeof(addr->sun_path), "%s",
+		 client->hostname);
 
 	fd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (fd < 0) {
diff --git a/diskutil.c b/diskutil.c
index 7be4c022431e..f074401501ba 100644
--- a/diskutil.c
+++ b/diskutil.c
@@ -181,8 +181,7 @@ static int get_device_numbers(char *file_name, int *maj, int *min)
 		/*
 		 * must be a file, open "." in that path
 		 */
-		tempname[PATH_MAX - 1] = '\0';
-		strncpy(tempname, file_name, PATH_MAX - 1);
+		snprintf(tempname, ARRAY_SIZE(tempname), "%s", file_name);
 		p = dirname(tempname);
 		if (stat(p, &st)) {
 			perror("disk util stat");
@@ -314,7 +313,8 @@ static struct disk_util *disk_util_add(struct thread_data *td, int majdev,
 		sfree(du);
 		return NULL;
 	}
-	strncpy((char *) du->dus.name, basename(path), FIO_DU_NAME_SZ - 1);
+	snprintf((char *) du->dus.name, ARRAY_SIZE(du->dus.name), "%s",
+		 basename(path));
 	du->sysfs_root = strdup(path);
 	du->major = majdev;
 	du->minor = mindev;
@@ -435,8 +435,7 @@ static struct disk_util *__init_per_file_disk_util(struct thread_data *td,
 			log_err("unknown sysfs layout\n");
 			return NULL;
 		}
-		tmp[PATH_MAX - 1] = '\0';
-		strncpy(tmp, p, PATH_MAX - 1);
+		snprintf(tmp, ARRAY_SIZE(tmp), "%s", p);
 		sprintf(path, "%s", tmp);
 	}
 
diff --git a/engines/net.c b/engines/net.c
index ca6fb344b897..91f25774690a 100644
--- a/engines/net.c
+++ b/engines/net.c
@@ -1105,8 +1105,7 @@ static int fio_netio_setup_connect_unix(struct thread_data *td,
 	struct sockaddr_un *soun = &nd->addr_un;
 
 	soun->sun_family = AF_UNIX;
-	memset(soun->sun_path, 0, sizeof(soun->sun_path));
-	strncpy(soun->sun_path, path, sizeof(soun->sun_path) - 1);
+	snprintf(soun->sun_path, sizeof(soun->sun_path), "%s", path);
 	return 0;
 }
 
@@ -1135,9 +1134,8 @@ static int fio_netio_setup_listen_unix(struct thread_data *td, const char *path)
 
 	mode = umask(000);
 
-	memset(addr, 0, sizeof(*addr));
 	addr->sun_family = AF_UNIX;
-	strncpy(addr->sun_path, path, sizeof(addr->sun_path) - 1);
+	snprintf(addr->sun_path, sizeof(addr->sun_path), "%s", path);
 	unlink(path);
 
 	len = sizeof(addr->sun_family) + strlen(path) + 1;
diff --git a/engines/sg.c b/engines/sg.c
index c46b9abaec87..a1a6de4ce248 100644
--- a/engines/sg.c
+++ b/engines/sg.c
@@ -1181,8 +1181,8 @@ static char *fio_sgio_errdetails(struct io_u *io_u)
 	}
 
 	if (!(hdr->info & SG_INFO_CHECK) && !strlen(msg))
-		strncpy(msg, "SG Driver did not report a Host, Driver or Device check",
-			MAXERRDETAIL - 1);
+		snprintf(msg, MAXERRDETAIL, "%s",
+			 "SG Driver did not report a Host, Driver or Device check");
 
 	return msg;
 }
diff --git a/filesetup.c b/filesetup.c
index 17fa31fb3cd4..57eca1bf5bf0 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -805,8 +805,7 @@ static unsigned long long get_fs_free_counts(struct thread_data *td)
 		} else if (f->filetype != FIO_TYPE_FILE)
 			continue;
 
-		buf[255] = '\0';
-		strncpy(buf, f->file_name, 255);
+		snprintf(buf, ARRAY_SIZE(buf), "%s", f->file_name);
 
 		if (stat(buf, &sb) < 0) {
 			if (errno != ENOENT)
@@ -829,8 +828,7 @@ static unsigned long long get_fs_free_counts(struct thread_data *td)
 			continue;
 
 		fm = calloc(1, sizeof(*fm));
-		strncpy(fm->__base, buf, sizeof(fm->__base));
-		fm->__base[255] = '\0'; 
+		snprintf(fm->__base, ARRAY_SIZE(fm->__base), "%s", buf);
 		fm->base = basename(fm->__base);
 		fm->key = sb.st_dev;
 		flist_add(&fm->list, &list);
diff --git a/gclient.c b/gclient.c
index 04275a1384c2..64324177ef1a 100644
--- a/gclient.c
+++ b/gclient.c
@@ -318,7 +318,7 @@ static void gfio_update_thread_status(struct gui_entry *ge,
 	static char message[100];
 	const char *m = message;
 
-	strncpy(message, status_message, sizeof(message) - 1);
+	snprintf(message, sizeof(message), "%s", status_message);
 	gtk_progress_bar_set_text(GTK_PROGRESS_BAR(ge->thread_status_pb), m);
 	gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(ge->thread_status_pb), perc / 100.0);
 	gtk_widget_queue_draw(ge->ui->window);
@@ -330,7 +330,7 @@ static void gfio_update_thread_status_all(struct gui *ui, char *status_message,
 	static char message[100];
 	const char *m = message;
 
-	strncpy(message, status_message, sizeof(message) - 1);
+	strncpy(message, sizeof(message), "%s", status_message);
 	gtk_progress_bar_set_text(GTK_PROGRESS_BAR(ui->thread_status_pb), m);
 	gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(ui->thread_status_pb), perc / 100.0);
 	gtk_widget_queue_draw(ui->window);
diff --git a/init.c b/init.c
index c9f6198ea63a..63f2168eabcb 100644
--- a/init.c
+++ b/init.c
@@ -1273,8 +1273,7 @@ static char *make_filename(char *buf, size_t buf_size,struct thread_options *o,
 	for (f = &fpre_keywords[0]; f->keyword; f++)
 		f->strlen = strlen(f->keyword);
 
-	buf[buf_size - 1] = '\0';
-	strncpy(buf, o->filename_format, buf_size - 1);
+	snprintf(buf, buf_size, "%s", o->filename_format);
 
 	memset(copy, 0, sizeof(copy));
 	for (f = &fpre_keywords[0]; f->keyword; f++) {
@@ -1353,7 +1352,7 @@ static char *make_filename(char *buf, size_t buf_size,struct thread_options *o,
 			if (post_start)
 				strncpy(dst, buf + post_start, dst_left);
 
-			strncpy(buf, copy, buf_size - 1);
+			snprintf(buf, buf_size, "%s", copy);
 		} while (1);
 	}
 
@@ -2029,20 +2028,12 @@ static int __parse_jobs_ini(struct thread_data *td,
 				 */
 				if (access(filename, F_OK) &&
 				    (ts = strrchr(file, '/'))) {
-					int len = ts - file +
-						strlen(filename) + 2;
-
-					if (!(full_fn = calloc(1, len))) {
+					if (asprintf(&full_fn, "%.*s%s",
+						 (int)(ts - file + 1), file,
+						 filename) < 0) {
 						ret = ENOMEM;
 						break;
 					}
-
-					strncpy(full_fn,
-						file, (ts - file) + 1);
-					strncpy(full_fn + (ts - file) + 1,
-						filename,
-						len - (ts - file) - 1);
-					full_fn[len - 1] = 0;
 					filename = full_fn;
 				}
 
diff --git a/ioengines.c b/ioengines.c
index aa4ccd2755c9..40fa75c382b4 100644
--- a/ioengines.c
+++ b/ioengines.c
@@ -125,8 +125,7 @@ static struct ioengine_ops *__load_ioengine(const char *name)
 {
 	char engine[64];
 
-	engine[sizeof(engine) - 1] = '\0';
-	strncpy(engine, name, sizeof(engine) - 1);
+	snprintf(engine, sizeof(engine), "%s", name);
 
 	/*
 	 * linux libaio has alias names, so convert to what we want
diff --git a/options.c b/options.c
index f4c9bedf377f..447f231e7148 100644
--- a/options.c
+++ b/options.c
@@ -4902,8 +4902,7 @@ char *fio_option_dup_subs(const char *opt)
 		return NULL;
 	}
 
-	in[OPT_LEN_MAX] = '\0';
-	strncpy(in, opt, OPT_LEN_MAX);
+	snprintf(in, sizeof(in), "%s", opt);
 
 	while (*inptr && nchr > 0) {
 		if (inptr[0] == '$' && inptr[1] == '{') {
diff --git a/parse.c b/parse.c
index a7d4516e4702..c4fd4626df9f 100644
--- a/parse.c
+++ b/parse.c
@@ -602,8 +602,7 @@ static int __handle_option(const struct fio_option *o, const char *ptr,
 		if (!is_time && o->is_time)
 			is_time = o->is_time;
 
-		tmp[sizeof(tmp) - 1] = '\0';
-		strncpy(tmp, ptr, sizeof(tmp) - 1);
+		snprintf(tmp, sizeof(tmp), "%s", ptr);
 		p = strchr(tmp, ',');
 		if (p)
 			*p = '\0';
@@ -829,8 +828,7 @@ static int __handle_option(const struct fio_option *o, const char *ptr,
 		char tmp[128];
 		char *p1, *p2;
 
-		tmp[sizeof(tmp) - 1] = '\0';
-		strncpy(tmp, ptr, sizeof(tmp) - 1);
+		snprintf(tmp, sizeof(tmp), "%s", ptr);
 
 		/* Handle bsrange with separate read,write values: */
 		p1 = strchr(tmp, ',');
diff --git a/server.c b/server.c
index 23e549a590a7..e7846227f2b1 100644
--- a/server.c
+++ b/server.c
@@ -865,7 +865,8 @@ static int handle_probe_cmd(struct fio_net_cmd *cmd)
 	strcpy(me, (char *) pdu->server);
 
 	gethostname((char *) probe.hostname, sizeof(probe.hostname));
-	strncpy((char *) probe.fio_version, fio_version_string, sizeof(probe.fio_version) - 1);
+	snprintf((char *) probe.fio_version, sizeof(probe.fio_version), "%s",
+		 fio_version_string);
 
 	/*
 	 * If the client supports compression and we do too, then enable it
@@ -1470,12 +1471,10 @@ void fio_server_send_ts(struct thread_stat *ts, struct group_run_stats *rs)
 
 	memset(&p, 0, sizeof(p));
 
-	strncpy(p.ts.name, ts->name, FIO_JOBNAME_SIZE);
-	p.ts.name[FIO_JOBNAME_SIZE - 1] = '\0';
-	strncpy(p.ts.verror, ts->verror, FIO_VERROR_SIZE);
-	p.ts.verror[FIO_VERROR_SIZE - 1] = '\0';
-	strncpy(p.ts.description, ts->description, FIO_JOBDESC_SIZE);
-	p.ts.description[FIO_JOBDESC_SIZE - 1] = '\0';
+	snprintf(p.ts.name, sizeof(p.ts.name), "%s", ts->name);
+	snprintf(p.ts.verror, sizeof(p.ts.verror), "%s", ts->verror);
+	snprintf(p.ts.description, sizeof(p.ts.description), "%s",
+		 ts->description);
 
 	p.ts.error		= cpu_to_le32(ts->error);
 	p.ts.thread_number	= cpu_to_le32(ts->thread_number);
@@ -1666,8 +1665,7 @@ static void convert_dus(struct disk_util_stat *dst, struct disk_util_stat *src)
 {
 	int i;
 
-	dst->name[FIO_DU_NAME_SZ - 1] = '\0';
-	strncpy((char *) dst->name, (char *) src->name, FIO_DU_NAME_SZ - 1);
+	snprintf((char *) dst->name, sizeof(dst->name), "%s", src->name);
 
 	for (i = 0; i < 2; i++) {
 		dst->s.ios[i]		= cpu_to_le64(src->s.ios[i]);
@@ -1977,8 +1975,7 @@ int fio_send_iolog(struct thread_data *td, struct io_log *log, const char *name)
 	else
 		pdu.compressed = 0;
 
-	strncpy((char *) pdu.name, name, FIO_NET_NAME_MAX);
-	pdu.name[FIO_NET_NAME_MAX - 1] = '\0';
+	snprintf((char *) pdu.name, sizeof(pdu.name), "%s", name);
 
 	/*
 	 * We can't do this for a pre-compressed log, but for that case,
@@ -2195,9 +2192,8 @@ static int fio_init_server_sock(void)
 
 	mode = umask(000);
 
-	memset(&addr, 0, sizeof(addr));
 	addr.sun_family = AF_UNIX;
-	strncpy(addr.sun_path, bind_sock, sizeof(addr.sun_path) - 1);
+	snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", bind_sock);
 
 	len = sizeof(addr.sun_family) + strlen(bind_sock) + 1;
 
@@ -2247,9 +2243,9 @@ static int fio_init_server_connection(void)
 		if (p)
 			strcat(p, port);
 		else
-			strncpy(bind_str, port, sizeof(bind_str) - 1);
+			snprintf(bind_str, sizeof(bind_str), "%s", port);
 	} else
-		strncpy(bind_str, bind_sock, sizeof(bind_str) - 1);
+		snprintf(bind_str, sizeof(bind_str), "%s", bind_sock);
 
 	log_info("fio: server listening on %s\n", bind_str);
 
diff --git a/stat.c b/stat.c
index bf87917c2956..33637900df62 100644
--- a/stat.c
+++ b/stat.c
@@ -1828,10 +1828,11 @@ void __show_run_stats(void)
 			/*
 			 * These are per-group shared already
 			 */
-			strncpy(ts->name, td->o.name, FIO_JOBNAME_SIZE - 1);
+			snprintf(ts->name, sizeof(ts->name), "%s", td->o.name);
 			if (td->o.description)
-				strncpy(ts->description, td->o.description,
-						FIO_JOBDESC_SIZE - 1);
+				snprintf(ts->description,
+					 sizeof(ts->description), "%s",
+					 td->o.description);
 			else
 				memset(ts->description, 0, FIO_JOBDESC_SIZE);
 
@@ -1868,12 +1869,12 @@ void __show_run_stats(void)
 			if (!td->error && td->o.continue_on_error &&
 			    td->first_error) {
 				ts->error = td->first_error;
-				ts->verror[sizeof(ts->verror) - 1] = '\0';
-				strncpy(ts->verror, td->verror, sizeof(ts->verror) - 1);
+				snprintf(ts->verror, sizeof(ts->verror), "%s",
+					 td->verror);
 			} else  if (td->error) {
 				ts->error = td->error;
-				ts->verror[sizeof(ts->verror) - 1] = '\0';
-				strncpy(ts->verror, td->verror, sizeof(ts->verror) - 1);
+				snprintf(ts->verror, sizeof(ts->verror), "%s",
+					 td->verror);
 			}
 		}
 
diff --git a/verify.c b/verify.c
index f79ab43aab6b..48ba051da3ad 100644
--- a/verify.c
+++ b/verify.c
@@ -1635,8 +1635,7 @@ struct all_io_list *get_all_io_list(int save_mask, size_t *sz)
 			s->rand.state32.s[3] = 0;
 			s->rand.use64 = 0;
 		}
-		s->name[sizeof(s->name) - 1] = '\0';
-		strncpy((char *) s->name, td->o.name, sizeof(s->name) - 1);
+		snprintf((char *) s->name, sizeof(s->name), "%s", td->o.name);
 		next = io_list_next(s);
 	}
 
-- 
2.22.0.rc1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/7] Refine packed annotations in stat.h
  2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
                   ` (2 preceding siblings ...)
  2019-08-14 20:10 ` [PATCH v3 3/7] Optimize the code that copies strings Bart Van Assche
@ 2019-08-14 20:10 ` Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 5/7] Verify the absence of holes in struct jobs_eta at compile time Bart Van Assche
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 20:10 UTC (permalink / raw
  To: Jens Axboe; +Cc: fio, Bart Van Assche

Instead of declaring the whole structure packed, only declare non-aligned
members packed. This patch is an alternative way to fix the following gcc 9
compiler warnings:

eta.c: In function 'calc_thread_status':
eta.c:510:7: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  510 |     je->rate);
      |     ~~^~~~~~
eta.c:522:66: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  522 |  calc_rate(unified_rw_rep, disp_time, io_bytes, disp_io_bytes, je->rate);
      |                                                                ~~^~~~~~
eta.c:523:64: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  523 |  calc_iops(unified_rw_rep, disp_time, io_iops, disp_io_iops, je->iops);
      |

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 stat.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/stat.h b/stat.h
index e9551381ce7b..c6353c70ae08 100644
--- a/stat.h
+++ b/stat.h
@@ -260,10 +260,11 @@ struct jobs_eta {
 
 	uint64_t m_rate[DDIR_RWDIR_CNT], t_rate[DDIR_RWDIR_CNT];
 	uint64_t rate[DDIR_RWDIR_CNT];
-	uint32_t m_iops[DDIR_RWDIR_CNT], t_iops[DDIR_RWDIR_CNT];
-	uint32_t iops[DDIR_RWDIR_CNT];
-	uint64_t elapsed_sec;
-	uint64_t eta_sec;
+	uint32_t m_iops[DDIR_RWDIR_CNT] __attribute__((packed));
+	uint32_t t_iops[DDIR_RWDIR_CNT] __attribute__((packed));
+	uint32_t iops[DDIR_RWDIR_CNT] __attribute__((packed));
+	uint64_t elapsed_sec __attribute__((packed));
+	uint64_t eta_sec __attribute__((packed));
 	uint32_t is_pow2;
 	uint32_t unit_base;
 
@@ -276,7 +277,7 @@ struct jobs_eta {
 	 */
 	uint32_t nr_threads;
 	uint8_t run_str[];
-} __attribute__((packed));
+};
 
 struct io_u_plat_entry {
 	struct flist_head list;
-- 
2.22.0.rc1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 5/7] Verify the absence of holes in struct jobs_eta at compile time
  2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
                   ` (3 preceding siblings ...)
  2019-08-14 20:10 ` [PATCH v3 4/7] Refine packed annotations in stat.h Bart Van Assche
@ 2019-08-14 20:10 ` Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 6/7] Restore type checking in calc_thread_status() Bart Van Assche
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 20:10 UTC (permalink / raw
  To: Jens Axboe; +Cc: fio, Bart Van Assche

This patch verifies the correctness of the previous patch.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 eta.c  |  3 +++
 stat.h | 55 +++++++++++++++++++++++++++++--------------------------
 2 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/eta.c b/eta.c
index 647a1bdd8eed..5900bc0fd919 100644
--- a/eta.c
+++ b/eta.c
@@ -736,6 +736,9 @@ void print_thread_status(void)
 
 void print_status_init(int thr_number)
 {
+	compiletime_assert(sizeof(struct jobs_eta) ==
+			   sizeof(struct jobs_eta_packed), "jobs_eta");
+
 	DRD_IGNORE_VAR(__run_str);
 	__run_str[thr_number] = 'P';
 	update_condensed_str(__run_str, run_str);
diff --git a/stat.h b/stat.h
index c6353c70ae08..c209ab6c7a96 100644
--- a/stat.h
+++ b/stat.h
@@ -251,33 +251,36 @@ struct thread_stat {
 	uint64_t cachemiss;
 } __attribute__((packed));
 
-struct jobs_eta {
-	uint32_t nr_running;
-	uint32_t nr_ramp;
-
-	uint32_t nr_pending;
-	uint32_t nr_setting_up;
-
-	uint64_t m_rate[DDIR_RWDIR_CNT], t_rate[DDIR_RWDIR_CNT];
-	uint64_t rate[DDIR_RWDIR_CNT];
-	uint32_t m_iops[DDIR_RWDIR_CNT] __attribute__((packed));
-	uint32_t t_iops[DDIR_RWDIR_CNT] __attribute__((packed));
-	uint32_t iops[DDIR_RWDIR_CNT] __attribute__((packed));
-	uint64_t elapsed_sec __attribute__((packed));
-	uint64_t eta_sec __attribute__((packed));
-	uint32_t is_pow2;
-	uint32_t unit_base;
-
-	uint32_t sig_figs;
-
-	uint32_t files_open;
+#define JOBS_ETA {							\
+	uint32_t nr_running;						\
+	uint32_t nr_ramp;						\
+									\
+	uint32_t nr_pending;						\
+	uint32_t nr_setting_up;						\
+									\
+	uint64_t m_rate[DDIR_RWDIR_CNT], t_rate[DDIR_RWDIR_CNT];	\
+	uint64_t rate[DDIR_RWDIR_CNT];					\
+	uint32_t m_iops[DDIR_RWDIR_CNT] __attribute__((packed));	\
+	uint32_t t_iops[DDIR_RWDIR_CNT] __attribute__((packed));	\
+	uint32_t iops[DDIR_RWDIR_CNT] __attribute__((packed));		\
+	uint64_t elapsed_sec __attribute__((packed));			\
+	uint64_t eta_sec __attribute__((packed));			\
+	uint32_t is_pow2;						\
+	uint32_t unit_base;						\
+									\
+	uint32_t sig_figs;						\
+									\
+	uint32_t files_open;						\
+									\
+	/*								\
+	 * Network 'copy' of run_str[]					\
+	 */								\
+	uint32_t nr_threads;						\
+	uint8_t run_str[];						\
+}
 
-	/*
-	 * Network 'copy' of run_str[]
-	 */
-	uint32_t nr_threads;
-	uint8_t run_str[];
-};
+struct jobs_eta JOBS_ETA;
+struct jobs_eta_packed JOBS_ETA __attribute__((packed));
 
 struct io_u_plat_entry {
 	struct flist_head list;
-- 
2.22.0.rc1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 6/7] Restore type checking in calc_thread_status()
  2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
                   ` (4 preceding siblings ...)
  2019-08-14 20:10 ` [PATCH v3 5/7] Verify the absence of holes in struct jobs_eta at compile time Bart Van Assche
@ 2019-08-14 20:10 ` Bart Van Assche
  2019-08-14 20:10 ` [PATCH v3 7/7] Makefile: Add 'fulltest' target Bart Van Assche
  2019-08-14 21:02 ` [PATCH v3 0/7] Seven fio patches Jens Axboe
  7 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 20:10 UTC (permalink / raw
  To: Jens Axboe; +Cc: fio, Bart Van Assche, Damien Le Moal

Due to a previous patch it is no longer necessary to hide the type of
accesses to the 'rate' and 'iops' members in struct jobs_eta.

This patch reverts commit df0ca15ce2ff ("eta: Fix compiler warning").

Cc: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 eta.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/eta.c b/eta.c
index 5900bc0fd919..286b45ee3837 100644
--- a/eta.c
+++ b/eta.c
@@ -392,9 +392,6 @@ bool calc_thread_status(struct jobs_eta *je, int force)
 	static unsigned long long disp_io_iops[DDIR_RWDIR_CNT];
 	static struct timespec rate_prev_time, disp_prev_time;
 
-	void *je_rate = (void *) je->rate;
-	void *je_iops = (void *) je->iops;
-
 	if (!force) {
 		if (!(output_format & FIO_OUTPUT_NORMAL) &&
 		    f_out == stdout)
@@ -510,7 +507,7 @@ bool calc_thread_status(struct jobs_eta *je, int force)
 
 	if (write_bw_log && rate_time > bw_avg_time && !in_ramp_time(td)) {
 		calc_rate(unified_rw_rep, rate_time, io_bytes, rate_io_bytes,
-				je_rate);
+				je->rate);
 		memcpy(&rate_prev_time, &now, sizeof(now));
 		add_agg_sample(sample_val(je->rate[DDIR_READ]), DDIR_READ, 0);
 		add_agg_sample(sample_val(je->rate[DDIR_WRITE]), DDIR_WRITE, 0);
@@ -522,8 +519,8 @@ bool calc_thread_status(struct jobs_eta *je, int force)
 	if (!force && !eta_time_within_slack(disp_time))
 		return false;
 
-	calc_rate(unified_rw_rep, disp_time, io_bytes, disp_io_bytes, je_rate);
-	calc_iops(unified_rw_rep, disp_time, io_iops, disp_io_iops, je_iops);
+	calc_rate(unified_rw_rep, disp_time, io_bytes, disp_io_bytes, je->rate);
+	calc_iops(unified_rw_rep, disp_time, io_iops, disp_io_iops, je->iops);
 
 	memcpy(&disp_prev_time, &now, sizeof(now));
 
-- 
2.22.0.rc1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 7/7] Makefile: Add 'fulltest' target
  2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
                   ` (5 preceding siblings ...)
  2019-08-14 20:10 ` [PATCH v3 6/7] Restore type checking in calc_thread_status() Bart Van Assche
@ 2019-08-14 20:10 ` Bart Van Assche
  2019-08-14 21:11   ` Sitsofe Wheeler
  2019-08-14 21:02 ` [PATCH v3 0/7] Seven fio patches Jens Axboe
  7 siblings, 1 reply; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 20:10 UTC (permalink / raw
  To: Jens Axboe; +Cc: fio, Bart Van Assche

Make it easier to run the zoned block device tests.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 Makefile | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Makefile b/Makefile
index fe02bf1df86f..7c21ef8316f7 100644
--- a/Makefile
+++ b/Makefile
@@ -531,6 +531,21 @@ doc: tools/plot/fio2gnuplot.1
 test: fio
 	./fio --minimal --thread --exitall_on_error --runtime=1s --name=nulltest --ioengine=null --rw=randrw --iodepth=2 --norandommap --random_generator=tausworthe64 --size=16T --name=verifyfstest --filename=fiotestfile.tmp --unlink=1 --rw=write --verify=crc32c --verify_state_save=0 --size=16K
 
+fulltest:
+	sudo modprobe null_blk &&				 	\
+	if [ ! -e /usr/include/libzbc/zbc.h ]; then			\
+	  git clone https://github.com/hgst/libzbc &&		 	\
+	  (cd libzbc &&						 	\
+	   ./autogen.sh &&					 	\
+	   ./configure --prefix=/usr &&				 	\
+	   make -j &&						 	\
+	   sudo make install)						\
+	fi &&					 			\
+	sudo t/zbd/run-tests-against-regular-nullb &&		 	\
+	if [ -e /sys/module/null_blk/parameters/zoned ]; then		\
+		sudo t/zbd/run-tests-against-zoned-nullb;	 	\
+	fi
+
 install: $(PROGS) $(SCRIPTS) tools/plot/fio2gnuplot.1 FORCE
 	$(INSTALL) -m 755 -d $(DESTDIR)$(bindir)
 	$(INSTALL) $(PROGS) $(SCRIPTS) $(DESTDIR)$(bindir)
@@ -541,3 +556,5 @@ install: $(PROGS) $(SCRIPTS) tools/plot/fio2gnuplot.1 FORCE
 	$(INSTALL) -m 644 $(SRCDIR)/tools/hist/fiologparser_hist.py.1 $(DESTDIR)$(mandir)/man1
 	$(INSTALL) -m 755 -d $(DESTDIR)$(sharedir)
 	$(INSTALL) -m 644 $(SRCDIR)/tools/plot/*gpm $(DESTDIR)$(sharedir)/
+
+.PHONY: test fulltest
-- 
2.22.0.rc1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/7] Seven fio patches
  2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
                   ` (6 preceding siblings ...)
  2019-08-14 20:10 ` [PATCH v3 7/7] Makefile: Add 'fulltest' target Bart Van Assche
@ 2019-08-14 21:02 ` Jens Axboe
  7 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2019-08-14 21:02 UTC (permalink / raw
  To: Bart Van Assche; +Cc: fio

On 8/14/19 2:10 PM, Bart Van Assche wrote:
> Hi Jens,
> 
> The following changes are included in this patch series:
> - Declare local functions 'static' in zbd.c.
> - Make the zbd test scripts more robust.
> - Use snprintf() instead of strncpy() + explicit null termination.
> - Rework the approach for suppressing gcc 9 address of packed member warnings.
> - Add a 'fulltest' target that runs the zbd tests.

Applied, thanks Bart.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 7/7] Makefile: Add 'fulltest' target
  2019-08-14 20:10 ` [PATCH v3 7/7] Makefile: Add 'fulltest' target Bart Van Assche
@ 2019-08-14 21:11   ` Sitsofe Wheeler
  2019-08-14 21:14     ` Bart Van Assche
  0 siblings, 1 reply; 12+ messages in thread
From: Sitsofe Wheeler @ 2019-08-14 21:11 UTC (permalink / raw
  To: Bart Van Assche; +Cc: Jens Axboe, fio

On Wed, 14 Aug 2019 at 21:12, Bart Van Assche <bvanassche@acm.org> wrote:
>
> Make it easier to run the zoned block device tests.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  Makefile | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index fe02bf1df86f..7c21ef8316f7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -531,6 +531,21 @@ doc: tools/plot/fio2gnuplot.1
>  test: fio
>         ./fio --minimal --thread --exitall_on_error --runtime=1s --name=nulltest --ioengine=null --rw=randrw --iodepth=2 --norandommap --random_generator=tausworthe64 --size=16T --name=verifyfstest --filename=fiotestfile.tmp --unlink=1 --rw=write --verify=crc32c --verify_state_save=0 --size=16K
>
> +fulltest:
> +       sudo modprobe null_blk &&                                       \
> +       if [ ! -e /usr/include/libzbc/zbc.h ]; then                     \
> +         git clone https://github.com/hgst/libzbc &&                   \
> +         (cd libzbc &&                                                 \
> +          ./autogen.sh &&                                              \
> +          ./configure --prefix=/usr &&                                 \
> +          make -j &&                                                   \
> +          sudo make install)                                           \
> +       fi &&                                                           \
> +       sudo t/zbd/run-tests-against-regular-nullb &&                   \
> +       if [ -e /sys/module/null_blk/parameters/zoned ]; then           \
> +               sudo t/zbd/run-tests-against-zoned-nullb;               \
> +       fi
> +
>  install: $(PROGS) $(SCRIPTS) tools/plot/fio2gnuplot.1 FORCE
>         $(INSTALL) -m 755 -d $(DESTDIR)$(bindir)
>         $(INSTALL) $(PROGS) $(SCRIPTS) $(DESTDIR)$(bindir)
> @@ -541,3 +556,5 @@ install: $(PROGS) $(SCRIPTS) tools/plot/fio2gnuplot.1 FORCE
>         $(INSTALL) -m 644 $(SRCDIR)/tools/hist/fiologparser_hist.py.1 $(DESTDIR)$(mandir)/man1
>         $(INSTALL) -m 755 -d $(DESTDIR)$(sharedir)
>         $(INSTALL) -m 644 $(SRCDIR)/tools/plot/*gpm $(DESTDIR)$(sharedir)/
> +
> +.PHONY: test fulltest
> --
> 2.22.0.rc1
>

Looks good. We just need to switch
https://github.com/axboe/fio/blob/master/.travis.yml over to run
both...

-- 
Sitsofe | http://sucs.org/~sits/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 7/7] Makefile: Add 'fulltest' target
  2019-08-14 21:11   ` Sitsofe Wheeler
@ 2019-08-14 21:14     ` Bart Van Assche
  2019-08-14 21:40       ` Sitsofe Wheeler
  0 siblings, 1 reply; 12+ messages in thread
From: Bart Van Assche @ 2019-08-14 21:14 UTC (permalink / raw
  To: Sitsofe Wheeler; +Cc: Jens Axboe, fio

On 8/14/19 2:11 PM, Sitsofe Wheeler wrote:
> Looks good. We just need to switch
> https://github.com/axboe/fio/blob/master/.travis.yml over to run
> both...

Hi Sitsofe,

I'm not sure whether Travis supports kernel module loading. See also
https://github.com/travis-ci/travis-ci/issues/2291.

Bart.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 7/7] Makefile: Add 'fulltest' target
  2019-08-14 21:14     ` Bart Van Assche
@ 2019-08-14 21:40       ` Sitsofe Wheeler
  0 siblings, 0 replies; 12+ messages in thread
From: Sitsofe Wheeler @ 2019-08-14 21:40 UTC (permalink / raw
  To: Bart Van Assche; +Cc: Jens Axboe, fio

On Wed, 14 Aug 2019 at 22:14, Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 8/14/19 2:11 PM, Sitsofe Wheeler wrote:
> > Looks good. We just need to switch
> > https://github.com/axboe/fio/blob/master/.travis.yml over to run
> > both...
>
> Hi Sitsofe,
>
> I'm not sure whether Travis supports kernel module loading. See also
> https://github.com/travis-ci/travis-ci/issues/2291.

Doh - I hadn't twigged. Apologies for the noise...

-- 
Sitsofe | http://sucs.org/~sits/


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-08-14 21:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-14 20:10 [PATCH v3 0/7] Seven fio patches Bart Van Assche
2019-08-14 20:10 ` [PATCH v3 1/7] zbd: Declare local functions 'static' Bart Van Assche
2019-08-14 20:10 ` [PATCH v3 2/7] zbd: Improve robustness of unit tests Bart Van Assche
2019-08-14 20:10 ` [PATCH v3 3/7] Optimize the code that copies strings Bart Van Assche
2019-08-14 20:10 ` [PATCH v3 4/7] Refine packed annotations in stat.h Bart Van Assche
2019-08-14 20:10 ` [PATCH v3 5/7] Verify the absence of holes in struct jobs_eta at compile time Bart Van Assche
2019-08-14 20:10 ` [PATCH v3 6/7] Restore type checking in calc_thread_status() Bart Van Assche
2019-08-14 20:10 ` [PATCH v3 7/7] Makefile: Add 'fulltest' target Bart Van Assche
2019-08-14 21:11   ` Sitsofe Wheeler
2019-08-14 21:14     ` Bart Van Assche
2019-08-14 21:40       ` Sitsofe Wheeler
2019-08-14 21:02 ` [PATCH v3 0/7] Seven fio patches Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.