Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verification header not laid out as often as --verify_interval requires #522

Closed
yurzo opened this issue Jan 31, 2018 · 15 comments
Closed

Verification header not laid out as often as --verify_interval requires #522

yurzo opened this issue Jan 31, 2018 · 15 comments

Comments

@yurzo
Copy link

yurzo commented Jan 31, 2018

../fio/fio --rw=write --size=32K --io_size=32K --direct=1 --ioengine=libaio --name=carlos --bs=8K --filename=test2.bin --verify_interval=4K --verify=md5 --filename test.bin
hexdump -C test.bin | grep -E "0000.[0,2,4,8,A,C,E]00"
00000000  ca ac 02 00 00 20 00 00  2e 56 db f6 8c 0a bb 88  |..... ...V......|
00000200  08 d1 14 d6 4e 04 98 11  21 9a 3e 72 7f 95 43 15  |....N...!.>r..C.|
00000400  19 2e cf 7b 11 2b af 05  c3 65 6d 98 7a 27 9c 1b  |...{.+...em.z'..|
00000800  40 dc e9 80 eb 09 f0 00  88 3b fd 81 c8 40 e8 10  |@........;...@..|
00001000  e5 24 57 ea e4 bc c4 00  9c 64 d8 0a 71 22 a9 0e  |.$W......d..q"..|
00001200  bf ff f1 b9 cc da 61 12  f7 bf 5e 97 60 de 5e 0f  |......a...^.`.^.|
00001400  fb ef 1d 2a 75 31 17 0d  ff 3d 46 ad 1f 51 05 16  |...*u1...=F..Q..|
00001800  01 27 11 81 0f 32 c1 1a  e0 a4 a1 5c b9 65 17 0f  |.'...2.....\.e..|
00002000  ca ac 02 00 00 20 00 00  34 94 94 93 9a 33 7a 18  |..... ..4....3z.|
00002200  05 53 6d 84 c8 e4 ea 08  60 2a 0b 67 62 1a 9c 16  |.Sm.....`*.gb...|
00002400  89 44 91 d5 67 10 66 06  91 a8 6d 58 44 d7 17 0a  |.D..g.f...mXD...|
00002800  30 24 94 b9 16 07 1f 02  86 84 1a c5 18 84 08 0e  |0$..............|
00003000  21 d6 46 92 7f 23 4c 11  c4 5a 38 87 4c bb 10 17  |!.F..#L..Z8.L...|
00003200  13 98 0f d9 3c 91 11 0b  02 73 18 cf 9f c5 88 18  |....<....s......|
00003400  5c 37 51 bb 03 b4 6e 0e  eb 26 bc db d7 f8 4f 04  |\7Q...n..&....O.|
00003800  dd f6 45 2e 99 ac 38 1d  db 3e 5a 2a 70 7e 25 1a  |..E...8..>Z*p~%.|
00004000

only the 8K aligned seems to have the header
I think this is also related to #163

@yurzo
Copy link
Author

yurzo commented Jan 31, 2018

this is a better repro:

../fio/fio --rw=write --size=32K --io_size=32K --direct=1 --ioengine=libaio --name=carlos --bs=8K --filename=test.bin --verify_interval=512 --verify=crc32c && hexdump -Cv test.bin | grep -E "  ca ac"
carlos: (g=0): rw=write, bs=(R) 8192B-8192B, (W) 8192B-8192B, (T) 8192B-8192B, ioengine=libaio, iodepth=1
fio-3.3-31-gca65
Starting 1 process
...
Run status group 0 (all jobs):
   READ: bw=3200KiB/s (3277kB/s), 3200KiB/s-3200KiB/s (3277kB/s-3277kB/s), io=32.0KiB (32.8kB), run=10-10msec
  WRITE: bw=31.2MiB/s (32.8MB/s), 31.2MiB/s-31.2MiB/s (32.8MB/s-32.8MB/s), io=32.0KiB (32.8kB), run=1-1msec

Disk stats (read/write):
  sda: ios=0/0, merge=0/0, ticks=0/0, in_queue=0, util=0.00%
00000000  ca ac 05 00 00 20 00 00  3c 54 09 c5 01 cc a4 00  |..... ..<T......|
00002000  ca ac 05 00 00 20 00 00  2e 56 db f6 8c 0a bb 88  |..... ...V......|
00004000  ca ac 05 00 00 20 00 00  d0 92 04 ac f3 14 98 41  |..... .........A|
00006000  ca ac 05 00 00 20 00 00  34 94 94 93 9a 33 7a 18  |..... ..4....3z.|

@yurzo
Copy link
Author

yurzo commented Feb 9, 2018

@axboe: I think I have a fix for this. However I'm not being able to create a branch. Could it be I'm missing permissions?

@axboe
Copy link
Owner

axboe commented Feb 9, 2018 via email

yurzo pushed a commit to yurzo/fio that referenced this issue Feb 9, 2018
The operands seems to have been inverted which in turn created the situation whereby the interval gets always changed to match the min_bs
@yurzo
Copy link
Author

yurzo commented Feb 9, 2018

@axboe: I pushed the fix. Per my shallow code and test, this solves the problem I had with verifying checksum more frequently that the write bs.
It does not however help me if I want to use something along the lines of:

verify=pattern
verify_pattern=%o

I think it'll also fix #163 but haven't checked.

@sitsofe
Copy link
Collaborator

sitsofe commented Feb 9, 2018

@yurzo Your patch looks good but could you retitle the commit to something like:

init: fix broken verify_interval

and then add

Fixes: https://github.com/axboe/fio/issues/522

and your Signed-off-by: to the bottom of the commit? See aedd021 as an example.

@sitsofe
Copy link
Collaborator

sitsofe commented Feb 9, 2018

@yurzo Just to follow up, your patch won't fix #163 as that issue isn't related to an incorrect verify_interval - it's to do with fio not understanding how to verify data that comes from two different passes (i.e. where the final pass wasn't full).

Re

verify_pattern=%o

being broken - could you give a more complete example? When I ran:

./fio --rw=write --size=32K --io_size=32K --direct=0 --ioengine=libaio --name=carlos --bs=8K --filename=/dev/shm/test2.bin --verify=pattern --verify_pattern=%o

That seemed to work fine for me.

@sitsofe
Copy link
Collaborator

sitsofe commented Feb 9, 2018

@yurzo OK I've just been doing some testing of the patch and won't this patch break the case where verification is requested but verify_interval hasn't been set? For example does

make test

work for you?

@yurzo
Copy link
Author

yurzo commented Feb 9, 2018

@sitsofe: Thanks for the pointers on commit messages and make test as I wasn't aware of either.

Before I get down to try and fix it I have a couple questions:

  • Is the design assumption that the buffer behind struct thread_options *o = &td->o; is zeroed unless an option is provided?
  • Is there a quick primer out there on how to debug fio? (It's has been a long while since I've last shipped a line of C)

@axboe
Copy link
Owner

axboe commented Feb 9, 2018

You can't assume that a zero value for an option means it hasn't been set. You can, however, use fio_option_is_set(&td->o, optname) to check if that's the case or not.

@yurzo
Copy link
Author

yurzo commented Feb 9, 2018

Got it!

yurzo pushed a commit to yurzo/fio that referenced this issue Feb 9, 2018
The operands seems to have been inverted which in turn created the situation whereby the interval was always changed to match the min_bs

Fixes: axboe#522
Signed-off-by: Damian Yurzola <damian@yurzola.net><damian.yurzola@purestorage.com>
@yurzo
Copy link
Author

yurzo commented Feb 9, 2018

@sitsofe:

I've adjusted the commit messages per your suggestions, addressed the syntax issues and the division by 0. It all looks good for verification with headers.
WRT verification with patterns and your question. Below my test script.

This test script fails for case 3 and 4. Because the pattern is laid out in 8K granularity (max_bs rather than verify_interval).

I do not understand why case 5 does not fail (is it verifying at all?)

VERIFY='--verify=pattern --verify_pattern=%o'
#VERIFY='--verify=md5' # --verify_pattern=%o'
echo "1"
./fio --rw=write --size=32K --io_size=32K --direct=1 --name=carlos --bs=8K --filename=test.bin --verify_interval=512 $VERIFY > /dev/null
echo $?
echo "2"
./fio --rw=read --size=32K --io_size=32K --direct=1 --name=carlos --bs=8K --filename=test.bin --verify_interval=512 $VERIFY > /dev/null
echo $?
echo "3"
./fio --rw=read --size=32K --io_size=32K --direct=1 --name=carlos --bs=512 --filename=test.bin --verify_interval=512 $VERIFY > /dev/null
echo $?
echo "4"
./fio --rw=read --size=32K --io_size=32K --direct=1 --name=carlos --bs=512 --filename=test.bin --verify_interval=8K $VERIFY > /dev/null
echo $?
echo "5"
./fio --rw=read --size=32K --io_size=32K --direct=1 --name=carlos --bs=512 --filename=test.bin $verify > /dev/null
echo $?

@sitsofe
Copy link
Collaborator

sitsofe commented Feb 10, 2018

@yurzo

Cases 3 and case 4 should fail (and they do) because when you use "%o" the pattern put down reflects the starting offset of the entire block (regardless of verify_interval) and if the block size you are verifying with is different to the one that the write portion put down the blocks will no longer match up.

Case 5 you've changed the case of the variable name from $VERIFY to $verify. Bash variables are case sensitive so $verify will output nothing thus case 5 is just a normal read job and not a verifying read job.

@yurzo
Copy link
Author

yurzo commented Feb 10, 2018

@sitsofe: Roger @ $verify.

%o reflects the starting offset of the entire block (regardless of verify_interval)

Should this not be added somewhere in the HOWTO. The HOWTO lead me to believe that verification by headers vs verification by parameter were equal citizens, when in reality they are not.

yurzo pushed a commit to yurzo/fio that referenced this issue Feb 10, 2018
The operands seems to have been inverted which in turn
created the situation whereby the interval was always
changed to match the min_bs

Fixes: axboe#522
Signed-off-by: Damian Yurzola <damian@yurzola.net>
baparham pushed a commit to baparham/fio that referenced this issue Jul 27, 2018
The operands seems to have been inverted which in turn
created the situation whereby the interval was always
changed to match the min_bs

Fixes: axboe#522
Signed-off-by: Damian Yurzola <damian@yurzola.net>
@nobelt
Copy link

nobelt commented Oct 12, 2021

This fix should have been in by fio version 3.1, correct? Because I'm seeing the same issue where only 8k block aligned have fio headers written to them, despite specifying the verify_interval to 512.

Running the same snippet of code from the original post produces this:

$ hexdump -Cv test.bin | grep -E "  ca ac"
00000000  ca ac 05 00 00 20 00 00  3c 54 09 c5 01 cc a4 00  |..... ..<T......|
00002000  ca ac 05 00 00 20 00 00  2e 56 db f6 8c 0a bb 88  |..... ...V......|
00004000  ca ac 05 00 00 20 00 00  d0 92 04 ac f3 14 98 41  |..... .........A|
00006000  ca ac 05 00 00 20 00 00  34 94 94 93 9a 33 7a 18  |..... ..4....3z.|

fio --rw=write --size=32K --io_size=32K --direct=1 --name=carlos --bs=8K --filename=test.bin --verify_interval=512 --verify=crc32c

@sitsofe
Copy link
Collaborator

sitsofe commented Oct 13, 2021

@nobelt

This fix should have been in by fio version 3.1, correct?

The commit c82acb8 shows the fix didn't arrive until fio-3.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants