[PATCH 1/8] Don't silently terminate td when no I/O performed due to error

[PATCH 1/8] Don't silently terminate td when no I/O performed due to error

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

Some runtime configurations can cause threads/processes to terminate
without any I/O performed, yet with no explicit error message, which
is quite confusing. In the example below, fio finishes with neither
error nor regular statistics due to file offset + bs being larger
than the file size while in get_io_u().

This commit calls log_err() when this happens. Since it's difficult
to tell the exact reason after thread has left the main I/O loop[*],
it gives advice to use a command line option --debug=io, similar to
the way td_io_init() gives advice.

[*] It can't just replace dprint(FD_IO, ...) calls with td_verror()
for e.g. while in get_io_u(), since those are also used by non error
paths too.

--
 # ./fio --name=xxxxx --ioengine=sync --rw=read --bs=2k --size=1m --nrfiles=1k --unlink=1
 xxxxx: (g=0): rw=read, bs=(R) 2048B-2048B, (W) 2048B-2048B, (T) 2048B-2048B, ioengine=sync, iodepth=1
 fio-2.18-59-g618e
 Starting 1 process
 xxxxx: Laying out IO files (1024 files / total 1MiB)

 Run status group 0 (all jobs):

 Disk stats (read/write):
     dm-0: ios=0/0, merge=0/0, ticks=0/0, in_queue=0, util=0.00%, aggrios=0/0, aggrmerge=0/0, aggrticks=0/0, aggrin_queue=0, aggrutil=0.00%
   sda: ios=0/0, merge=0/0, ticks=0/0, in_queue=0, util=0.00%

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 backend.c | 17 +++++++++++++++--
 io_u.c    |  5 +++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/backend.c b/backend.c
index 38ef348..e5be187 100644
--- a/backend.c
+++ b/backend.c
@@ -1456,6 +1456,7 @@ static void *thread_main(void *data)
 	struct thread_data *td = fd->td;
 	struct thread_options *o = &td->o;
 	struct sk_out *sk_out = fd->sk_out;
+	uint64_t bytes_done[DDIR_RWDIR_CNT];
 	int deadlock_loop_cnt;
 	int clear_state;
 	int ret;
@@ -1677,7 +1678,9 @@ static void *thread_main(void *data)
 					sizeof(td->bw_sample_time));
 	}
 
+	memset(bytes_done, 0, sizeof(bytes_done));
 	clear_state = 0;
+
 	while (keep_running(td)) {
 		uint64_t verify_bytes;
 
@@ -1696,8 +1699,6 @@ static void *thread_main(void *data)
 		if (td->o.verify_only && td_write(td))
 			verify_bytes = do_dry_run(td);
 		else {
-			uint64_t bytes_done[DDIR_RWDIR_CNT];
-
 			do_io(td, bytes_done);
 
 			if (!ddir_rw_sum(bytes_done)) {
@@ -1776,6 +1777,18 @@ static void *thread_main(void *data)
 			break;
 	}
 
+	/*
+	 * If td ended up with no I/O when it should have had,
+	 * then something went wrong unless FIO_NOIO or FIO_DISKLESSIO.
+	 * (Are we not missing other flags that can be ignored ?)
+	 */
+	if ((td->o.size || td->o.io_size) && !ddir_rw_sum(bytes_done) &&
+	    !(td_ioengine_flagged(td, FIO_NOIO) ||
+	      td_ioengine_flagged(td, FIO_DISKLESSIO)))
+		log_err("%s: No I/O performed by %s, "
+			 "perhaps try --debug=io option for details?\n",
+			 td->o.name, td->io_ops->name);
+
 	td_set_runstate(td, TD_FINISHING);
 
 	update_rusage_stat(td);
diff --git a/io_u.c b/io_u.c
index 363bfe1..88f35c9 100644
--- a/io_u.c
+++ b/io_u.c
@@ -899,8 +899,9 @@ static int fill_io_u(struct thread_data *td, struct io_u *io_u)
 	}
 
 	if (io_u->offset + io_u->buflen > io_u->file->real_file_size) {
-		dprint(FD_IO, "io_u %p, offset too large\n", io_u);
-		dprint(FD_IO, "  off=%llu/%lu > %llu\n",
+		dprint(FD_IO, "io_u %p, offset + buflen exceeds file size\n",
+			io_u);
+		dprint(FD_IO, "  offset=%llu/buflen=%lu > %llu\n",
 			(unsigned long long) io_u->offset, io_u->buflen,
 			(unsigned long long) io_u->file->real_file_size);
 		return 1;
-- 
2.9.3

[PATCH 2/8] dump_td_info() doesn't really need to be a function

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

since it's short enough, and too specific to be called as dump_td_info().

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 backend.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/backend.c b/backend.c
index e5be187..67b4c6b 100644
--- a/backend.c
+++ b/backend.c
@@ -1861,14 +1861,6 @@ err:
 	return (void *) (uintptr_t) td->error;
 }
 
-static void dump_td_info(struct thread_data *td)
-{
-	log_err("fio: job '%s' (state=%d) hasn't exited in %lu seconds, it "
-		"appears to be stuck. Doing forceful exit of this job.\n",
-			td->o.name, td->runstate,
-			(unsigned long) time_since_now(&td->terminate_time));
-}
-
 /*
  * Run over the job map and reap the threads that have exited, if any.
  */
@@ -1953,7 +1945,11 @@ static void reap_threads(unsigned int *nr_running, uint64_t *t_rate,
 		if (td->terminate &&
 		    td->runstate < TD_FSYNCING &&
 		    time_since_now(&td->terminate_time) >= FIO_REAP_TIMEOUT) {
-			dump_td_info(td);
+			log_err("fio: job '%s' (state=%d) hasn't exited in "
+				"%lu seconds, it appears to be stuck. Doing "
+				"forceful exit of this job.\n",
+				td->o.name, td->runstate,
+				(unsigned long) time_since_now(&td->terminate_time));
 			td_set_runstate(td, TD_REAPED);
 			goto reaped;
 		}
-- 
2.9.3

[PATCH 3/8] Add assert(0) to DDIR_DATASYNC sync path if fdatasync(2) is unsupported

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

If ddir is DDIR_DATASYNC, it means fio supports fdatasync(2), or it
at least compiled on ./configure. If a platform without fdatasync(2)
happens to take DDIR_DATASYNC path in do_io_u_sync(), it's simply wrong
(because ddir should never be DDIR_DATASYNC due to td->o.fdatasync_blocks == 0)
thus should be aborted rather than continue with EINVAL.

This commit also leaves the existing code from #else path to avoid
compilers complain for uninitialized ret variable.

Tested on FreeBSD and DragonFlyBSD.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 HOWTO  | 3 ++-
 init.c | 4 ++++
 io_u.c | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/HOWTO b/HOWTO
index 80b9e75..b9114b4 100644
--- a/HOWTO
+++ b/HOWTO
@@ -1109,7 +1109,8 @@ I/O type
 
 	Like :option:`fsync` but uses :manpage:`fdatasync(2)` to only sync data and
 	not metadata blocks.  In Windows, FreeBSD, and DragonFlyBSD there is no
-	:manpage:`fdatasync(2)`, this falls back to using :manpage:`fsync(2)`.
+	:manpage:`fdatasync(2)`, this falls back to using :manpage:`fsync(2)`
+	by overwriting :manpage:`fsync(2)` value with this option.
 	Defaults to 0, which means no sync data every certain number of writes.
 
 .. option:: write_barrier=int
diff --git a/init.c b/init.c
index 2f9433b..90aaea3 100644
--- a/init.c
+++ b/init.c
@@ -782,6 +782,10 @@ static int fixup_options(struct thread_data *td)
 	}
 
 #ifndef CONFIG_FDATASYNC
+	/*
+	 * If the platform doesn't support fdatasync(2) (e.g. FreeBSD),
+	 * force fsync= using fdatasync= value specified.
+	 */
 	if (o->fdatasync_blocks) {
 		log_info("fio: this platform does not support fdatasync()"
 			 " falling back to using fsync().  Use the 'fsync'"
diff --git a/io_u.c b/io_u.c
index 88f35c9..86c5fc1 100644
--- a/io_u.c
+++ b/io_u.c
@@ -2115,6 +2115,7 @@ int do_io_u_sync(const struct thread_data *td, struct io_u *io_u)
 #else
 		ret = io_u->xfer_buflen;
 		io_u->error = EINVAL;
+		assert(0); /* should be falling back to fsync(2) */
 #endif
 	} else if (io_u->ddir == DDIR_SYNC_FILE_RANGE)
 		ret = do_sync_file_range(td, io_u->file);
-- 
2.9.3

[PATCH 4/8] Make lib/prio_tree.c a stand-alone library

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

lib/prio_tree.c not having dependency on fio.h enables it to be
a stand-alone library, which is useful for debugging purpose.
In fact, most of the files under lib/ directory do things this way.

--
 # cat ./test2.c
 #include <stdio.h>
 #include "lib/prio_tree.h"
 int main(void) {
         struct prio_tree_root tree;
         struct prio_tree_node node;
         INIT_PRIO_TREE_ROOT(&tree);
         INIT_PRIO_TREE_NODE(&node);
         prio_tree_insert(&tree, &node);
         prio_tree_remove(&tree, &node);
         printf("%d\n", prio_tree_empty(&tree));
         return 0;
 }
 # gcc -Wall -g -DBITS_PER_LONG=64 ./test2.c ./lib/prio_tree.c
 # ./a.out
 1

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 lib/prio_tree.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/prio_tree.c b/lib/prio_tree.c
index e18ae32..de3fe1c 100644
--- a/lib/prio_tree.c
+++ b/lib/prio_tree.c
@@ -13,9 +13,12 @@
 
 #include <stdlib.h>
 #include <limits.h>
-#include "../fio.h"
+
+#include "../compiler/compiler.h"
 #include "prio_tree.h"
 
+#define ARRAY_SIZE(x)    (sizeof((x)) / (sizeof((x)[0])))
+
 /*
  * A clever mix of heap and radix trees forms a radix priority search tree (PST)
  * which is useful for storing intervals, e.g, we can consider a vma as a closed
-- 
2.9.3

[PATCH 5/8] Make lib/memalign.c a stand-alone library

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

lib/memalign.c not having dependency on fio.h enables it to be
a stand-alone library, which is useful for debugging purpose.
In fact, most of the files under lib/ directory do things this way.

(For lib/memalign.c, it was actually myself who introduced this
dependency as a part of changes made by 248c9436, so this commit
is partially reverting that)

--
 # cat ./test3.c
 #include <stdio.h>
 #include "lib/memalign.h"
 int main(void) {
         char *p = fio_memalign(4096, 1000000);
         printf("%p\n", p);
         fio_memfree(p, 1000000);
         return 0;
 }
 # gcc -Wall -g ./test3.c ./lib/memalign.c
 # ./a.out
 0x7fbb7bbbc000

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 lib/memalign.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/memalign.c b/lib/memalign.c
index 1d1ba9b..137cc8e 100644
--- a/lib/memalign.c
+++ b/lib/memalign.c
@@ -3,7 +3,9 @@
 #include <inttypes.h>
 
 #include "memalign.h"
-#include "../fio.h"
+
+#define PTR_ALIGN(ptr, mask)   \
+	(char *)((uintptr_t)((ptr) + (mask)) & ~(mask))
 
 struct align_footer {
 	unsigned int offset;
-- 
2.9.3

[PATCH 6/8] Make lib/num2str.c a stand-alone library by adding lib/num2str.h

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

lib/num2str.c not having dependency on fio.h by adding a new header
lib/num2str.h enables it to be a stand-alone library function, which
is useful for debugging purpose. In fact, most of the files under
lib/ directory do things this way.

--
 # cat ./test1.c
 #include <stdio.h>
 #include "lib/num2str.h"
 int main(void) {
         printf("%s\n", num2str(1024, 4, 1, 1, N2S_BYTE));
         return 0;
 }
 # gcc -Wall -g -DCONFIG_STATIC_ASSERT ./test1.c ./lib/num2str.c
 # ./a.out
 1024B

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 fio.h         |  9 +--------
 lib/num2str.c |  7 +++++--
 lib/num2str.h | 15 +++++++++++++++
 3 files changed, 21 insertions(+), 10 deletions(-)
 create mode 100644 lib/num2str.h

diff --git a/fio.h b/fio.h
index 3955a81..2975076 100644
--- a/fio.h
+++ b/fio.h
@@ -35,6 +35,7 @@
 #include "oslib/getopt.h"
 #include "lib/rand.h"
 #include "lib/rbtree.h"
+#include "lib/num2str.h"
 #include "client.h"
 #include "server.h"
 #include "stat.h"
@@ -520,7 +521,6 @@ extern void fio_options_mem_dupe(struct thread_data *);
 extern void td_fill_rand_seeds(struct thread_data *);
 extern void td_fill_verify_state_seed(struct thread_data *);
 extern void add_job_opts(const char **, int);
-extern char *num2str(uint64_t, int, int, int, int);
 extern int ioengine_load(struct thread_data *);
 extern bool parse_dryrun(void);
 extern int fio_running_or_pending_io_threads(void);
@@ -533,13 +533,6 @@ extern uintptr_t page_size;
 extern int initialize_fio(char *envp[]);
 extern void deinitialize_fio(void);
 
-#define N2S_NONE	0
-#define N2S_BITPERSEC 	1	/* match unit_base for bit rates */
-#define N2S_PERSEC	2
-#define N2S_BIT		3
-#define N2S_BYTE	4
-#define N2S_BYTEPERSEC 	8	/* match unit_base for byte rates */
-
 #define FIO_GETOPT_JOB		0x89000000
 #define FIO_GETOPT_IOENGINE	0x98000000
 #define FIO_NR_OPTIONS		(FIO_MAX_OPTS + 128)
diff --git a/lib/num2str.c b/lib/num2str.c
index ed3545d..2f714cc 100644
--- a/lib/num2str.c
+++ b/lib/num2str.c
@@ -2,7 +2,10 @@
 #include <stdio.h>
 #include <string.h>
 
-#include "../fio.h"
+#include "../compiler/compiler.h"
+#include "num2str.h"
+
+#define ARRAY_SIZE(x)    (sizeof((x)) / (sizeof((x)[0])))
 
 /**
  * num2str() - Cheesy number->string conversion, complete with carry rounding error.
@@ -10,7 +13,7 @@
  * @maxlen: max number of digits in the output string (not counting prefix and units)
  * @base: multiplier for num (e.g., if num represents Ki, use 1024)
  * @pow2: select unit prefix - 0=power-of-10 decimal SI, nonzero=power-of-2 binary IEC
- * @units: select units - N2S_* macros defined in fio.h
+ * @units: select units - N2S_* macros defined in num2str.h
  * @returns a malloc'd buffer containing "number[<unit prefix>][<units>]"
  */
 char *num2str(uint64_t num, int maxlen, int base, int pow2, int units)
diff --git a/lib/num2str.h b/lib/num2str.h
new file mode 100644
index 0000000..81358a1
--- /dev/null
+++ b/lib/num2str.h
@@ -0,0 +1,15 @@
+#ifndef FIO_NUM2STR_H
+#define FIO_NUM2STR_H
+
+#include <inttypes.h>
+
+#define N2S_NONE	0
+#define N2S_BITPERSEC	1	/* match unit_base for bit rates */
+#define N2S_PERSEC	2
+#define N2S_BIT		3
+#define N2S_BYTE	4
+#define N2S_BYTEPERSEC	8	/* match unit_base for byte rates */
+
+extern char *num2str(uint64_t, int, int, int, int);
+
+#endif
-- 
2.9.3

[PATCH 7/8] Fix num2str() output when maxlen <= strlen(tmp)

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

Since a local variable decimals is unsigned int, this conditional
 if (decimals <= 1)
needs cast as shown below.
 if ((int)decimals <= 1)

Otherwise it results in showing some garbage in case of maxlen == 0,

 # cat ./test0.c
 #include <stdio.h>
 #include "lib/num2str.h"
 int main(void) {
         printf("%s\n", num2str(123, 3, 1, 1, N2S_BYTE));
         printf("%s\n", num2str(123, 2, 1, 1, N2S_BYTE));
         printf("%s\n", num2str(123, 1, 1, 1, N2S_BYTE));
         printf("%s\n", num2str(123, 0, 1, 1, N2S_BYTE));
         return 0;
 }
 # gcc -Wall -g -DCONFIG_STATIC_ASSERT ./test0.c ./lib/num2str.c
 # ./a.out
 123B
 0KiB
 0KiB
 0.0?$-JB
 # ./a.out
 123B
 0KiB
 0KiB
 0.0#;bB
 # ./a.out
 123B
 0KiB
 0KiB
 0.0|B

whereas with this commit it prints "0B".

 # gcc -Wall -g -DCONFIG_STATIC_ASSERT ./test0.c ./lib/num2str.c
 # ./a.out
 123B
 0KiB
 0KiB
 0B

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 lib/num2str.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/num2str.c b/lib/num2str.c
index 2f714cc..448d3ff 100644
--- a/lib/num2str.c
+++ b/lib/num2str.c
@@ -86,7 +86,7 @@ done:
 
 	sprintf(tmp, "%llu", (unsigned long long) num);
 	decimals = maxlen - strlen(tmp);
-	if (decimals <= 1) {
+	if ((int)decimals <= 1) {
 		if (carry)
 			num++;
 		goto done;
-- 
2.9.3

[PATCH 8/8] Fix num2str() output when modulo != -1U

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

I'm not sure what 05463816 actually fixed (as it only says "fix"),
but this isn't properly working at least for some input numbers unless
".%s" part of sprintf() is meant to be something other than decimal
part of the input number.

This commit might be breaking something that 05463816 had fixed,
which then needs to be rejected, but it at least prints decimal number
as expected. For example, when 12345 is 12.0556640625 KiB, and 23456
is 22.90625 KiB,

 # python -c "print(12345.0 / 1024)"
 12.0556640625
 # python -c "print(23456.0 / 1024)"
 22.90625

running this code as well as fio result in

 # cat ./test1.c
 #include <stdio.h>
 #include "lib/num2str.h"
 int main(void) {
         printf("%s-%s\n",
                 num2str(12345, 4, 1, 1, N2S_BYTE),
                 num2str(23456, 4, 1, 1, N2S_BYTE));
         return 0;
 }
 # gcc -Wall -g -DCONFIG_STATIC_ASSERT ./test1.c ./lib/num2str.c

below with the current implementation

 # ./a.out
 12.6KiB-22.1KiB
 # ./fio --name=xxxxx --ioengine=sync --rw=read --size=1m --unlink=1 --bsrange=12345:23456 | grep "rw=read"
 xxxxx: (g=0): rw=read, bs=(R) 12.6KiB-22.1KiB, (W) 12.6KiB-22.1KiB, (T) 12.6KiB-22.1KiB, ioengine=sync, iodepth=1

whereas below with this commit.

 # ./a.out
 12.1KiB-22.9KiB
 # ./fio --name=xxxxx --ioengine=sync --rw=read --size=1m --unlink=1 --bsrange=12345:23456 | grep "rw=read"
 xxxxx: (g=0): rw=read, bs=(R) 12.1KiB-22.9KiB, (W) 12.1KiB-22.9KiB, (T) 12.1KiB-22.9KiB, ioengine=sync, iodepth=1

-- some more verification with various parameters
 # cat ./testx.c
 #include <stdio.h>
 #include "lib/num2str.h"
 static void f(uint64_t n, int maxlen, int base, int pow2, int units) {
 	printf("%10jd %s\n", n, num2str(n, maxlen, base, pow2, units));
 }
 int main(void) {
 	/* 1 */
 	f(1, 1, 1, 1, N2S_BYTE);
 	f(1, 1, 1, 0, N2S_BYTE);
 	printf("====================\n");
 	/* 10 */
 	f(10, 2, 1, 1, N2S_BYTE);
 	f(10, 2, 1, 0, N2S_BYTE);
 	printf("====================\n");
 	/* 1000 */
 	f(1000, 5, 1, 1, N2S_BYTE);
 	f(1000, 5, 1, 0, N2S_BYTE);
 	f(1000, 4, 1, 1, N2S_BYTE);
 	f(1000, 4, 1, 0, N2S_BYTE);
 	f(1000, 3, 1, 1, N2S_BYTE);
 	f(1000, 3, 1, 0, N2S_BYTE);
 	f(1000, 2, 1, 1, N2S_BYTE);
 	f(1000, 2, 1, 0, N2S_BYTE);
 	f(1000, 1, 1, 1, N2S_BYTE);
 	f(1000, 1, 1, 0, N2S_BYTE);
 	printf("====================\n");
 	/* 1024 */
 	f(1024, 5, 1, 1, N2S_BYTE);
 	f(1024, 5, 1, 0, N2S_BYTE);
 	f(1024, 4, 1, 1, N2S_BYTE);
 	f(1024, 4, 1, 0, N2S_BYTE);
 	f(1024, 3, 1, 1, N2S_BYTE);
 	f(1024, 3, 1, 0, N2S_BYTE);
 	f(1024, 2, 1, 1, N2S_BYTE);
 	f(1024, 2, 1, 0, N2S_BYTE);
 	f(1024, 1, 1, 1, N2S_BYTE);
 	f(1024, 1, 1, 0, N2S_BYTE);
 	printf("====================\n");
 	/* 12345 */
 	f(12345, 6, 1, 1, N2S_BYTE);
 	f(12345, 6, 1, 0, N2S_BYTE);
 	f(12345, 5, 1, 1, N2S_BYTE);
 	f(12345, 5, 1, 0, N2S_BYTE);
 	f(12345, 4, 1, 1, N2S_BYTE);
 	f(12345, 4, 1, 0, N2S_BYTE);
 	f(12345, 3, 1, 1, N2S_BYTE);
 	f(12345, 3, 1, 0, N2S_BYTE);
 	f(12345, 2, 1, 1, N2S_BYTE);
 	f(12345, 2, 1, 0, N2S_BYTE);
 	f(12345, 1, 1, 1, N2S_BYTE);
 	f(12345, 1, 1, 0, N2S_BYTE);
 	printf("====================\n");
 	/* 1234567 */
 	f(1234567, 8, 1, 1, N2S_BYTE);
 	f(1234567, 8, 1, 0, N2S_BYTE);
 	f(1234567, 7, 1, 1, N2S_BYTE);
 	f(1234567, 7, 1, 0, N2S_BYTE);
 	f(1234567, 6, 1, 1, N2S_BYTE);
 	f(1234567, 6, 1, 0, N2S_BYTE);
 	f(1234567, 5, 1, 1, N2S_BYTE);
 	f(1234567, 5, 1, 0, N2S_BYTE);
 	f(1234567, 4, 1, 1, N2S_BYTE);
 	f(1234567, 4, 1, 0, N2S_BYTE);
 	f(1234567, 3, 1, 1, N2S_BYTE);
 	f(1234567, 3, 1, 0, N2S_BYTE);
 	f(1234567, 2, 1, 1, N2S_BYTE);
 	f(1234567, 2, 1, 0, N2S_BYTE);
 	f(1234567, 1, 1, 1, N2S_BYTE);
 	f(1234567, 1, 1, 0, N2S_BYTE);
 	printf("====================\n");
 	/* 1234567 with base 1024 */
 	f(1234567, 8, 1024, 1, N2S_BYTE);
 	f(1234567, 8, 1024, 0, N2S_BYTE);
 	f(1234567, 7, 1024, 1, N2S_BYTE);
 	f(1234567, 7, 1024, 0, N2S_BYTE);
 	f(1234567, 6, 1024, 1, N2S_BYTE);
 	f(1234567, 6, 1024, 0, N2S_BYTE);
 	f(1234567, 5, 1024, 1, N2S_BYTE);
 	f(1234567, 5, 1024, 0, N2S_BYTE);
 	f(1234567, 4, 1024, 1, N2S_BYTE);
 	f(1234567, 4, 1024, 0, N2S_BYTE);
 	f(1234567, 3, 1024, 1, N2S_BYTE);
 	f(1234567, 3, 1024, 0, N2S_BYTE);
 	f(1234567, 2, 1024, 1, N2S_BYTE);
 	f(1234567, 2, 1024, 0, N2S_BYTE);
 	f(1234567, 1, 1024, 1, N2S_BYTE);
 	f(1234567, 1, 1024, 0, N2S_BYTE);
 	printf("====================\n");
 	/* 1234567 with base 1024 with N2S_BIT */
 	f(1234567, 9, 1024, 1, N2S_BIT);
 	f(1234567, 9, 1024, 0, N2S_BIT);
 	f(1234567, 8, 1024, 1, N2S_BIT);
 	f(1234567, 8, 1024, 0, N2S_BIT);
 	f(1234567, 7, 1024, 1, N2S_BIT);
 	f(1234567, 7, 1024, 0, N2S_BIT);
 	f(1234567, 6, 1024, 1, N2S_BIT);
 	f(1234567, 6, 1024, 0, N2S_BIT);
 	f(1234567, 5, 1024, 1, N2S_BIT);
 	f(1234567, 5, 1024, 0, N2S_BIT);
 	f(1234567, 4, 1024, 1, N2S_BIT);
 	f(1234567, 4, 1024, 0, N2S_BIT);
 	f(1234567, 3, 1024, 1, N2S_BIT);
 	f(1234567, 3, 1024, 0, N2S_BIT);
 	f(1234567, 2, 1024, 1, N2S_BIT);
 	f(1234567, 2, 1024, 0, N2S_BIT);
 	f(1234567, 1, 1024, 1, N2S_BIT);
 	f(1234567, 1, 1024, 0, N2S_BIT);
 	printf("====================\n");
 	return 0;
 }
 # gcc -Wall -g -DCONFIG_STATIC_ASSERT ./testx.c ./lib/num2str.c
 # ./a.out
          1 1B
          1 1B
 ====================
         10 10B
         10 10B
 ====================
       1000 1000B
       1000 1000B
       1000 1000B
       1000 1000B
       1000 0.0KiB
       1000 1.0kB
       1000 1KiB
       1000 1kB
       1000 1KiB
       1000 1kB
 ====================
       1024 1024B
       1024 1024B
       1024 1024B
       1024 1024B
       1024 1.0KiB
       1024 1.0kB
       1024 1KiB
       1024 1kB
       1024 1KiB
       1024 1kB
 ====================
      12345 12345B
      12345 12345B
      12345 12345B
      12345 12345B
      12345 12.1KiB
      12345 12.3kB
      12345 12KiB
      12345 12kB
      12345 12KiB
      12345 12kB
      12345 0MiB
      12345 0MB
 ====================
    1234567 1234567B
    1234567 1234567B
    1234567 1234567B
    1234567 1234567B
    1234567 1205.6KiB
    1234567 1234.6kB
    1234567 1206KiB
    1234567 1235kB
    1234567 1206KiB
    1234567 1235kB
    1234567 1.2MiB
    1234567 1.2MB
    1234567 1MiB
    1234567 1MB
    1234567 1MiB
    1234567 1MB
 ====================
    1234567 1234567KiB
    1234567 1234567kB
    1234567 1234567KiB
    1234567 1234567kB
    1234567 1205.6MiB
    1234567 1234.6MB
    1234567 1206MiB
    1234567 1235MB
    1234567 1206MiB
    1234567 1235MB
    1234567 1.2GiB
    1234567 1.2GB
    1234567 1GiB
    1234567 1GB
    1234567 1GiB
    1234567 1GB
 ====================
    1234567 9876536Kibit
    1234567 9876536kbit
    1234567 9876536Kibit
    1234567 9876536kbit
    1234567 9876536Kibit
    1234567 9876536kbit
    1234567 9645.1Mibit
    1234567 9876.5Mbit
    1234567 9645Mibit
    1234567 9877Mbit
    1234567 9645Mibit
    1234567 9877Mbit
    1234567 9.4Gibit
    1234567 9.9Gbit
    1234567 9Gibit
    1234567 10Gbit
    1234567 9Gibit
    1234567 10Gbit
 ====================

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 lib/num2str.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/lib/num2str.c b/lib/num2str.c
index 448d3ff..8d08841 100644
--- a/lib/num2str.c
+++ b/lib/num2str.c
@@ -10,7 +10,7 @@
 /**
  * num2str() - Cheesy number->string conversion, complete with carry rounding error.
  * @num: quantity (e.g., number of blocks, bytes or bits)
- * @maxlen: max number of digits in the output string (not counting prefix and units)
+ * @maxlen: max number of digits in the output string (not counting prefix and units, but counting .)
  * @base: multiplier for num (e.g., if num represents Ki, use 1024)
  * @pow2: select unit prefix - 0=power-of-10 decimal SI, nonzero=power-of-2 binary IEC
  * @units: select units - N2S_* macros defined in num2str.h
@@ -23,9 +23,9 @@ char *num2str(uint64_t num, int maxlen, int base, int pow2, int units)
 	const char **unitprefix;
 	const char *unitstr[] = { "", "/s", "B", "bit", "B/s", "bit/s" };
 	const unsigned int thousand[] = { 1000, 1024 };
-	unsigned int modulo, decimals;
+	unsigned int modulo;
 	int unit_index = 0, post_index, carry = 0;
-	char tmp[32];
+	char tmp[32], fmt[32];
 	char *buf;
 
 	compiletime_assert(sizeof(sistr) == sizeof(iecstr), "unit prefix arrays must be identical sizes");
@@ -62,6 +62,9 @@ char *num2str(uint64_t num, int maxlen, int base, int pow2, int units)
 		break;
 	}
 
+	/*
+	 * Divide by K/Ki until string length of num <= maxlen.
+	 */
 	modulo = -1U;
 	while (post_index < sizeof(sistr)) {
 		sprintf(tmp, "%llu", (unsigned long long) num);
@@ -74,6 +77,9 @@ char *num2str(uint64_t num, int maxlen, int base, int pow2, int units)
 		post_index++;
 	}
 
+	/*
+	 * If no modulo, then we're done.
+	 */
 	if (modulo == -1U) {
 done:
 		if (post_index >= ARRAY_SIZE(sistr))
@@ -84,23 +90,25 @@ done:
 		return buf;
 	}
 
+	/*
+	 * If no room for decimals, then we're done.
+	 */
 	sprintf(tmp, "%llu", (unsigned long long) num);
-	decimals = maxlen - strlen(tmp);
-	if ((int)decimals <= 1) {
+	if ((int)(maxlen - strlen(tmp)) <= 1) {
 		if (carry)
 			num++;
 		goto done;
 	}
 
-	do {
-		sprintf(tmp, "%u", modulo);
-		if (strlen(tmp) <= decimals - 1)
-			break;
-
-		modulo = (modulo + 9) / 10;
-	} while (1);
+	/*
+	 * Fill in everything and return the result.
+	 */
+	assert(maxlen - strlen(tmp) - 1 > 0);
+	assert(modulo < thousand[!!pow2]);
+	sprintf(fmt, "%%.%df", (int)(maxlen - strlen(tmp) - 1));
+	sprintf(tmp, fmt, (double)modulo / (double)thousand[!!pow2]);
 
-	sprintf(buf, "%llu.%u%s%s", (unsigned long long) num, modulo,
+	sprintf(buf, "%llu.%s%s%s", (unsigned long long) num, &tmp[2],
 			unitprefix[post_index], unitstr[unit_index]);
 	return buf;
 }
-- 
2.9.3

RE: [PATCH 8/8] Fix num2str() output when modulo != -1U

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: fio-owner@vger.kernel.org [mailto:fio-owner@vger.kernel.org] On
> Behalf Of kusumi.tomohiro@gmail.com
> Sent: Tuesday, April 4, 2017 3:22 PM
> Subject: [PATCH 8/8] Fix num2str() output when modulo != -1U
...
> @@ -62,6 +62,9 @@ char *num2str(uint64_t num, int maxlen, int base, int
> pow2, int units)
>  		break;
...
> +	sprintf(fmt, "%%.%df", (int)(maxlen - strlen(tmp) - 1));
> +	sprintf(tmp, fmt, (double)modulo / (double)thousand[!!pow2]);

I suspect one of the goals of that function was to restrict itself
to integer math and avoid invoking the floating point unit (which
might not even exist on some CPUs).

Re: [PATCH 3/8] Add assert(0) to DDIR_DATASYNC sync path if fdatasync(2) is unsupported

[Jens Axboe]

On Tue, Apr 04 2017, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> If ddir is DDIR_DATASYNC, it means fio supports fdatasync(2), or it
> at least compiled on ./configure. If a platform without fdatasync(2)
> happens to take DDIR_DATASYNC path in do_io_u_sync(), it's simply wrong
> (because ddir should never be DDIR_DATASYNC due to td->o.fdatasync_blocks == 0)
> thus should be aborted rather than continue with EINVAL.
> 
> This commit also leaves the existing code from #else path to avoid
> compilers complain for uninitialized ret variable.

EINVAL should cause termination, I don't think there's any reason to use
assert here. In general, if we can handle even cases that we think
should not occur, then we should do so instead of dumping core.

-- 
Jens Axboe

Re: [PATCH 1/8] Don't silently terminate td when no I/O performed due to error

[Jens Axboe]

On Tue, Apr 04 2017, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> Some runtime configurations can cause threads/processes to terminate
> without any I/O performed, yet with no explicit error message, which
> is quite confusing. In the example below, fio finishes with neither
> error nor regular statistics due to file offset + bs being larger
> than the file size while in get_io_u().
> 
> This commit calls log_err() when this happens. Since it's difficult
> to tell the exact reason after thread has left the main I/O loop[*],
> it gives advice to use a command line option --debug=io, similar to
> the way td_io_init() gives advice.
> 
> [*] It can't just replace dprint(FD_IO, ...) calls with td_verror()
> for e.g. while in get_io_u(), since those are also used by non error
> paths too.

Thanks, applied 1-2, and 4-7.

-- 
Jens Axboe