public inbox for fio@vger.kernel.org
 help / color / mirror / Atom feed
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


  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