* [PATCH v2 1/4] verify: fix bytes_done accounting of experimental verify
2022-10-20 6:38 [PATCH v2 0/4] fix experimental verify Shin'ichiro Kawasaki
@ 2022-10-20 6:38 ` Shin'ichiro Kawasaki
2022-10-20 6:38 ` [PATCH v2 2/4] verify: fix numberio " Shin'ichiro Kawasaki
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Shin'ichiro Kawasaki @ 2022-10-20 6:38 UTC (permalink / raw)
To: fio, Jens Axboe, Vincent Fu
Cc: Damien Le Moal, Dmitry Fomichev, Niklas Cassel,
Shin'ichiro Kawasaki
The commit 55312f9f5572 ("Add ->bytes_done[] to struct thread_data")
moved bytes_done[] on stack to struct thread_data. However, this unified
two bytes_done[] in do_io() and do_verify() stacks into single
td->bytes_done[]. This caused wrong condition check in do_verify() in
experimental verify path since td->bytes_done[] holds values for do_io()
not for do_verify(). This caused unexpected loop break in do_verify()
and verify read skip when experimental_verify=1 option is specified.
To fix this, add bytes_verified to struct thread_data for do_verify() in
same manner as bytes_done[] for do_io(). Introduce a helper function
io_u_update_bytes_done() to factor out same code for bytes_done[] and
bytes_verified[].
Fixes: 55312f9f5572 ("Add ->bytes_done[] to struct thread_data")
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
backend.c | 2 +-
fio.h | 1 +
io_u.c | 23 +++++++++++++++++------
libfio.c | 1 +
rate-submit.c | 2 ++
5 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/backend.c b/backend.c
index d8f4f2a5..15c6e0b3 100644
--- a/backend.c
+++ b/backend.c
@@ -682,7 +682,7 @@ static void do_verify(struct thread_data *td, uint64_t verify_bytes)
break;
}
} else {
- if (ddir_rw_sum(td->bytes_done) + td->o.rw_min_bs > verify_bytes)
+ if (td->bytes_verified + td->o.rw_min_bs > verify_bytes)
break;
while ((io_u = get_io_u(td)) != NULL) {
diff --git a/fio.h b/fio.h
index de7eca79..0592a4c3 100644
--- a/fio.h
+++ b/fio.h
@@ -370,6 +370,7 @@ struct thread_data {
uint64_t zone_bytes;
struct fio_sem *sem;
uint64_t bytes_done[DDIR_RWDIR_CNT];
+ uint64_t bytes_verified;
uint64_t *thinktime_blocks_counter;
struct timespec last_thinktime;
diff --git a/io_u.c b/io_u.c
index 91f1a358..8035f4b7 100644
--- a/io_u.c
+++ b/io_u.c
@@ -2121,13 +2121,26 @@ static void ios_completed(struct thread_data *td,
}
}
+static void io_u_update_bytes_done(struct thread_data *td,
+ struct io_completion_data *icd)
+{
+ int ddir;
+
+ if (td->runstate == TD_VERIFYING) {
+ td->bytes_verified += icd->bytes_done[DDIR_READ];
+ return;
+ }
+
+ for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++)
+ td->bytes_done[ddir] += icd->bytes_done[ddir];
+}
+
/*
* Complete a single io_u for the sync engines.
*/
int io_u_sync_complete(struct thread_data *td, struct io_u *io_u)
{
struct io_completion_data icd;
- int ddir;
init_icd(td, &icd, 1);
io_completed(td, &io_u, &icd);
@@ -2140,8 +2153,7 @@ int io_u_sync_complete(struct thread_data *td, struct io_u *io_u)
return -1;
}
- for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++)
- td->bytes_done[ddir] += icd.bytes_done[ddir];
+ io_u_update_bytes_done(td, &icd);
return 0;
}
@@ -2153,7 +2165,7 @@ int io_u_queued_complete(struct thread_data *td, int min_evts)
{
struct io_completion_data icd;
struct timespec *tvp = NULL;
- int ret, ddir;
+ int ret;
struct timespec ts = { .tv_sec = 0, .tv_nsec = 0, };
dprint(FD_IO, "io_u_queued_complete: min=%d\n", min_evts);
@@ -2179,8 +2191,7 @@ int io_u_queued_complete(struct thread_data *td, int min_evts)
return -1;
}
- for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++)
- td->bytes_done[ddir] += icd.bytes_done[ddir];
+ io_u_update_bytes_done(td, &icd);
return ret;
}
diff --git a/libfio.c b/libfio.c
index 1a891776..ac521974 100644
--- a/libfio.c
+++ b/libfio.c
@@ -94,6 +94,7 @@ static void reset_io_counters(struct thread_data *td, int all)
td->rate_next_io_time[ddir] = 0;
td->last_usec[ddir] = 0;
}
+ td->bytes_verified = 0;
}
td->zone_bytes = 0;
diff --git a/rate-submit.c b/rate-submit.c
index 268356d1..2fe768c0 100644
--- a/rate-submit.c
+++ b/rate-submit.c
@@ -263,6 +263,8 @@ static void sum_ddir(struct thread_data *dst, struct thread_data *src,
sum_val(&dst->this_io_blocks[ddir], &src->this_io_blocks[ddir]);
sum_val(&dst->this_io_bytes[ddir], &src->this_io_bytes[ddir]);
sum_val(&dst->bytes_done[ddir], &src->bytes_done[ddir]);
+ if (ddir == DDIR_READ)
+ sum_val(&dst->bytes_verified, &src->bytes_verified);
pthread_double_unlock(&dst->io_wq.stat_lock, &src->io_wq.stat_lock);
}
--
2.37.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/4] verify: fix numberio accounting of experimental verify
2022-10-20 6:38 [PATCH v2 0/4] fix experimental verify Shin'ichiro Kawasaki
2022-10-20 6:38 ` [PATCH v2 1/4] verify: fix bytes_done accounting of " Shin'ichiro Kawasaki
@ 2022-10-20 6:38 ` Shin'ichiro Kawasaki
2022-10-20 6:38 ` [PATCH v2 3/4] test: add test for verify read back " Shin'ichiro Kawasaki
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Shin'ichiro Kawasaki @ 2022-10-20 6:38 UTC (permalink / raw)
To: fio, Jens Axboe, Vincent Fu
Cc: Damien Le Moal, Dmitry Fomichev, Niklas Cassel,
Shin'ichiro Kawasaki
As for non-experimental verify, numberio is compared between the numbers
saved in metadata and written data header. As for experimental verify,
the metadata is not available. Instead of numberio in metadata, it
refers td->io_issues[] as the numberio value for the comparison.
However, td->io_issues[] is used not only for verify reads but also for
normal I/Os. It results in comparison with wrong numberio value and
verification failure.
Fix this issue by adding a new field td->verify_read_issues which counts
up number of verify reads. Substitute td->verify_read_issues to
io_u->numberio to refer it for the comparison in experimental verify
path. Also move td->io_issues[] substitution to io_u->numberio out of
populate_verify_io_u() to keep same behavior in non-experimental verify
path.
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
backend.c | 6 +++++-
fio.h | 1 +
verify.c | 2 --
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/backend.c b/backend.c
index 15c6e0b3..ba954a6b 100644
--- a/backend.c
+++ b/backend.c
@@ -711,6 +711,8 @@ static void do_verify(struct thread_data *td, uint64_t verify_bytes)
break;
} else if (io_u->ddir == DDIR_WRITE) {
io_u->ddir = DDIR_READ;
+ io_u->numberio = td->verify_read_issues;
+ td->verify_read_issues++;
populate_verify_io_u(td, io_u);
break;
} else {
@@ -1030,8 +1032,10 @@ static void do_io(struct thread_data *td, uint64_t *bytes_done)
break;
}
- if (io_u->ddir == DDIR_WRITE && td->flags & TD_F_DO_VERIFY)
+ if (io_u->ddir == DDIR_WRITE && td->flags & TD_F_DO_VERIFY) {
+ io_u->numberio = td->io_issues[io_u->ddir];
populate_verify_io_u(td, io_u);
+ }
ddir = io_u->ddir;
diff --git a/fio.h b/fio.h
index 0592a4c3..8da77640 100644
--- a/fio.h
+++ b/fio.h
@@ -356,6 +356,7 @@ struct thread_data {
* Issue side
*/
uint64_t io_issues[DDIR_RWDIR_CNT];
+ uint64_t verify_read_issues;
uint64_t io_issue_bytes[DDIR_RWDIR_CNT];
uint64_t loops;
diff --git a/verify.c b/verify.c
index 0e1e4639..d6a229ca 100644
--- a/verify.c
+++ b/verify.c
@@ -1287,8 +1287,6 @@ void populate_verify_io_u(struct thread_data *td, struct io_u *io_u)
if (td->o.verify == VERIFY_NULL)
return;
- io_u->numberio = td->io_issues[io_u->ddir];
-
fill_pattern_headers(td, io_u, 0, 0);
}
--
2.37.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/4] test: add test for verify read back of experimental verify
2022-10-20 6:38 [PATCH v2 0/4] fix experimental verify Shin'ichiro Kawasaki
2022-10-20 6:38 ` [PATCH v2 1/4] verify: fix bytes_done accounting of " Shin'ichiro Kawasaki
2022-10-20 6:38 ` [PATCH v2 2/4] verify: fix numberio " Shin'ichiro Kawasaki
@ 2022-10-20 6:38 ` Shin'ichiro Kawasaki
2022-10-20 6:38 ` [PATCH v2 4/4] test: add test for experimental verify with loops and time_based options Shin'ichiro Kawasaki
2022-10-24 18:26 ` [PATCH v2 0/4] fix experimental verify Vincent Fu
4 siblings, 0 replies; 6+ messages in thread
From: Shin'ichiro Kawasaki @ 2022-10-20 6:38 UTC (permalink / raw)
To: fio, Jens Axboe, Vincent Fu
Cc: Damien Le Moal, Dmitry Fomichev, Niklas Cassel,
Shin'ichiro Kawasaki
Add a test case to confirm fix of bytes_done accounting issue of
experimental verify. The test job runs a simple workload with
experimental verify and confirm verify read amount is the file size.
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
t/jobs/t0025.fio | 8 ++++++++
t/run-fio-tests.py | 22 ++++++++++++++++++++++
2 files changed, 30 insertions(+)
create mode 100644 t/jobs/t0025.fio
diff --git a/t/jobs/t0025.fio b/t/jobs/t0025.fio
new file mode 100644
index 00000000..2b35a9ac
--- /dev/null
+++ b/t/jobs/t0025.fio
@@ -0,0 +1,8 @@
+[job]
+filename=t0025file
+size=128k
+readwrite=write
+do_verify=1
+verify=md5
+experimental_verify=1
+
diff --git a/t/run-fio-tests.py b/t/run-fio-tests.py
index df87ae72..439991a1 100755
--- a/t/run-fio-tests.py
+++ b/t/run-fio-tests.py
@@ -788,6 +788,18 @@ class FioJobTest_t0024(FioJobTest_t0023):
self.check_all_offsets("bssplit_bw.log", 512, filesize)
+class FioJobTest_t0025(FioJobTest):
+ """Test experimental verify read backs written data pattern."""
+ def check_result(self):
+ super(FioJobTest_t0025, self).check_result()
+
+ if not self.passed:
+ return
+
+ if self.json_data['jobs'][0]['read']['io_kbytes'] != 128:
+ self.passed = False
+
+
class FioJobTest_iops_rate(FioJobTest):
"""Test consists of fio test job t0009
Confirm that job0 iops == 1000
@@ -1183,6 +1195,16 @@ TEST_LIST = [
'pre_success': None,
'requirements': [],
},
+ {
+ 'test_id': 25,
+ 'test_class': FioJobTest_t0025,
+ 'job': 't0025.fio',
+ 'success': SUCCESS_DEFAULT,
+ 'pre_job': None,
+ 'pre_success': None,
+ 'output_format': 'json',
+ 'requirements': [],
+ },
{
'test_id': 1000,
'test_class': FioExeTest,
--
2.37.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 4/4] test: add test for experimental verify with loops and time_based options
2022-10-20 6:38 [PATCH v2 0/4] fix experimental verify Shin'ichiro Kawasaki
` (2 preceding siblings ...)
2022-10-20 6:38 ` [PATCH v2 3/4] test: add test for verify read back " Shin'ichiro Kawasaki
@ 2022-10-20 6:38 ` Shin'ichiro Kawasaki
2022-10-24 18:26 ` [PATCH v2 0/4] fix experimental verify Vincent Fu
4 siblings, 0 replies; 6+ messages in thread
From: Shin'ichiro Kawasaki @ 2022-10-20 6:38 UTC (permalink / raw)
To: fio, Jens Axboe, Vincent Fu
Cc: Damien Le Moal, Dmitry Fomichev, Niklas Cassel,
Shin'ichiro Kawasaki
Add a test case to confirm fix of numberio accounting issue of
experimental verify using loops and time_based options.
Of note is that this case fails on Windows with error "bad header
rand_seed". Until it gets fixed, require to non-Windows OS for this test
case so that CI does not report the known failure.
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
t/jobs/t0026.fio | 20 ++++++++++++++++++++
t/run-fio-tests.py | 9 +++++++++
2 files changed, 29 insertions(+)
create mode 100644 t/jobs/t0026.fio
diff --git a/t/jobs/t0026.fio b/t/jobs/t0026.fio
new file mode 100644
index 00000000..4436392f
--- /dev/null
+++ b/t/jobs/t0026.fio
@@ -0,0 +1,20 @@
+[job1]
+filename=t0026file
+size=1M
+readwrite=randwrite
+loops=8
+do_verify=1
+verify=md5
+experimental_verify=1
+
+[job2]
+stonewall=1
+filename=t0026file
+size=1M
+readwrite=randrw
+time_based
+runtime=5
+do_verify=1
+verify=md5
+experimental_verify=1
+
diff --git a/t/run-fio-tests.py b/t/run-fio-tests.py
index 439991a1..e5b307ac 100755
--- a/t/run-fio-tests.py
+++ b/t/run-fio-tests.py
@@ -1205,6 +1205,15 @@ TEST_LIST = [
'output_format': 'json',
'requirements': [],
},
+ {
+ 'test_id': 26,
+ 'test_class': FioJobTest,
+ 'job': 't0026.fio',
+ 'success': SUCCESS_DEFAULT,
+ 'pre_job': None,
+ 'pre_success': None,
+ 'requirements': [Requirements.not_windows],
+ },
{
'test_id': 1000,
'test_class': FioExeTest,
--
2.37.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/4] fix experimental verify
2022-10-20 6:38 [PATCH v2 0/4] fix experimental verify Shin'ichiro Kawasaki
` (3 preceding siblings ...)
2022-10-20 6:38 ` [PATCH v2 4/4] test: add test for experimental verify with loops and time_based options Shin'ichiro Kawasaki
@ 2022-10-24 18:26 ` Vincent Fu
4 siblings, 0 replies; 6+ messages in thread
From: Vincent Fu @ 2022-10-24 18:26 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, fio, Jens Axboe
Cc: Damien Le Moal, Dmitry Fomichev, Niklas Cassel
On 10/20/22 02:38, Shin'ichiro Kawasaki wrote:
> Recently I noticed that verify workload with --experimental_verify option does
> not read back verify data. I confirmed it with simple job below:
>
> [job]
> filename=t0025file
> size=128k
> readwrite=write
> do_verify=1
> verify=md5
> experimental_verify=1
>
> Fio reported as follows. No read for verify data.
>
> Run status group 0 (all jobs):
> READ: bw=0B/s (0B/s), 0B/s-0B/s (0B/s-0B/s), io=0B (0B), run=1-1msec
> WRITE: bw=41.7MiB/s (43.7MB/s), 41.7MiB/s-41.7MiB/s (43.7MB/s-43.7MB/s)...
>
> The debug log trace with --debug=io,verify option showed no verify data read
> either. It indicates that experimental verify is not working as expected.
>
> This series addresses two issues to make experimental verify work again. Also
> add two test cases to confirm the issue fixes.
>
> Of note is that this series makes test case #54 of t/zbd/test-zbd-support fail.
> I'm preparing another series to address it as well as other verify issues
> related to zonemode=zbd.
>
> Changes from v1:
> * Reflected comments on the list to patch 3 and 4
>
> Shin'ichiro Kawasaki (4):
> verify: fix bytes_done accounting of experimental verify
> verify: fix numberio accounting of experimental verify
> test: add test for verify read back of experimental verify
> test: add test for experimental verify with loops and time_based
> options
>
> backend.c | 8 ++++++--
> fio.h | 2 ++
> io_u.c | 23 +++++++++++++++++------
> libfio.c | 1 +
> rate-submit.c | 2 ++
> t/jobs/t0025.fio | 8 ++++++++
> t/jobs/t0026.fio | 20 ++++++++++++++++++++
> t/run-fio-tests.py | 31 +++++++++++++++++++++++++++++++
> verify.c | 2 --
> 9 files changed, 87 insertions(+), 10 deletions(-)
> create mode 100644 t/jobs/t0025.fio
> create mode 100644 t/jobs/t0026.fio
>
Applied. Thanks.
Vincent
^ permalink raw reply [flat|nested] 6+ messages in thread