NAK/cmnt: [PATCH] [act] UBUNTU: SAUCE: ubuntu_performance_fio: fix build on Hirsute/Impish and correct all gcc-9 warnings
Po-Hsu Lin
po-hsu.lin at canonical.com
Mon Jul 5 03:21:20 UTC 2021
On Fri, Jul 2, 2021 at 10:03 PM Paolo Pisati <paolo.pisati at canonical.com> wrote:
>
> As the subject says, this patch fixes fio build in Hirsute/Impish and fixes all
> gcc-9 warnings about truncated strcpy() and unaligned pointer value.
>
> All patches are clean cherry-picks from upstream.
>
> Signed-off-by: Paolo Pisati <paolo.pisati at canonical.com>
> ---
> ...cate-global-definition-of-tsc_reliab.patch | 44 ++
> ...Fix-string-copy-compilation-warnings.patch | 95 ++++
> ...ptimize-the-code-that-copies-strings.patch | 422 ++++++++++++++++++
> .../0005-eta-Fix-compiler-warning.patch | 62 +++
> .../ubuntu_performance_fio.py | 6 +-
> 5 files changed, 628 insertions(+), 1 deletion(-)
> create mode 100644 ubuntu_performance_fio/0002-fio-remove-duplicate-global-definition-of-tsc_reliab.patch
> create mode 100644 ubuntu_performance_fio/0003-Fix-string-copy-compilation-warnings.patch
> create mode 100644 ubuntu_performance_fio/0004-Optimize-the-code-that-copies-strings.patch
> create mode 100644 ubuntu_performance_fio/0005-eta-Fix-compiler-warning.patch
>
> diff --git a/ubuntu_performance_fio/0002-fio-remove-duplicate-global-definition-of-tsc_reliab.patch b/ubuntu_performance_fio/0002-fio-remove-duplicate-global-definition-of-tsc_reliab.patch
> new file mode 100644
> index 00000000..9b7e1237
> --- /dev/null
> +++ b/ubuntu_performance_fio/0002-fio-remove-duplicate-global-definition-of-tsc_reliab.patch
> @@ -0,0 +1,44 @@
> +From 7548e599168b73549d97670ad2b3a72c1fa71e76 Mon Sep 17 00:00:00 2001
> +From: Eric Sandeen <sandeen at redhat.com>
> +Date: Fri, 31 Jan 2020 13:19:07 -0600
> +Subject: [PATCH 2/5] fio: remove duplicate global definition of tsc_reliable
> +
> +Newer gcc v10 complains about globals being defined twice due to
> +a new default -fno-common flag. This causes build failures:
> +
> + LINK t/fio-dedupe
> +/usr/bin/ld: t/arch.o:(.bss+0x4): multiple definition of `tsc_reliable'; gettime.o:(.bss+0x4): first defined here
> +collect2: error: ld returned 1 exit status
> +make: *** [Makefile:499: t/fio-dedupe] Error 1
> +
> +Each of the tests which call arch_init() to initialize tsc_reliable:
> +
> + File Function Line
> +1 t/dedupe.c main 536 arch_init(argv);
> +2 t/lfsr-test.c main 37 arch_init(argv);
> +3 t/stest.c main 86 arch_init(argv);
> +
> +already link with gettime.o which defines tsc_reliable,
> +so there is no need to provide it from t/arch.c
> +
> +Signed-off-by: Eric Sandeen <sandeen at redhat.com>
> +Signed-off-by: Jens Axboe <axboe at kernel.dk>
> +(cherry picked from https://github.com/axboe/fio.git commit 019604f22a4cbeb71bb80d8077fa511ae62ea14c)
> +Signed-off-by: Paolo Pisati <paolo.pisati at canonical.com>
> +---
> + fio-3.10/t/arch.c | 1 -
> + 1 file changed, 1 deletion(-)
> +
> +diff --git a/fio-3.10/t/arch.c b/fio-3.10/t/arch.c
> +index bd28a848..a72cef3a 100644
> +--- a/fio-3.10/t/arch.c
> ++++ b/fio-3.10/t/arch.c
> +@@ -1,5 +1,4 @@
> + #include "../arch/arch.h"
> +
> + unsigned long arch_flags = 0;
> +-bool tsc_reliable;
> + int arch_random;
> +--
> +2.31.1
> +
> diff --git a/ubuntu_performance_fio/0003-Fix-string-copy-compilation-warnings.patch b/ubuntu_performance_fio/0003-Fix-string-copy-compilation-warnings.patch
> new file mode 100644
> index 00000000..43ab34ac
> --- /dev/null
> +++ b/ubuntu_performance_fio/0003-Fix-string-copy-compilation-warnings.patch
> @@ -0,0 +1,95 @@
> +From 19a3ddff198e431f0d6f7ca7954aaf2190e5595b Mon Sep 17 00:00:00 2001
> +From: Damien Le Moal <damien.lemoal at wdc.com>
> +Date: Tue, 4 Jun 2019 16:07:31 +0900
> +Subject: [PATCH 3/5] Fix string copy compilation warnings
> +
> +Fix the many warnings that gcc 9 spits out.
> +
> +Signed-off-by: Damien Le Moal <damien.lemoal at wdc.com>
> +Signed-off-by: Jens Axboe <axboe at kernel.dk>
> +(cherry picked from https://github.com/axboe/fio.git commit 32e31c8c5f7b1695a85a79da02ac854899abbb39)
> +Signed-off-by: Paolo Pisati <paolo.pisati at canonical.com>
> +---
> + fio-3.10/exp/expression-parser.y | 6 +++---
> + fio-3.10/filesetup.c | 3 ++-
> + fio-3.10/finit.c | 5 +++--
> + fio-3.10/fserver.c | 9 ++++++---
> + 4 files changed, 14 insertions(+), 9 deletions(-)
> +
> +diff --git a/fio-3.10/exp/expression-parser.y b/fio-3.10/exp/expression-parser.y
> +index 04a6e07a..8619025c 100644
> +--- a/fio-3.10/exp/expression-parser.y
> ++++ b/fio-3.10/eexp/expression-parser.y
> +@@ -204,9 +204,9 @@ static void setup_to_parse_string(const char *string)
> + {
> + unsigned int len;
> +
> +- len = strlen(string);
> +- if (len > sizeof(lexer_input_buffer) - 3)
> +- len = sizeof(lexer_input_buffer) - 3;
> ++ len = sizeof(lexer_input_buffer) - 3;
> ++ if (len > strlen(string))
> ++ len = strlen(string);
> +
> + strncpy(lexer_input_buffer, string, len);
> + lexer_input_buffer[len] = '\0';
> +diff --git a/fio-3.10/filesetup.c b/fio-3.10/filesetup.c
> +index 580403db..4e547e79 100644
> +--- a/fio-3.10/filesetup.c
> ++++ b/fio-3.10/filesetup.c
> +@@ -829,7 +829,8 @@ 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) - 1);
> ++ strncpy(fm->__base, buf, sizeof(fm->__base));
> ++ fm->__base[255] = '\0';
> + fm->base = basename(fm->__base);
> + fm->key = sb.st_dev;
> + flist_add(&fm->list, &list);
> +diff --git a/fio-3.10/init.c b/fio-3.10/init.c
> +index c235b05e..faa9ab4e 100644
> +--- a/fio-3.10/init.c
> ++++ b/fio-3.10/init.c
> +@@ -1438,7 +1438,7 @@ static int add_job(struct thread_data *td, const char *jobname, int job_add_num,
> + int recursed, int client_type)
> + {
> + unsigned int i;
> +- char fname[PATH_MAX];
> ++ char fname[PATH_MAX + 1];
> + int numjobs, file_alloced;
> + struct thread_options *o = &td->o;
> + char logname[PATH_MAX + 32];
> +@@ -2032,7 +2032,8 @@ static int __parse_jobs_ini(struct thread_data *td,
> + strncpy(full_fn,
> + file, (ts - file) + 1);
> + strncpy(full_fn + (ts - file) + 1,
> +- filename, strlen(filename));
> ++ filename,
> ++ len - (ts - file) - 1);
> + full_fn[len - 1] = 0;
> + filename = full_fn;
> + }
> +diff --git a/fio-3.10/server.c b/fio-3.10/server.c
> +index b966c66c..56e798a5 100644
> +--- a/fio-3.10/server.c
> ++++ b/fio-3.10/server.c
> +@@ -1466,9 +1466,12 @@ 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 - 1);
> +- strncpy(p.ts.verror, ts->verror, FIO_VERROR_SIZE - 1);
> +- strncpy(p.ts.description, ts->description, FIO_JOBDESC_SIZE - 1);
> ++ 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';
> +
> + p.ts.error = cpu_to_le32(ts->error);
> + p.ts.thread_number = cpu_to_le32(ts->thread_number);
> +--
> +2.31.1
> +
> diff --git a/ubuntu_performance_fio/0004-Optimize-the-code-that-copies-strings.patch b/ubuntu_performance_fio/0004-Optimize-the-code-that-copies-strings.patch
> new file mode 100644
> index 00000000..a73a0aa6
> --- /dev/null
> +++ b/ubuntu_performance_fio/0004-Optimize-the-code-that-copies-strings.patch
> @@ -0,0 +1,422 @@
> +From 1512d7c4f605c46d4736f3b20b3a64d09bdb2828 Mon Sep 17 00:00:00 2001
> +From: Bart Van Assche <bvanassche at acm.org>
> +Date: Wed, 14 Aug 2019 13:10:08 -0700
> +Subject: [PATCH 4/5] Optimize the code that copies strings
> +
> +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 at wdc.com>
> +Signed-off-by: Bart Van Assche <bvanassche at acm.org>
> +Signed-off-by: Jens Axboe <axboe at kernel.dk>
> +(cherry picked from https://github.com/axboe/fio.git commit 36833fb04b5f9a734e96a571dfb52fc54b5b95e7)
> +Signed-off-by: Paolo Pisati <paolo.pisati at canonical.com>
> +---
> + fio-3.10/cconv.c | 7 +++----
> + fio-3.10/client.c | 5 +++--
> + fio-3.10/diskutil.c | 9 ++++-----
> + fio-3.10/engines/net.c | 6 ++----
> + fio-3.10/engines/sg.c | 4 ++--
> + fio-3.10/filesetup.c | 6 ++----
> + fio-3.10/gclient.c | 4 ++--
> + fio-3.10/init.c | 19 +++++--------------
> + fio-3.10/ioengines.c | 3 +--
> + fio-3.10/options.c | 3 +--
> + fio-3.10/parse.c | 6 ++----
> + fio-3.10/server.c | 26 +++++++++++---------------
> + fio-3.10/stat.c | 15 ++++++++-------
> + fio-3.10/verify.c | 3 +--
> + 14 files changed, 47 insertions(+), 69 deletions(-)
> +
> +diff --git a/fio-3.10/cconv.c b/fio-3.10/cconv.c
> +index 1d7f6f22..f1eab686 100644
> +--- a/fio-3.10/cconv.c
> ++++ b/fio-3.10/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/fio-3.10/client.c b/fio-3.10/client.c
> +index 32489067..70c1127d 100644
> +--- a/fio-3.10/client.c
> ++++ b/fio-3.10/client.c
> +@@ -519,7 +519,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);
> + }
> +@@ -573,7 +573,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/fio-3.10/diskutil.c b/fio-3.10/diskutil.c
> +index 7be4c022..f0744015 100644
> +--- a/fio-3.10/diskutil.c
> ++++ b/fio-3.10/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/fio-3.10/engines/net.c b/fio-3.10/engines/net.c
> +index ca6fb344..91f25774 100644
> +--- a/fio-3.10/engines/net.c
> ++++ b/fio-3.10/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/fio-3.10/engines/sg.c b/fio-3.10/engines/sg.c
> +index 3cc068f3..0685350d 100644
> +--- a/fio-3.10/engines/sg.c
> ++++ b/fio-3.10/engines/sg.c
> +@@ -1152,8 +1152,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/fio-3.10/filesetup.c b/fio-3.10/filesetup.c
> +index 4e547e79..a5e3e363 100644
> +--- a/fio-3.10/filesetup.c
> ++++ b/fio-3.10/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/fio-3.10/gclient.c b/fio-3.10/gclient.c
> +index 04275a13..64324177 100644
> +--- a/fio-3.10/gclient.c
> ++++ b/fio-3.10/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/fio-3.10/init.c b/fio-3.10/init.c
> +index faa9ab4e..8f318caa 100644
> +--- a/fio-3.10/init.c
> ++++ b/fio-3.10/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);
> + }
> +
> +@@ -2021,20 +2020,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/fio-3.10/ioengines.c b/fio-3.10/ioengines.c
> +index ba02952b..cc5854cd 100644
> +--- a/fio-3.10/ioengines.c
> ++++ b/fio-3.10/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/fio-3.10/options.c b/fio-3.10/options.c
> +index 6bd74555..6515a92e 100644
> +--- a/fio-3.10/options.c
> ++++ b/fio-3.10/options.c
> +@@ -4870,8 +4870,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/fio-3.10/parse.c b/fio-3.10/parse.c
> +index 5d88d910..75e3ec3e 100644
> +--- a/fio-3.10/parse.c
> ++++ b/fio-3.10/parse.c
> +@@ -575,8 +575,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';
> +@@ -802,8 +801,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/fio-3.10/server.c b/fio-3.10/server.c
> +index 56e798a5..96ac316a 100644
> +--- a/fio-3.10/server.c
> ++++ b/fio-3.10/server.c
> +@@ -861,7 +861,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
> +@@ -1466,12 +1467,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);
> +@@ -1658,8 +1657,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]);
> +@@ -1969,8 +1967,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,
> +@@ -2187,9 +2184,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;
> +
> +@@ -2239,9 +2235,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/fio-3.10/stat.c b/fio-3.10/stat.c
> +index 5fca9984..6dfd6ff4 100644
> +--- a/fio-3.10/stat.c
> ++++ b/fio-3.10/stat.c
> +@@ -1764,10 +1764,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);
> +
> +@@ -1804,12 +1805,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/fio-3.10/verify.c b/fio-3.10/verify.c
> +index 01492f24..2eeffc75 100644
> +--- a/fio-3.10/verify.c
> ++++ b/fio-3.10/verify.c
> +@@ -1631,8 +1631,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.31.1
> +
> diff --git a/ubuntu_performance_fio/0005-eta-Fix-compiler-warning.patch b/ubuntu_performance_fio/0005-eta-Fix-compiler-warning.patch
> new file mode 100644
> index 00000000..4016d0b8
> --- /dev/null
> +++ b/ubuntu_performance_fio/0005-eta-Fix-compiler-warning.patch
> @@ -0,0 +1,62 @@
> +From 209e556433c1ca8ee6cbd05b7a36bd632e785e02 Mon Sep 17 00:00:00 2001
> +From: Damien Le Moal <damien.lemoal at wdc.com>
> +Date: Tue, 4 Jun 2019 16:07:32 +0900
> +Subject: [PATCH 5/5] eta: Fix compiler warning
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +gcc 9 complains about directly referencing pointer values in the packed
> +structure jobs_eta. E.g.:
> +
> +warning: taking address of packed member of ‘struct jobs_eta’ may
> +result in an unaligned pointer value [-Waddress-of-packed-member]
> +
> +Remove this by using a local void pointer for the rate and iops array
> +references that generate the warning.
> +
> +Signed-off-by: Damien Le Moal <damien.lemoal at wdc.com>
> +Signed-off-by: Jens Axboe <axboe at kernel.dk>
> +(cherry picked from https://github.com/axboe/fio.git commit df0ca15ce2ffac0df3e9dcf4bfe4121333c4aa27)
> +Signed-off-by: Paolo Pisati <paolo.pisati at canonical.com>
> +---
> + fio-3.10/eta.c | 9 ++++++---
> + 1 file changed, 6 insertions(+), 3 deletions(-)
> +
> +diff --git a/fio-3.10/eta.c b/fio-3.10/eta.c
> +index 970a67df..f9e0990d 100644
> +--- a/fio-3.10/eta.c
> ++++ b/fio-3.10/eta.c
> +@@ -377,6 +377,9 @@ 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)
> +@@ -492,7 +495,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);
> +@@ -504,8 +507,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.31.1
> +
> diff --git a/ubuntu_performance_fio/ubuntu_performance_fio.py b/ubuntu_performance_fio/ubuntu_performance_fio.py
> index 39750702..0a15eee2 100644
> --- a/ubuntu_performance_fio/ubuntu_performance_fio.py
> +++ b/ubuntu_performance_fio/ubuntu_performance_fio.py
> @@ -194,7 +194,11 @@ class ubuntu_performance_fio(test.test):
> self.results = utils.system_output('unxz < %s | tar xf - ' % os.path.join(self.bindir, 'fio-3.10.tar.xz'), retain_output=True)
> if platform.linux_distribution()[2] not in "bionic":
> self.results += utils.system_output('patch -p1 < %s' % os.path.join(self.bindir,'0001-Rename-gettid-to-sys_gettid-to-avoid-name-clash.patch'), retain_output=True)
> - os.chdir(os.path.join(self.srcdir, 'fio-3.10'))
> + self.results += utils.system_output('patch -p1 < %s' % os.path.join(self.bindir,'0002-fio-remove-duplicate-global-definition-of-tsc_reliab.patch'), retain_output=True)
> + self.results += utils.system_output('patch -p1 < %s' % os.path.join(self.bindir,'0003-Fix-string-copy-compilation-warnings.patch'), retain_output=True)
> + self.results += utils.system_output('patch -p1 < %s' % os.path.join(self.bindir,'0004-Optimize-the-code-that-copies-strings.patch'), retain_output=True)
> + self.results += utils.system_output('patch -p1 < %s' % os.path.join(self.bindir,'0005-eta-Fix-compiler-warning.patch'), retain_output=True)
Hi Paolo,
this line breaks the test build on bionic, this should be placed
outside the if statement.
> + os.chdir(os.path.join(self.srcdir, 'fio-3.10'))
Otherwise this patch is looking good.
I am wondering if we should just use different fio version, like using
different tarballs for different releases.
Thanks
Sam
> self.results += utils.system_output('make', retain_output=True)
>
> print(self.results)
> --
> 2.31.1
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list