fio icon indicating copy to clipboard operation
fio copied to clipboard

The exclusiveness between `randseed` and `randrepeat` is not well defined

Open mingnus opened this issue 3 years ago • 2 comments

Please acknowledge the following before creating a ticket

  • [x] I have read the GitHub issues section of REPORTING-BUGS.

Description of the bug:

According to the function setup_random_seeds(), fio supports setting random seeds in the following manners:

  • Randomly initialize all the random seeds from /dev/urandom, if neither randseed nor randrepeat is set.
  • Default-initialize all the seed values using the preset constant (FIO_RANDSEED), if allrandrepeat=1 is present.
  • Only default-initialize the seed for io offset (rand_seeds[FIO_RAND_BLOCK_OFF]), if randrepeat is set to 1. This is the default behavior of fio.
  • Manually specify all the random seeds using the randseed option.

Questions arose with the third way since it might conflict with the randseed option.

The first issue is that the randseed doesn't change io offsets unless we specify randrepeat=0 explicitly. Assume that the two options are mutually exclusive, the given randseed could be the source for all the seed values, so we don't have to specify randrepeat=0 explicitly. That's why the function fixup_options() implicitly disables the randrepeat option if randseed is present. However, such override doesn't work due to the calling sequence. This issue could be fixed if we run fixup_options() before calling setup_random_seeds().

The second issue is about how the seeds are initialized if randseed is not set. It seems to me that options randrepeat and allrandrepeat control the randomness of seed values under this situation. However, the current fio doesn't well distinguish the two options. It seeds the generators with two constants: FIO_RANDSEED, and the magic 0x89 defined in option.c, without any randomness. In addition, sometimes the two different seed constants are used interchangeably, which results in subtle differences in the generated values, and that's the third issue.

I think the initialization of random seeds needs some cleanup, at least gathering all the initializations into a single function.

Environment: Fedora 32

fio version: fio 3.30-15

Reproduction steps

// Issue1. `randseed` doesn't change the io offsets if `randrepeat=0` is not set.
$ fio --name test --filename <anyfile> --rw=randread --bs=4k --io_size 64k --randseed=1234 --debug=io

// Issue2. all the seed values are not changed between runs, if the `randseed` is not set.
$ fio --name test --filename <anyfile> --rw=randread --bs=4k --io_size 64k --debug=io

// Issue3. `allrandrepeat` doesn't serve as the superset of `randrepeat`.
// Given `allrandrepeat=1`, the io offsets vary depending on the value of `randrepeat`.
$ fio --name test --filename <anyfile> --rw=randread --bs=4k --io_size 64k --allrandrepeat=1 --randrepeat=1 --debug=io
$ fio --name test --filename <anyfile> --rw=randread --bs=4k --io_size 64k --allrandrepeat=1 --randrepeat=0 --debug=io

mingnus avatar Apr 25 '22 15:04 mingnus

I think the initialization of random seeds needs some cleanup, at least gathering all the initializations into a single function.

I don't disagree, care to send a patch?

axboe avatar Apr 26 '22 01:04 axboe

Sure, but I'm concerning the backward compatibility since the patch changes the default way of random number generation.

I might split the whole patch into three or more for easier review:

  1. Make randseed and randrepeat mutually exclusive by moving up fixup_options().
  2. Move all the initialization of rand_seeds into setup_random_seeds().
  3. Make allrandrepeat as a superset of randrepeat , that fixes rand_seeds[FIO_RAND_BLOCK_OFF] unchanged if allrandrepeat=1.
  4. (might be the most concerned) Distinguish randrepeat and allrandrepeat by randomly initializing all the seeds except rand_seeds[FIO_RAND_BLOCK_OFF] if allrandrepeat is not present.

Then we'll have four ways to initialize the random seeds, as mentioned in my original post if all the patches are applied.

The last one changes the program's default behavior. Setting allrandrepeat=1 as default could minimize potential side effects.

mingnus avatar Apr 26 '22 03:04 mingnus

Address by patches referenced by https://github.com/axboe/fio/pull/1546

vincentkfu avatar May 09 '23 20:05 vincentkfu