Re: [PATCHv2 1/2] blktests: test direct io offsets

Shinichiro Kawasaki
2025-11-25T11:26:34Z

On Nov 19, 2025 / 11:54, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> Tests various direct IO memory and length alignments against the
> device's queue limits reported from sysfs.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
[...]
> diff --git a/src/dio-offsets.c b/src/dio-offsets.c
> new file mode 100644
> index 0000000..8e46091
> --- /dev/null
> +++ b/src/dio-offsets.c
[...]
> +static void init_buffers()
> +{
> + unsigned long lb_mask = logical_block_size - 1;
> + int fd, ret;
> +
> + buf_size = max_bytes * max_segments / 2;
> + if (buf_size < logical_block_size * max_segments)
> + err(EINVAL, "%s: logical block size is too big", __func__);
> +
> + if (buf_size < logical_block_size * 1024 * 4)
> + buf_size = logical_block_size * 1024 * 4;
> +
> + if (buf_size & lb_mask)
> + buf_size = (buf_size + lb_mask) & ~(lb_mask);

I investigated why this new test case fails with my QEMU SATA device and noticed
that the device has 128MiB size. The calculation above sets buf_size = 192MiB.
Then pwrite() in test_full_size_aligned() is called with 192MiB size and it was
truncated to 128MiB. Then the following compare() check failed.

Is it okay to cap the buf_size with the device size? If so, I suggest the change
below. With this change, the test case passes with the device. If you are okay
with the change, I can fold it in when I apply the patch.

-------------------- diff start -------------------
diff --git a/src/dio-offsets.c b/src/dio-offsets.c
index 8e46091..7e1aecd 100644
--- a/src/dio-offsets.c
+++ b/src/dio-offsets.c
@@ -22,6 +22,8 @@
#include <sys/stat.h>
#include <sys/statfs.h>
#include <sys/uio.h>
+#include <sys/ioctl.h>
+#include <sys/mount.h>

#define power_of_2(x) ((x) && !((x) & ((x) - 1)))

@@ -81,6 +83,7 @@ static void init_buffers()
{
unsigned long lb_mask = logical_block_size - 1;
int fd, ret;
+ unsigned long long dev_bytes;

buf_size = max_bytes * max_segments / 2;
if (buf_size < logical_block_size * max_segments)
@@ -92,6 +95,13 @@ static void init_buffers()
if (buf_size & lb_mask)
buf_size = (buf_size + lb_mask) & ~(lb_mask);

+ ret = ioctl(test_fd, BLKGETSIZE64, &dev_bytes);
+ if (ret < 0)
+ err(ret, "%s: ioctl BLKGETSIZE64 failed", __func__);
+
+ if (dev_bytes < buf_size)
+ buf_size = dev_bytes;
+
ret = posix_memalign((void **)&in_buf, pagesize, buf_size);
if (ret)
err(EINVAL, "%s: failed to allocate in-buf", __func__);
-------------------- diff end -------------------

> +
> + ret = posix_memalign((void **)&in_buf, pagesize, buf_size);
> + if (ret)
> + err(EINVAL, "%s: failed to allocate in-buf", __func__);
> +
> + ret = posix_memalign((void **)&out_buf, pagesize, buf_size);
> + if (ret)
> + err(EINVAL, "%s: failed to allocate out-buf", __func__);
> +
> + fd = open("/dev/urandom", O_RDONLY);
> + if (fd < 0)
> + err(EINVAL, "%s: failed to open urandom", __func__);
> +
> + ret = read(fd, out_buf, buf_size);
> + if (ret < 0)
> + err(EINVAL, "%s: failed to randomize output buffer", __func__);
> +
> + close(fd);
> +}
[...]
> +/* ./$prog-name file */
> +int main(int argc, char **argv)
> +{
> + if (argc < 2)
> + errx(EINVAL, "expect argments: file");
> +
> + init_args(argv);
> + init_buffers();
> + run_tests();
> + close(test_fd);
> + free(out_buf);
> + free(in_buf);
> +
> + return 0;

Nit: last "}" is missing here. I can add it when I apply this patch.

> diff --git a/tests/block/042 b/tests/block/042
> new file mode 100644
> index 0000000..a911d82
> --- /dev/null
> +++ b/tests/block/042
> @@ -0,0 +1,26 @@
> +#!/bin/bash

Is it okay to add any GPL SPDX license and your copyright here? It is not ideal
that I add your copyright and license, but if you specify them in this thread, I
will add the Link to this thread in the commit message and fold-in the specifed
SPDX license and your copyright.

> +
> +. tests/block/rc
> +
> +DESCRIPTION="Test unusual direct-io offsets"
> +QUICK=1
> +
> +device_requires() {
> + _require_test_dev_sysfs
> +}
> +
> +test_device() {
> + echo "Running ${TEST_NAME}"
> +
> + sys_max_segments=$(cat "${TEST_DEV_SYSFS}/queue/max_segments")
> + sys_dma_alignment=$(cat "${TEST_DEV_SYSFS}/queue/dma_alignment")
> + sys_virt_boundary_mask=$(cat "${TEST_DEV_SYSFS}/queue/virt_boundary_mask")
> + sys_logical_block_size=$(cat "${TEST_DEV_SYSFS}/queue/logical_block_size")
> + sys_max_sectors_kb=$(cat "${TEST_DEV_SYSFS}/queue/max_sectors_kb")
> +
> + if ! src/dio-offsets ${TEST_DEV} $sys_max_segments $sys_max_sectors_kb $sys_dma_alignment $sys_virt_boundary_mask $sys_logical_block_size; then
> + echo "src/dio-offsets failed"
> + fi
> +
> + echo "Test complete"
> +}