From: Vincent Fu Date: Thu, 8 May 2025 18:58:11 +0000 (-0400) Subject: verify: make verify_pattern=%o thread safe X-Git-Tag: fio-3.40~9 X-Git-Url: https://git.kernel.dk/?a=commitdiff_plain;h=7431e1549e49166663ce6625eb9a31ce1833b491;p=fio.git verify: make verify_pattern=%o thread safe 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% Link: https://lore.kernel.org/r/20250508185832.3702-4-vincent.fu@samsung.com Signed-off-by: Vincent Fu --- 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)