From: Vincent Fu <vincentfu@gmail.com>
To: fio@vger.kernel.org, axboe@kernel.dk
Cc: Vincent Fu <vincent.fu@samsung.com>
Subject: [PATCH 03/11] verify: make verify_pattern=%o thread safe
Date: Thu, 8 May 2025 14:58:11 -0400 [thread overview]
Message-ID: <20250508185832.3702-4-vincent.fu@samsung.com> (raw)
In-Reply-To: <20250508185832.3702-1-vincent.fu@samsung.com>
When verify_async is set, multiple threads create instances of
the verify_pattern in the same buffer. This is not a problem if the
pattern is a constant value. However, if the pattern depends on the
offset then pattern verification will produce false failures.
This patch changes verify_io_u_pattern() to allocate brand new
buffers to instantiate the verify_pattern when verify_pattern contains
the %o format specifier and verify_async is set. With each thread having
its own pattern buffer they will no longer interfere with each other.
Failing use case example:
root@localhost:~/fio-dev/fio-canonical# ./fio --name=test --ioengine=io_uring --iodepth=32 --filesize=256k --verify_pattern=%o --verify_async=2 --rw=write
test: (g=0): rw=write, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T) 4096B-4096B, ioengine=io_uring, iodepth=32
fio-3.39-44-g19d9
Starting 1 process
fio: got pattern '10', wanted '70'. Bad bits 2
fio: bad pattern block offset 41
pattern: verify failed at file test.0.0 offset 200704, length 4096 (requested block: offset=200704, length=4096, flags=84)
test: (groupid=0, jobs=1): err= 0: pid=3972816: Tue Apr 22 16:59:55 2025
read: IOPS=21.3k, BW=83.3MiB/s (87.4MB/s)(256KiB/3msec)
slat (nsec): min=3989, max=35487, avg=5319.23, stdev=4033.94
clat (usec): min=43, max=2008, avg=1058.31, stdev=513.19
lat (usec): min=47, max=2012, avg=1063.63, stdev=512.36
clat percentiles (usec):
| 1.00th=[ 44], 5.00th=[ 178], 10.00th=[ 347], 20.00th=[ 570],
| 30.00th=[ 775], 40.00th=[ 930], 50.00th=[ 1074], 60.00th=[ 1237],
| 70.00th=[ 1385], 80.00th=[ 1532], 90.00th=[ 1729], 95.00th=[ 1811],
| 99.00th=[ 2008], 99.50th=[ 2008], 99.90th=[ 2008], 99.95th=[ 2008],
| 99.99th=[ 2008]
write: IOPS=32.0k, BW=125MiB/s (131MB/s)(256KiB/2msec); 0 zone resets
slat (usec): min=2, max=328, avg= 9.50, stdev=40.93
clat (usec): min=250, max=585, avg=382.74, stdev=86.17
lat (usec): min=262, max=588, avg=392.24, stdev=88.98
clat percentiles (usec):
| 1.00th=[ 251], 5.00th=[ 262], 10.00th=[ 293], 20.00th=[ 306],
| 30.00th=[ 330], 40.00th=[ 334], 50.00th=[ 359], 60.00th=[ 404],
| 70.00th=[ 416], 80.00th=[ 474], 90.00th=[ 494], 95.00th=[ 545],
| 99.00th=[ 586], 99.50th=[ 586], 99.90th=[ 586], 99.95th=[ 586],
| 99.99th=[ 586]
lat (usec) : 50=0.78%, 100=0.78%, 250=2.34%, 500=50.78%, 750=9.38%
lat (usec) : 1000=8.59%
lat (msec) : 2=26.56%, 4=0.78%
cpu : usr=0.00%, sys=50.00%, ctx=54, majf=0, minf=15
IO depths : 1=1.6%, 2=3.1%, 4=6.2%, 8=12.5%, 16=25.0%, 32=51.6%, >=64=0.0%
submit : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
complete : 0=0.0%, 4=97.1%, 8=0.0%, 16=0.0%, 32=2.9%, 64=0.0%, >=64=0.0%
issued rwts: total=64,64,0,0 short=0,0,0,0 dropped=0,0,0,0
latency : target=0, window=0, percentile=100.00%, depth=32
Run status group 0 (all jobs):
READ: bw=83.3MiB/s (87.4MB/s), 83.3MiB/s-83.3MiB/s (87.4MB/s-87.4MB/s), io=256KiB (262kB), run=3-3msec
WRITE: bw=125MiB/s (131MB/s), 125MiB/s-125MiB/s (131MB/s-131MB/s), io=256KiB (262kB), run=2-2msec
Disk stats (read/write):
sda: ios=0/0, sectors=0/0, merge=0/0, ticks=0/0, in_queue=0, util=0.00%
Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
verify.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/verify.c b/verify.c
index e7e619d3..c90c008c 100644
--- a/verify.c
+++ b/verify.c
@@ -373,6 +373,20 @@ static inline void *io_u_verify_off(struct verify_header *hdr, struct vcont *vc)
return vc->io_u->buf + vc->hdr_num * hdr->len + hdr_size(vc->td, hdr);
}
+/*
+ * The current thread will need its own buffer if there are multiple threads
+ * and the pattern contains the offset. Fio currently only has one pattern
+ * format specifier so we only need to check that one, but this may need to be
+ * changed if fio ever gains more pattern format specifiers.
+ */
+static inline bool pattern_need_buffer(struct thread_data *td)
+{
+ return td->o.verify_async &&
+ td->o.verify_fmt_sz &&
+ td->o.verify_fmt[0].desc->paste == paste_blockoff;
+}
+
+
static int verify_io_u_pattern(struct verify_header *hdr, struct vcont *vc)
{
struct thread_data *td = vc->td;
@@ -386,6 +400,16 @@ static int verify_io_u_pattern(struct verify_header *hdr, struct vcont *vc)
pattern_size = td->o.verify_pattern_bytes;
assert(pattern_size != 0);
+ /*
+ * Make this thread safe when verify_async is set and the verify
+ * pattern includes the offset.
+ */
+ if (pattern_need_buffer(td)) {
+ pattern = malloc(pattern_size);
+ assert(pattern);
+ memcpy(pattern, td->o.verify_pattern, pattern_size);
+ }
+
(void)paste_format_inplace(pattern, pattern_size,
td->o.verify_fmt, td->o.verify_fmt_sz, io_u);
@@ -395,7 +419,7 @@ static int verify_io_u_pattern(struct verify_header *hdr, struct vcont *vc)
rc = cmp_pattern(pattern, pattern_size, mod, buf, len);
if (!rc)
- return 0;
+ goto done;
/* Slow path, compare each byte */
for (i = 0; i < len; i++) {
@@ -411,16 +435,18 @@ static int verify_io_u_pattern(struct verify_header *hdr, struct vcont *vc)
i + header_size);
vc->name = "pattern";
log_verify_failure(hdr, vc);
- return EILSEQ;
+ rc = EILSEQ;
+ goto done;
}
mod++;
if (mod == td->o.verify_pattern_bytes)
mod = 0;
}
- /* Unreachable line */
- assert(0);
- return EILSEQ;
+done:
+ if (pattern_need_buffer(td))
+ free(pattern);
+ return rc;
}
static int verify_io_u_xxhash(struct verify_header *hdr, struct vcont *vc)
--
2.47.2
next prev parent reply other threads:[~2025-05-08 18:59 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-08 18:58 [PATCH 00/11] verify pattern interval Vincent Fu
2025-05-08 18:58 ` [PATCH 01/11] verify: add verify mode for a pattern with header Vincent Fu
2025-05-08 18:58 ` [PATCH 02/11] verify: fix verify_offset when used with pattern_hdr Vincent Fu
2025-05-08 18:58 ` Vincent Fu [this message]
2025-05-08 18:58 ` [PATCH 04/11] verify: omit verify type mismatch error message for pattern verify Vincent Fu
2025-05-08 18:58 ` [PATCH 05/11] t/fiotestcommon: lengthen timeout for longer tests Vincent Fu
2025-05-08 18:58 ` [PATCH 06/11] ci: for nightly verify tests use all checksum methods Vincent Fu
2025-05-08 18:58 ` [PATCH 07/11] ci: don't skip verify tests when triggered manually Vincent Fu
2025-05-08 18:58 ` [PATCH 08/11] verify: add verify_pattern_interval option Vincent Fu
2025-05-08 18:58 ` [PATCH 09/11] t/verify: test cases for running pattern and pattern_hdr Vincent Fu
2025-05-08 18:58 ` [PATCH 10/11] t/verify: Windows --output work-around Vincent Fu
2025-05-08 18:58 ` [PATCH 11/11] t/verify: add tests to exercise verify_pattern_interval Vincent Fu
2025-05-08 19:49 ` [PATCH 00/11] verify pattern interval fiotestbot
2025-05-09 0:54 ` Fio Test Bot
2025-05-16 18:08 ` Fio Test Bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250508185832.3702-4-vincent.fu@samsung.com \
--to=vincentfu@gmail.com \
--cc=axboe@kernel.dk \
--cc=fio@vger.kernel.org \
--cc=vincent.fu@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox