* [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes
@ 2024-02-06 10:57 Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 1/5] zbd: avoid assertions during sequential read I/O Dmitry Fomichev
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Dmitry Fomichev @ 2024-02-06 10:57 UTC (permalink / raw
To: Jens Axboe, Vincent Fu, fio
Cc: Damien Le Moal, Shin'ichiro Kawasaki, Niklas Cassel,
Dmitry Fomichev
This series contains a few patches to make improvements to fio support
for zoned block devices (ZBDs).
The first patch fixes an assertion that has been reported in the field.
The fourth patch fixes a failure that happens while running the unit
tests against an uncommon class of ZBDs.
The rest are clean ups and some minor feature additions.
v1 -> v2:
- correct the commit referred in the Fixes tag in the first
patch (Niklas)
- edit commit messages to make them more clear
Dmitry Fomichev (5):
zbd: avoid assertions during sequential read I/O
oslib: log BLKREPORTZONE error code
zbd: use a helper to calculate zone index
t/zbd: check device for unrestricted read support
t/zbd: add -s option to test-zbd-support script
oslib/linux-blkzoned.c | 2 ++
t/zbd/functions | 22 ++++++++++++++++++++++
t/zbd/test-zbd-support | 20 ++++++++++++++++++--
zbd.c | 21 +++++++++++++++------
4 files changed, 57 insertions(+), 8 deletions(-)
--
2.37.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/5] zbd: avoid assertions during sequential read I/O
2024-02-06 10:57 [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Dmitry Fomichev
@ 2024-02-06 10:57 ` Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 2/5] oslib: log BLKREPORTZONE error code Dmitry Fomichev
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Dmitry Fomichev @ 2024-02-06 10:57 UTC (permalink / raw
To: Jens Axboe, Vincent Fu, fio
Cc: Damien Le Moal, Shin'ichiro Kawasaki, Niklas Cassel,
Dmitry Fomichev
The following assert has been observed to be triggered
if the I/O offset + I/O size exceeds the device capacity in a
sequential read workload -
“zbd.c:110: zone_lock: Assertion `f->min_zone <= nz && nz < f->max_zone' failed.”
The code in zbd_zone_align_file_sizes() rounds down the I/O size to
avoid these situations, but it is bypassed if
td->o.td_ddir == TD_DDIR_READ.
Avoid this issue by modifying zbd_zone_align_file_sizes() to round down
the I/O size for read I/Os and leave the I/O offset untouched.
Fixes: bfbdd35b3e8f ("Add support for zoned block devices")
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
zbd.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/zbd.c b/zbd.c
index 61b5b688..7577472e 100644
--- a/zbd.c
+++ b/zbd.c
@@ -674,9 +674,20 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
return false;
}
+ if (td->o.td_ddir == TD_DDIR_READ) {
+ z = zbd_offset_to_zone(f, f->file_offset + f->io_size);
+ new_end = z->start;
+ if (f->file_offset + f->io_size > new_end) {
+ log_info("%s: rounded io_size from %"PRIu64" to %"PRIu64"\n",
+ f->file_name, f->io_size,
+ new_end - f->file_offset);
+ f->io_size = new_end - f->file_offset;
+ }
+ return true;
+ }
+
z = zbd_offset_to_zone(f, f->file_offset);
- if ((f->file_offset != z->start) &&
- (td->o.td_ddir != TD_DDIR_READ)) {
+ if (f->file_offset != z->start) {
new_offset = zbd_zone_end(z);
if (new_offset >= f->file_offset + f->io_size) {
log_info("%s: io_size must be at least one zone\n",
@@ -692,8 +703,7 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
z = zbd_offset_to_zone(f, f->file_offset + f->io_size);
new_end = z->start;
- if ((td->o.td_ddir != TD_DDIR_READ) &&
- (f->file_offset + f->io_size != new_end)) {
+ if (f->file_offset + f->io_size != new_end) {
if (new_end <= f->file_offset) {
log_info("%s: io_size must be at least one zone\n",
f->file_name);
--
2.37.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/5] oslib: log BLKREPORTZONE error code
2024-02-06 10:57 [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 1/5] zbd: avoid assertions during sequential read I/O Dmitry Fomichev
@ 2024-02-06 10:57 ` Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 3/5] zbd: use a helper to calculate zone index Dmitry Fomichev
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Dmitry Fomichev @ 2024-02-06 10:57 UTC (permalink / raw
To: Jens Axboe, Vincent Fu, fio
Cc: Damien Le Moal, Shin'ichiro Kawasaki, Niklas Cassel,
Dmitry Fomichev
BLKREPORTZONE may fail because of a variety of reasons.
Log both the return code and errno when this ioctl fails.
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
oslib/linux-blkzoned.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/oslib/linux-blkzoned.c b/oslib/linux-blkzoned.c
index 2c3ecf33..1cc8d288 100644
--- a/oslib/linux-blkzoned.c
+++ b/oslib/linux-blkzoned.c
@@ -242,6 +242,8 @@ int blkzoned_report_zones(struct thread_data *td, struct fio_file *f,
hdr->sector = offset >> 9;
ret = ioctl(fd, BLKREPORTZONE, hdr);
if (ret) {
+ log_err("%s: BLKREPORTZONE ioctl failed, ret=%d, err=%d.\n",
+ f->file_name, ret, -errno);
ret = -errno;
goto out;
}
--
2.37.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/5] zbd: use a helper to calculate zone index
2024-02-06 10:57 [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 1/5] zbd: avoid assertions during sequential read I/O Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 2/5] oslib: log BLKREPORTZONE error code Dmitry Fomichev
@ 2024-02-06 10:57 ` Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 4/5] t/zbd: check device for unrestricted read support Dmitry Fomichev
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Dmitry Fomichev @ 2024-02-06 10:57 UTC (permalink / raw
To: Jens Axboe, Vincent Fu, fio
Cc: Damien Le Moal, Shin'ichiro Kawasaki, Niklas Cassel,
Dmitry Fomichev
zone_lock() function contains the debug code to verify that the zone
being locked belongs to the job's working zone range. Clean up this
code by using a previously defined helper for calculating the zone
index to check.
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
zbd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/zbd.c b/zbd.c
index 7577472e..37417660 100644
--- a/zbd.c
+++ b/zbd.c
@@ -104,8 +104,7 @@ static void zone_lock(struct thread_data *td, const struct fio_file *f,
struct fio_zone_info *z)
{
#ifndef NDEBUG
- struct zoned_block_device_info *zbd = f->zbd_info;
- uint32_t const nz = z - zbd->zone_info;
+ unsigned int const nz = zbd_zone_idx(f, z);
/* A thread should never lock zones outside its working area. */
assert(f->min_zone <= nz && nz < f->max_zone);
assert(z->has_wp);
--
2.37.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/5] t/zbd: check device for unrestricted read support
2024-02-06 10:57 [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Dmitry Fomichev
` (2 preceding siblings ...)
2024-02-06 10:57 ` [PATCH v2 3/5] zbd: use a helper to calculate zone index Dmitry Fomichev
@ 2024-02-06 10:57 ` Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 5/5] t/zbd: add -s option to test-zbd-support script Dmitry Fomichev
2024-02-07 3:05 ` [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Shinichiro Kawasaki
5 siblings, 0 replies; 8+ messages in thread
From: Dmitry Fomichev @ 2024-02-06 10:57 UTC (permalink / raw
To: Jens Axboe, Vincent Fu, fio
Cc: Damien Le Moal, Shin'ichiro Kawasaki, Niklas Cassel,
Dmitry Fomichev
ZBD unit tests in t/zbd/test-zbd-support currently assume that the
drive that is being tested supports unrestricted reads, i.e. reads
that (partially or entirely) occur above the write pointer. This is
always the case with ZBD core code because Linux kernel rejects zoned
devices with restricted reads. However, libzbc ioengine does support
such devices.
The restricted/unrestricted reads feature is controlled by URSWRZ
device bit ("Unrestricted Reads of Sequential Write Required Zones")
which, depending on the device design, can be hard-coded to be reported
as 1 or 0 or it can be made configurable via MODE SET or SET FEATURES
commands. The unit tests need to behave correctly with any URSWRZ bit
value reported by the device if libzbc ioengine is used for testing.
Test #4 in the test script currently expects the device to have
unrestricted SWR zone reads. This test is guaranteed to fail if
the script is run against a drive that reports URSWRZ=0 with libzbc
ioengine.
Check if the drive has unrestricted read support disabled and process
the outcome of test #4 accordingly.
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
t/zbd/functions | 22 ++++++++++++++++++++++
t/zbd/test-zbd-support | 15 +++++++++++++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/t/zbd/functions b/t/zbd/functions
index 028df404..7734371e 100644
--- a/t/zbd/functions
+++ b/t/zbd/functions
@@ -290,6 +290,28 @@ min_seq_write_size() {
fi
}
+urswrz() {
+ local dev=$1
+
+ if [ -n "${sg_inq}" ] && [ ! -n "${use_libzbc}" ]; then
+ if ! ${sg_inq} -e --page=0xB6 --len=10 --hex "$dev" \
+ > /dev/null 2>&1; then
+ # Couldn't get URSWRZ bit. Assume the reads are unrestricted
+ # because this configuration is more common.
+ echo 1
+ else
+ ${sg_inq} -e --page=0xB6 --len=10 --hex "$dev" | tail -1 |
+ {
+ read -r offset b0 b1 b2 b3 b4 trailer && \
+ echo $(( $b4 & 0x01 )) || echo 0
+ }
+ fi
+ else
+ ${zbc_info} "$dev" |
+ sed -n 's/^[[:blank:]].*Read commands are \(un\)restricted*/\1/p' | grep -q ^ && echo 1 || echo 0
+ fi
+}
+
is_zbc() {
local dev=$1
diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 532860eb..defb3652 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -412,8 +412,16 @@ test4() {
opts+=("--size=$size" "--thread=1" "--read_beyond_wp=1")
opts+=("$(ioengine "psync")" "--rw=read" "--direct=1" "--disable_lat=1")
opts+=("--zonemode=zbd" "--zonesize=${zone_size}")
- run_fio "${opts[@]}" >> "${logfile}.${test_number}" 2>&1 || return $?
- check_read $size || return $?
+ run_fio "${opts[@]}" >> "${logfile}.${test_number}" 2>&1
+ fio_rc=$?
+ if [[ $unrestricted_reads != 0 ]]; then
+ if [[ $fio_rc != 0 ]]; then
+ return "$fio_rc"
+ fi
+ check_read $size || return $?
+ else
+ [ $fio_rc == 0 ] && return 1 || return 0
+ fi
}
# Sequential write to sequential zones.
@@ -1664,6 +1672,7 @@ if [[ -b "$realdev" ]]; then
first_sequential_zone_sector=${result[0]}
sectors_per_zone=${result[1]}
zone_size=$((sectors_per_zone * 512))
+ unrestricted_reads=$(urswrz "$dev")
if ! max_open_zones=$(max_open_zones "$dev"); then
echo "Failed to determine maximum number of open zones"
exit 1
@@ -1681,6 +1690,7 @@ if [[ -b "$realdev" ]]; then
sectors_per_zone=$((zone_size / 512))
max_open_zones=128
max_active_zones=0
+ unrestricted_reads=1
set_io_scheduler "$basename" none || exit $?
;;
esac
@@ -1712,6 +1722,7 @@ elif [[ -c "$realdev" ]]; then
first_sequential_zone_sector=${result[0]}
sectors_per_zone=${result[1]}
zone_size=$((sectors_per_zone * 512))
+ unrestricted_reads=$(urswrz "$dev")
if ! max_open_zones=$(max_open_zones "$dev"); then
echo "Failed to determine maximum number of open zones"
exit 1
--
2.37.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 5/5] t/zbd: add -s option to test-zbd-support script
2024-02-06 10:57 [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Dmitry Fomichev
` (3 preceding siblings ...)
2024-02-06 10:57 ` [PATCH v2 4/5] t/zbd: check device for unrestricted read support Dmitry Fomichev
@ 2024-02-06 10:57 ` Dmitry Fomichev
2024-02-07 3:05 ` [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Shinichiro Kawasaki
5 siblings, 0 replies; 8+ messages in thread
From: Dmitry Fomichev @ 2024-02-06 10:57 UTC (permalink / raw
To: Jens Axboe, Vincent Fu, fio
Cc: Damien Le Moal, Shin'ichiro Kawasaki, Niklas Cassel,
Dmitry Fomichev
The total number of ZBD tests in test-zbd-support script has grown
considerably over the years and zoned drive capacity has significantly
increased as well. Today, the test run duration may reach one hour for
large drives. If a terminal session failure happens during a run, it
is more efficient to restart the tests from the point where the last
run stopped rather than from the beginning.
Add -s option to the script command line to specify the starting
test number.
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
t/zbd/test-zbd-support | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index defb3652..c27d2ad6 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -15,6 +15,7 @@ usage() {
echo -e "\t-w Reset all zones before executing each write test case"
echo -e "\t-o <max_open_zones> Run fio with max_open_zones limit"
echo -e "\t-t <test #> Run only a single test case with specified number"
+ echo -e "\t-s <test #> Start testing from the case with the specified number"
echo -e "\t-q Quit the test run after any failed test"
echo -e "\t-z Run fio with debug=zbd option"
echo -e "\t-u Use io_uring ioengine in place of libaio"
@@ -1602,6 +1603,7 @@ zbd_debug=
max_open_zones_opt=
quit_on_err=
force_io_uring=
+start_test=1
while [ "${1#-}" != "$1" ]; do
case "$1" in
@@ -1615,6 +1617,7 @@ while [ "${1#-}" != "$1" ]; do
-w) reset_before_write=1; shift;;
-t) tests+=("$2"); shift; shift;;
-o) max_open_zones_opt="${2}"; shift; shift;;
+ -s) start_test=$2; shift; shift;;
-v) dynamic_analyzer=(valgrind "--read-var-info=yes");
shift;;
-q) quit_on_err=1; shift;;
@@ -1694,6 +1697,7 @@ if [[ -b "$realdev" ]]; then
set_io_scheduler "$basename" none || exit $?
;;
esac
+
elif [[ -c "$realdev" ]]; then
# For an SG node, we must have libzbc option specified
if [[ ! -n "$use_libzbc" ]]; then
@@ -1772,6 +1776,7 @@ trap 'intr=1' SIGINT
ret=0
for test_number in "${tests[@]}"; do
+ [ "${test_number}" -lt "${start_test}" ] && continue
rm -f "${logfile}.${test_number}"
unset SKIP_REASON
echo -n "Running test $(printf "%02d" $test_number) ... "
--
2.37.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes
2024-02-06 10:57 [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Dmitry Fomichev
` (4 preceding siblings ...)
2024-02-06 10:57 ` [PATCH v2 5/5] t/zbd: add -s option to test-zbd-support script Dmitry Fomichev
@ 2024-02-07 3:05 ` Shinichiro Kawasaki
2024-02-07 13:44 ` Vincent Fu
5 siblings, 1 reply; 8+ messages in thread
From: Shinichiro Kawasaki @ 2024-02-07 3:05 UTC (permalink / raw
To: Dmitry Fomichev
Cc: Jens Axboe, Vincent Fu, fio@vger.kernel.org, Damien Le Moal,
Niklas Cassel
On Feb 06, 2024 / 19:57, Dmitry Fomichev wrote:
> This series contains a few patches to make improvements to fio support
> for zoned block devices (ZBDs).
>
> The first patch fixes an assertion that has been reported in the field.
> The fourth patch fixes a failure that happens while running the unit
> tests against an uncommon class of ZBDs.
>
> The rest are clean ups and some minor feature additions.
The patches look good to me. I also ran them with my test set and observed no
regression. For the series,
Reviewed-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes
2024-02-07 3:05 ` [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Shinichiro Kawasaki
@ 2024-02-07 13:44 ` Vincent Fu
0 siblings, 0 replies; 8+ messages in thread
From: Vincent Fu @ 2024-02-07 13:44 UTC (permalink / raw
To: Shinichiro Kawasaki, Dmitry Fomichev
Cc: Jens Axboe, Vincent Fu, fio@vger.kernel.org, Damien Le Moal,
Niklas Cassel
On 2/6/24 22:05, Shinichiro Kawasaki wrote:
> On Feb 06, 2024 / 19:57, Dmitry Fomichev wrote:
>> This series contains a few patches to make improvements to fio support
>> for zoned block devices (ZBDs).
>>
>> The first patch fixes an assertion that has been reported in the field.
>> The fourth patch fixes a failure that happens while running the unit
>> tests against an uncommon class of ZBDs.
>>
>> The rest are clean ups and some minor feature additions.
>
> The patches look good to me. I also ran them with my test set and observed no
> regression. For the series,
>
> Reviewed-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>
Applied. Thanks.
Vincent
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-02-07 13:44 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-06 10:57 [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 1/5] zbd: avoid assertions during sequential read I/O Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 2/5] oslib: log BLKREPORTZONE error code Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 3/5] zbd: use a helper to calculate zone index Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 4/5] t/zbd: check device for unrestricted read support Dmitry Fomichev
2024-02-06 10:57 ` [PATCH v2 5/5] t/zbd: add -s option to test-zbd-support script Dmitry Fomichev
2024-02-07 3:05 ` [PATCH v2 0/5] zbd, t/zbd: miscellaneous fixes Shinichiro Kawasaki
2024-02-07 13:44 ` Vincent Fu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).