Linux block layer
 help / color / mirror / Atom feed
* [PATCH blktests] block/049: test polled dio with user PI metadata
@ 2026-09-24  2:55 Yang Xiuwei
  2026-09-25  6:56 ` Christoph Hellwig
  2026-09-25 13:16 ` Shin'ichiro Kawasaki
  0 siblings, 2 replies; 4+ messages in thread
From: Yang Xiuwei @ 2026-09-24  2:55 UTC (permalink / raw)
  To: Shin'ichiro Kawasaki; +Cc: Christoph Hellwig, linux-block, Yang Xiuwei

IOPOLL combined with a PI attribute used to leave iocb->private as a
uio_meta and oops in bio_poll(). The kernel rejects that combination
with -EOPNOTSUPP. Add a scsi_debug reproducer that expects this result.

Assisted-by: LLM
Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
---
Depends on the kernel fix that rejects IOPOLL plus user PI metadata
with -EOPNOTSUPP. Without that fix this test oopses in bio_poll().

Link: https://lore.kernel.org/linux-block/20260923093732.1504291-1-yangxiuwei@kylinos.cn/

The fix is not in mainline yet. Is an oops on kernels without it
acceptable here, or how should the test avoid that? _have_kver after
the fix lands would still skip a stable backport, which keeps the old
version number. The only userspace difference is -EOPNOTSUPP versus
the oops.

 src/Makefile          |   1 +
 src/metadata-iopoll.c | 101 ++++++++++++++++++++++++++++++++++++++++++
 tests/block/049       |  46 +++++++++++++++++++
 tests/block/049.out   |   2 +
 4 files changed, 150 insertions(+)
 create mode 100644 src/metadata-iopoll.c
 create mode 100755 tests/block/049
 create mode 100644 tests/block/049.out

diff --git a/src/Makefile b/src/Makefile
index dd64694..1e571c6 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -31,6 +31,7 @@ C_TARGETS := \
 	zbdioctl
 
 C_URING_TARGETS := metadata \
+	metadata-iopoll \
 	nvme-passthru-admin-uring
 C_UBLK_TARGETS := miniublk
 
diff --git a/src/metadata-iopoll.c b/src/metadata-iopoll.c
new file mode 100644
index 0000000..95e2bc9
--- /dev/null
+++ b/src/metadata-iopoll.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-3.0+
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <liburing.h>
+
+#ifndef IORING_RW_ATTR_FLAG_PI
+#define PI_URING_COMPAT
+#define IORING_RW_ATTR_FLAG_PI (1U << 0)
+struct io_uring_attr_pi {
+	__u16 flags;
+	__u16 app_tag;
+	__u32 len;
+	__u64 addr;
+	__u64 seed;
+	__u64 rsvd;
+};
+#endif
+
+#ifndef IO_INTEGRITY_CHK_GUARD
+#define IO_INTEGRITY_CHK_GUARD  (1U << 0)
+#define IO_INTEGRITY_CHK_REFTAG (1U << 1)
+#endif
+
+#define DATA_LEN 4096
+#define META_LEN 64
+
+static void on_alarm(int sig)
+{
+	(void)sig;
+	dprintf(STDERR_FILENO, "timed out\n");
+	_exit(1);
+}
+
+int main(int argc, char **argv)
+{
+	struct io_uring ring;
+	struct io_uring_sqe *sqe;
+	struct io_uring_cqe *cqe;
+	struct io_uring_attr_pi pi = {
+		.flags = IO_INTEGRITY_CHK_GUARD | IO_INTEGRITY_CHK_REFTAG,
+		.len = META_LEN,
+		.seed = 1,
+	};
+	void *data, *meta;
+	int fd, ret;
+
+	if (argc != 2)
+		return 1;
+
+	signal(SIGALRM, on_alarm);
+	alarm(10);
+
+	fd = open(argv[1], O_RDONLY | O_DIRECT);
+	if (fd < 0) {
+		perror("open");
+		return 1;
+	}
+	if (posix_memalign(&data, 4096, DATA_LEN) ||
+	    posix_memalign(&meta, 4096, META_LEN)) {
+		perror("posix_memalign");
+		return 1;
+	}
+	pi.addr = (__u64)(uintptr_t)meta;
+
+	ret = io_uring_queue_init(8, &ring, IORING_SETUP_IOPOLL);
+	if (ret < 0) {
+		fprintf(stderr, "queue_init: %s\n", strerror(-ret));
+		return 1;
+	}
+	sqe = io_uring_get_sqe(&ring);
+	if (!sqe)
+		return 1;
+	io_uring_prep_read(sqe, fd, data, DATA_LEN, 0);
+#ifdef PI_URING_COMPAT
+	sqe->__pad2[0] = IORING_RW_ATTR_FLAG_PI;
+	sqe->addr3 = (__u64)&pi;
+#else
+	sqe->attr_type_mask = IORING_RW_ATTR_FLAG_PI;
+	sqe->attr_ptr = (__u64)&pi;
+#endif
+	ret = io_uring_submit(&ring);
+	if (ret < 1) {
+		fprintf(stderr, "submit: %d\n", ret);
+		return 1;
+	}
+	ret = io_uring_wait_cqe(&ring, &cqe);
+	if (ret < 0) {
+		fprintf(stderr, "wait: %s\n", strerror(-ret));
+		return 1;
+	}
+	if (cqe->res != -EOPNOTSUPP) {
+		fprintf(stderr, "cqe %d\n", cqe->res);
+		return 1;
+	}
+	return 0;
+}
diff --git a/tests/block/049 b/tests/block/049
new file mode 100755
index 0000000..b932414
--- /dev/null
+++ b/tests/block/049
@@ -0,0 +1,46 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2026 Yang Xiuwei <yangxiuwei@kylinos.cn>
+#
+# IOPOLL combined with a PI attribute used to oops in bio_poll(). The kernel
+# rejects that combination with -EOPNOTSUPP.
+
+. tests/block/rc
+. common/scsi_debug
+
+DESCRIPTION="iopoll dio with user PI metadata returns -EOPNOTSUPP"
+QUICK=1
+
+requires() {
+	_have_kernel_option IO_URING
+	_have_kernel_option BLK_DEV_INTEGRITY
+	_have_module scsi_debug
+	_have_src_program metadata-iopoll
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	if ! _init_scsi_debug dif=1 dix=1 dev_size_mb=64 sector_size=512 \
+			submit_queues=2 poll_queues=1; then
+		return 1
+	fi
+
+	local dev="${SCSI_DEBUG_DEVICES[0]}"
+
+	if [[ "$(<"/sys/block/${dev}/queue/io_poll")" != 1 ]]; then
+		SKIP_REASONS+=("scsi_debug has no poll queue")
+		_exit_scsi_debug
+		return
+	fi
+
+	src/metadata-iopoll "/dev/${dev}" >>"$FULL" 2>&1
+	local rc=$?
+
+	_exit_scsi_debug
+	if ((rc)); then
+		echo "metadata-iopoll failed"
+		return 1
+	fi
+	echo "Test complete"
+}
diff --git a/tests/block/049.out b/tests/block/049.out
new file mode 100644
index 0000000..c88edfe
--- /dev/null
+++ b/tests/block/049.out
@@ -0,0 +1,2 @@
+Running block/049
+Test complete
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests] block/049: test polled dio with user PI metadata
  2026-09-24  2:55 [PATCH blktests] block/049: test polled dio with user PI metadata Yang Xiuwei
@ 2026-09-25  6:56 ` Christoph Hellwig
  2026-09-25 13:16 ` Shin'ichiro Kawasaki
  1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2026-09-25  6:56 UTC (permalink / raw)
  To: Yang Xiuwei; +Cc: Shin'ichiro Kawasaki, Christoph Hellwig, linux-block

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests] block/049: test polled dio with user PI metadata
  2026-09-24  2:55 [PATCH blktests] block/049: test polled dio with user PI metadata Yang Xiuwei
  2026-09-25  6:56 ` Christoph Hellwig
@ 2026-09-25 13:16 ` Shin'ichiro Kawasaki
  2026-09-28  0:53   ` Yang Xiuwei
  1 sibling, 1 reply; 4+ messages in thread
From: Shin'ichiro Kawasaki @ 2026-09-25 13:16 UTC (permalink / raw)
  To: Yang Xiuwei; +Cc: Christoph Hellwig, linux-block

On Sep 24, 2026 / 10:55, Yang Xiuwei wrote:
> IOPOLL combined with a PI attribute used to leave iocb->private as a
> uio_meta and oops in bio_poll(). The kernel rejects that combination
> with -EOPNOTSUPP. Add a scsi_debug reproducer that expects this result.
> 
> Assisted-by: LLM
> Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
> ---
> Depends on the kernel fix that rejects IOPOLL plus user PI metadata
> with -EOPNOTSUPP. Without that fix this test oopses in bio_poll().
> 
> Link: https://lore.kernel.org/linux-block/20260923093732.1504291-1-yangxiuwei@kylinos.cn/

Thanks for the patch. I confirmed the new test case recreates the Oops, and the
kernel fix patch avoids it. Good from test run point of view.

> 
> The fix is not in mainline yet. Is an oops on kernels without it
> acceptable here, or how should the test avoid that? _have_kver after
> the fix lands would still skip a stable backport, which keeps the old
> version number. The only userspace difference is -EOPNOTSUPP versus
> the oops.

I understand the concern. My current policy is to wait for the kernel side
fix gets upstreamed to Linus master branch, then add the test case to blktests.
This way we can keep blktests runs healthy with Linus master branch. It can
cause the failure with stable kernels, but I expect it will work as the signal
to encourage backport of the fix. So, let's wait for the kernel side fix to
get settled on Linus master branch.

Also, please find my inline comments below.

...

> diff --git a/src/metadata-iopoll.c b/src/metadata-iopoll.c
> new file mode 100644
> index 0000000..95e2bc9
> --- /dev/null
> +++ b/src/metadata-iopoll.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-3.0+

Your copyright is missing here.

> +#include <errno.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <stdio.h>

...

> diff --git a/tests/block/049 b/tests/block/049
> new file mode 100755
> index 0000000..b932414
> --- /dev/null
> +++ b/tests/block/049
> @@ -0,0 +1,46 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2026 Yang Xiuwei <yangxiuwei@kylinos.cn>
> +#
> +# IOPOLL combined with a PI attribute used to oops in bio_poll(). The kernel
> +# rejects that combination with -EOPNOTSUPP.
> +
> +. tests/block/rc
> +. common/scsi_debug
> +
> +DESCRIPTION="iopoll dio with user PI metadata returns -EOPNOTSUPP"
> +QUICK=1
> +
> +requires() {
> +	_have_kernel_option IO_URING
> +	_have_kernel_option BLK_DEV_INTEGRITY
> +	_have_module scsi_debug

I recommend _have_loadable_scsi_debug instead of "_have_module scsi_debug".
It will do some more check for the scsi_dubug module status.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests] block/049: test polled dio with user PI metadata
  2026-09-25 13:16 ` Shin'ichiro Kawasaki
@ 2026-09-28  0:53   ` Yang Xiuwei
  0 siblings, 0 replies; 4+ messages in thread
From: Yang Xiuwei @ 2026-09-28  0:53 UTC (permalink / raw)
  To: shinichiro.kawasaki; +Cc: hch, linux-block

Hi Shin'ichiro,

On Thu, Sep 25, 2026 at 01:16:00PM +0000, Shin'ichiro Kawasaki wrote:
> I understand the concern. My current policy is to wait for the kernel side
> fix gets upstreamed to Linus master branch, then add the test case to blktests.
> This way we can keep blktests runs healthy with Linus master branch. It can
> cause the failure with stable kernels, but I expect it will work as the signal
> to encourage backport of the fix. So, let's wait for the kernel side fix to
> get settled on Linus master branch.

That makes sense. I will wait until the kernel fix lands in Linus' tree,
then resend the blktests patch.

> Your copyright is missing here.

Will add it in the next version.

> I recommend _have_loadable_scsi_debug instead of "_have_module scsi_debug".

Will switch to _have_loadable_scsi_debug.

Thanks for reviewing and for confirming the test behavior.

Thanks,
Yang Xiuwei


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-28  0:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24  2:55 [PATCH blktests] block/049: test polled dio with user PI metadata Yang Xiuwei
2026-09-25  6:56 ` Christoph Hellwig
2026-09-25 13:16 ` Shin'ichiro Kawasaki
2026-09-28  0:53   ` Yang Xiuwei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox