rand_seed truncation issue on Windows

[mingnus]

trafficstars

Some functions use unsigned long to store the rand_seed, which leads to value truncation when using LLP64 compilers:

1. The random buffer is not fully initialized. The upper 32-bit is zeroed.
2. The rand_seed in the state header is also truncated. fio cannot verify it when --rw=readwrite. For example, the following command doesn't work on Windows:

   $ fio --filename=test.bin --name=test --size=1m --rw=rw --verify=md5
   

I also have doubt on the following if-statement: Should it be AND or OR?

int verify_io_u(struct thread_data *td, struct io_u **io_u_ptr)
{
    ....
                /*
                 * Make rand_seed check pass when have verify_backlog.
                 */
                if (!td_rw(td) || (td->flags & TD_F_VER_BACKLOG))
                        io_u->rand_seed = hdr->rand_seed;

If there's no requirement on backward compatibility, then we should replace the data type by unsigned long long. Here's my patch for review:

diff --git a/lib/rand.c b/lib/rand.c
index f18bd8d8..c166d3a6 100644
--- a/lib/rand.c
+++ b/lib/rand.c
@@ -95,7 +95,7 @@ void init_rand_seed(struct frand_state *state, unsigned int seed, bool use64)
 		__init_rand64(&state->state64, seed);
 }
 
-void __fill_random_buf(void *buf, unsigned int len, unsigned long seed)
+void __fill_random_buf(void *buf, unsigned int len, unsigned long long seed)
 {
 	void *ptr = buf;
 
@@ -122,10 +122,10 @@ void __fill_random_buf(void *buf, unsigned int len, unsigned long seed)
 	}
 }
 
-unsigned long fill_random_buf(struct frand_state *fs, void *buf,
+unsigned long long fill_random_buf(struct frand_state *fs, void *buf,
 			      unsigned int len)
 {
-	unsigned long r = __rand(fs);
+	unsigned long long r = __rand(fs);
 
 	if (sizeof(int) != sizeof(long *))
 		r *= (unsigned long) __rand(fs);
@@ -134,7 +134,7 @@ unsigned long fill_random_buf(struct frand_state *fs, void *buf,
 	return r;
 }
 
-void __fill_random_buf_percentage(unsigned long seed, void *buf,
+void __fill_random_buf_percentage(unsigned long long seed, void *buf,
 				  unsigned int percentage,
 				  unsigned int segment, unsigned int len,
 				  char *pattern, unsigned int pbytes)
@@ -183,12 +183,12 @@ void __fill_random_buf_percentage(unsigned long seed, void *buf,
 	}
 }
 
-unsigned long fill_random_buf_percentage(struct frand_state *fs, void *buf,
+unsigned long long fill_random_buf_percentage(struct frand_state *fs, void *buf,
 					 unsigned int percentage,
 					 unsigned int segment, unsigned int len,
 					 char *pattern, unsigned int pbytes)
 {
-	unsigned long r = __rand(fs);
+	unsigned long long r = __rand(fs);
 
 	if (sizeof(int) != sizeof(long *))
 		r *= (unsigned long) __rand(fs);
diff --git a/lib/rand.h b/lib/rand.h
index 1676cf98..117d8974 100644
--- a/lib/rand.h
+++ b/lib/rand.h
@@ -150,9 +150,9 @@ static inline uint64_t rand_between(struct frand_state *state, uint64_t start,
 
 extern void init_rand(struct frand_state *, bool);
 extern void init_rand_seed(struct frand_state *, unsigned int seed, bool);
-extern void __fill_random_buf(void *buf, unsigned int len, unsigned long seed);
-extern unsigned long fill_random_buf(struct frand_state *, void *buf, unsigned int len);
-extern void __fill_random_buf_percentage(unsigned long, void *, unsigned int, unsigned int, unsigned int, char *, unsigned int);
-extern unsigned long fill_random_buf_percentage(struct frand_state *, void *, unsigned int, unsigned int, unsigned int, char *, unsigned int);
+extern void __fill_random_buf(void *buf, unsigned int len, unsigned long long seed);
+extern unsigned long long fill_random_buf(struct frand_state *, void *buf, unsigned int len);
+extern void __fill_random_buf_percentage(unsigned long long, void *, unsigned int, unsigned int, unsigned int, char *, unsigned int);
+extern unsigned long long fill_random_buf_percentage(struct frand_state *, void *, unsigned int, unsigned int, unsigned int, char *, unsigned int);
 
 #endif
diff --git a/verify.c b/verify.c
index da429e79..7d01a611 100644
--- a/verify.c
+++ b/verify.c
@@ -45,7 +45,7 @@ static void __fill_buffer(struct thread_options *o, unsigned long seed, void *p,
 	__fill_random_buf_percentage(seed, p, o->compress_percentage, len, len, o->buffer_pattern, o->buffer_pattern_bytes);
 }
 
-static unsigned long fill_buffer(struct thread_data *td, void *p,
+static unsigned long long fill_buffer(struct thread_data *td, void *p,
 				 unsigned int len)
 {
 	struct frand_state *fs = &td->verify_state;

[axboe]

Can you submit this as a proper patch? I can't apply something from a comment. The patch looks fine, it's the eternal w32 issue with unsigned long. But can you change it to an uint64_t instead? That's what we use in most other places, the ones that don't pre-date that usage.

[sitsofe]

There's now a patch over on https://github.com/axboe/fio/pull/768/commits/9781c080385364d24ac18a95544484864a27611a .

[mingnus]

One more question: To generate identical outputs on all the platforms, should we always use 64-bit GOLDEN_RATIO_PRIME?

[sitsofe]

@mingnus This sounds desirable assuming it won't make things dramatically slower. @axboe any thoughts?

[axboe]

Don't feel that strongly about it, but a ioengine=null comparison between the two might be useful. I doubt that many are running 32-bit builds these days anyway.

[sitsofe]

@mingnus any follow up to @axboe's comment?

[mingnus]

Hi @axboe @sitsofe ,

You're right. I make a mistake. My previous commit dramatically decreases the buffer-filling performance of 32-bit builds. Here's the test result of 32-bit Linux builds. When using --refill-buffers, the commit degrades the throughput to 67%. The throughput then drops again with 64-bit GOLDEN_RATIO_PRIME.

Processor	w/o 96416da5	with 96416da5	96416da5 + 64-bit PRIME
Celeron N3150	1500 MB/s	1012 MB/s	785 MB/s
i5-3470	4121 MB/s	2715 MB/s	1696 MB/s

If you agree, I could send a revert commit. But I haven't figure out a better solution. Any suggestion?

Issues for discussion:

• The rand_seed in the verification header was truncated. Scope:
  • Windows 64-bit build
  • Any 32-bit build with --random_generator=tausworthe64
• The random buffer is not fully initialized. It might affect the result of dedup and compression. Scope:
  • Windows 64-bit build
  • Any 32-bit build

---

Compile options:

./configure --extra-cflags=-m32; EXTCFLAGS=-m32 make V=1

Test configurations:

[global]
rw=write
ioengine=null
refill_buffers
time_based
runtime=100s
[test]
filename=test.bin
size=1m