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

[docs] Using bssplit with zonal disks uses min bssplit not sector size as min #1836

Open
ckenna opened this issue Oct 31, 2024 · 2 comments
Open

Comments

@ckenna
Copy link

ckenna commented Oct 31, 2024

Please acknowledge the following before creating a ticket

Description of the bug:

This is mostly a bug to consider clarifying some documentation. The short version of this is that when using a zonal device, the writes have to be aligned to physical block size. Couple that with using a bssplit setting, and I discovered that the min block size is coming from bssplit and not from the device itself.

So two possible outcomes are: Add this to fio docs and/or change the ZBD code to get min_bs from the zonal device and not bssplit.

Extra background:

For example, when I create a read job using bssplit=6ki/50:10ki/50 (full job file is below), I see only 6ki reads from fio.

I found a place in the zbd code where fio requires all writes to be aligned to the minimum block size, but it's taking the min block size from `bssplit. I traced the location down to here with some debug prints I added:

fio/zbd.c

Lines 2013 to 2023 in a191635

/*
* Make sure the I/O does not cross over the zone wp position.
*/
new_len = min((unsigned long long)io_u->buflen,
(unsigned long long)(zb->wp - io_u->offset));
new_len = new_len / min_bs * min_bs;
if (new_len < io_u->buflen) {
io_u->buflen = new_len;
dprint(FD_IO, "Changed length from %u into %llu\n",
orig_len, io_u->buflen);
}

Here is the output of my debug printing (lines I added are prefixed with kenna:) and some commentary from me:

# On line 2018, the code unconditionally executes this:
# new_len = new_len / min_bs * min_bs
# https://github.com/axboe/fio/blob/a191635ad42893a60d0a917b27e97e3fc73f7fee/zbd.c#L2018
io       10439 kenna: after doing new_len / min_bs (6144) * min_bs => new_len = 6144
# So after doing 10240 / 6144 * 6144 in integer division, we are left with 6144.
# That's validated by the original debug printf.
io       10439 Changed length from 10240 into 6144
io       10439 kenna: because new_len (6144) < io_u->buflen (6144)

The minimum block size is 6144 but probably should be based on the actual disk block size.

Environment: Linux 5.10.210

fio version: fio-3.38-4-gcd56-dirty (I am assuming dirty is b/c I added my own dprintf statements. git is at cd56c0)

Reproduction steps

[global]
direct=1
time_based
runtime=5s
ioengine=libaio
filename=/dev/sda
zonemode=zbd
zonesize=268435456
ignore_zone_limits=1
thread=1
lat_percentiles=1

[read]
rw=randread
# Does NOT issue any 10ki reads, issues 6k reads, no 4k reads.
bssplit=6ki/50:10ki/50

write_iolog=/tmp/io-log.txt
@ckenna ckenna changed the title Using bssplit with zonal disks requires minimum blocksize multiple despite use of bs_unaligned [docs] Using bssplit with zonal disks requires minimum blocksize multiple despite use of bs_unaligned Oct 31, 2024
@ckenna ckenna changed the title [docs] Using bssplit with zonal disks requires minimum blocksize multiple despite use of bs_unaligned [docs] Using bssplit with zonal disks uses min bssplit not sector size as min Oct 31, 2024
@vincentkfu
Copy link
Collaborator

@kawasaki @damien-lemoal Do you have any thoughts about this?

@damien-lemoal
Copy link
Contributor

Note: Zonal disk -> zoned block device (or zoned disk or SMR disk), please. "zonal" is not the correct term.

Regarding "The short version of this is that when using a zonal device, the writes have to be aligned to physical block size", yes, that is a hardware requirement. Not sure if we actually check that anywhere in fio/zbd.c, but we should. Will check.

Regarding the bssplit use, as far as I understand it, all IOs should always have an offset aligned to the block size used, unless bs_unaligned is also set. And that is not special to zbd.c. Would need to check that again. The code in zbd.c does ignore bs_unaligned though, so that probably needs to be fixed after checking what the regular use case (not zonemode=zbd) does as we should do the same.

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

3 participants