verify: make verify_pattern=%o thread safe
authorVincent Fu <vincentfu@gmail.com>
Thu, 8 May 2025 18:58:11 +0000 (14:58 -0400)
committerVincent Fu <vincent.fu@samsung.com>
Fri, 16 May 2025 16:09:30 +0000 (12:09 -0400)
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 <vincent.fu@samsung.com>
verify.c

index e7e619d3adacc1c8a568642960574644038e098d..c90c008c92f2e23faa078c78a3b50583525f1be4 100644 (file)
--- 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)