[PATCH v4 0/6] io_uring/mock: add basic infra for test mock files

[PATCH v4 0/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

io_uring commands provide an ioctl style interface for files to
implement file specific operations. io_uring provides many features and
advanced api to commands, and it's getting hard to test as it requires
specific files/devices.

Add basic infrastucture for creating special mock files that will be
implementing the cmd api and using various io_uring features we want to
test. It'll also be useful to test some more obscure read/write/polling
edge cases in the future, which was initially suggested by Chase.

v4: require CAP_ADMIN and limit to KASAN builds
v3: fix memleak, + release fop callback
v2: add rw support with basic options
    implement features not as bitmask but sequence number

Pavel Begunkov (6):
  io_uring/mock: add basic infra for test mock files
  io_uring/mock: add cmd using vectored regbufs
  io_uring/mock: add sync read/write
  io_uring/mock: allow to choose FMODE_NOWAIT
  io_uring/mock: support for async read/write
  io_uring/mock: add trivial poll handler

 init/Kconfig         |  11 ++
 io_uring/Makefile    |   1 +
 io_uring/mock_file.c | 357 +++++++++++++++++++++++++++++++++++++++++++
 io_uring/mock_file.h |  47 ++++++
 4 files changed, 416 insertions(+)
 create mode 100644 io_uring/mock_file.c
 create mode 100644 io_uring/mock_file.h

-- 
2.49.0

[PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

io_uring commands provide an ioctl style interface for files to
implement file specific operations. io_uring provides many features and
advanced api to commands, and it's getting hard to test as it requires
specific files/devices.

Add basic infrastucture for creating special mock files that will be
implementing the cmd api and using various io_uring features we want to
test. It'll also be useful to test some more obscure read/write/polling
edge cases in the future.

Suggested-by: chase xd <sl1589472800@gmail.com>
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 init/Kconfig         |  11 ++++
 io_uring/Makefile    |   1 +
 io_uring/mock_file.c | 142 +++++++++++++++++++++++++++++++++++++++++++
 io_uring/mock_file.h |  22 +++++++
 4 files changed, 176 insertions(+)
 create mode 100644 io_uring/mock_file.c
 create mode 100644 io_uring/mock_file.h

diff --git a/init/Kconfig b/init/Kconfig
index 63f5974b9fa6..9e8a5b810804 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
 	  the io_uring subsystem, hence this should only be enabled for
 	  specific test purposes.
 
+config IO_URING_MOCK_FILE
+	tristate "Enable io_uring mock files (Experimental)" if EXPERT
+	default n
+	depends on IO_URING && KASAN
+	help
+	  Enable mock files for io_uring subststem testing. The ABI might
+	  still change, so it's still experimental and should only be enabled
+	  for specific test purposes.
+
+	  If unsure, say N.
+
 config ADVISE_SYSCALLS
 	bool "Enable madvise/fadvise syscalls" if EXPERT
 	default y
diff --git a/io_uring/Makefile b/io_uring/Makefile
index d97c6b51d584..b3f1bd492804 100644
--- a/io_uring/Makefile
+++ b/io_uring/Makefile
@@ -21,3 +21,4 @@ obj-$(CONFIG_EPOLL)		+= epoll.o
 obj-$(CONFIG_NET_RX_BUSY_POLL)	+= napi.o
 obj-$(CONFIG_NET) += net.o cmd_net.o
 obj-$(CONFIG_PROC_FS) += fdinfo.o
+obj-$(CONFIG_IO_URING_MOCK_FILE) += mock_file.o
diff --git a/io_uring/mock_file.c b/io_uring/mock_file.c
new file mode 100644
index 000000000000..3bb5614c85f1
--- /dev/null
+++ b/io_uring/mock_file.c
@@ -0,0 +1,142 @@
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/anon_inodes.h>
+
+#include <linux/io_uring/cmd.h>
+#include <linux/io_uring_types.h>
+#include "mock_file.h"
+
+static int io_mock_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
+{
+	return -ENOTSUPP;
+}
+
+static const struct file_operations io_mock_fops = {
+	.owner		= THIS_MODULE,
+	.uring_cmd	= io_mock_cmd,
+};
+
+static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flags)
+{
+	const struct io_uring_sqe *sqe = cmd->sqe;
+	struct io_uring_mock_create mc, __user *uarg;
+	struct file *file = NULL;
+	size_t uarg_size;
+	int fd, ret;
+
+	uarg = u64_to_user_ptr(READ_ONCE(sqe->addr));
+	uarg_size = READ_ONCE(sqe->len);
+
+	if (sqe->ioprio || sqe->__pad1 || sqe->addr3 || sqe->file_index)
+		return -EINVAL;
+	if (uarg_size != sizeof(mc))
+		return -EINVAL;
+
+	memset(&mc, 0, sizeof(mc));
+	if (copy_from_user(&mc, uarg, uarg_size))
+		return -EFAULT;
+	if (!mem_is_zero(mc.__resv, sizeof(mc.__resv)) || mc.flags)
+		return -EINVAL;
+
+	fd = get_unused_fd_flags(O_RDWR | O_CLOEXEC);
+	if (fd < 0)
+		return fd;
+
+	file = anon_inode_create_getfile("[io_uring_mock]", &io_mock_fops,
+					 NULL, O_RDWR | O_CLOEXEC, NULL);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		goto fail;
+	}
+
+	mc.out_fd = fd;
+	if (copy_to_user(uarg, &mc, uarg_size)) {
+		fput(file);
+		ret = -EFAULT;
+		goto fail;
+	}
+
+	fd_install(fd, file);
+	return 0;
+fail:
+	put_unused_fd(fd);
+	return ret;
+}
+
+static int io_probe_mock(struct io_uring_cmd *cmd)
+{
+	const struct io_uring_sqe *sqe = cmd->sqe;
+	struct io_uring_mock_probe mp, __user *uarg;
+	size_t uarg_size;
+
+	uarg = u64_to_user_ptr(READ_ONCE(sqe->addr));
+	uarg_size = READ_ONCE(sqe->len);
+
+	if (sqe->ioprio || sqe->__pad1 || sqe->addr3 || sqe->file_index ||
+	    uarg_size != sizeof(mp))
+		return -EINVAL;
+
+	memset(&mp, 0, sizeof(mp));
+	if (copy_from_user(&mp, uarg, uarg_size))
+		return -EFAULT;
+	if (!mem_is_zero(&mp, sizeof(mp)))
+		return -EINVAL;
+
+	mp.features = 0;
+
+	if (copy_to_user(uarg, &mp, uarg_size))
+		return -EFAULT;
+	return 0;
+}
+
+static int iou_mock_mgr_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
+{
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	switch (cmd->cmd_op) {
+	case IORING_MOCK_MGR_CMD_PROBE:
+		return io_probe_mock(cmd);
+	case IORING_MOCK_MGR_CMD_CREATE:
+		return io_create_mock_file(cmd, issue_flags);
+	}
+	return -EOPNOTSUPP;
+}
+
+static const struct file_operations iou_mock_dev_fops = {
+	.owner		= THIS_MODULE,
+	.uring_cmd	= iou_mock_mgr_cmd,
+};
+
+static struct miscdevice iou_mock_miscdev = {
+	.minor			= MISC_DYNAMIC_MINOR,
+	.name			= "io_uring_mock",
+	.fops			= &iou_mock_dev_fops,
+};
+
+static int __init io_mock_init(void)
+{
+	int ret;
+
+	ret = misc_register(&iou_mock_miscdev);
+	if (ret < 0) {
+		pr_err("Could not initialize io_uring mock device\n");
+		return ret;
+	}
+	return 0;
+}
+
+static void __exit io_mock_exit(void)
+{
+	misc_deregister(&iou_mock_miscdev);
+}
+
+module_init(io_mock_init)
+module_exit(io_mock_exit)
+
+MODULE_AUTHOR("Pavel Begunkov <asml.silence@gmail.com>");
+MODULE_DESCRIPTION("io_uring mock file");
+MODULE_LICENSE("GPL");
diff --git a/io_uring/mock_file.h b/io_uring/mock_file.h
new file mode 100644
index 000000000000..8b00045480cd
--- /dev/null
+++ b/io_uring/mock_file.h
@@ -0,0 +1,22 @@
+#ifndef IOU_MOCK_H
+#define IOU_MOCK_H
+
+#include <linux/types.h>
+
+struct io_uring_mock_probe {
+	__u64		features;
+	__u64		__resv[9];
+};
+
+struct io_uring_mock_create {
+	__u32		out_fd;
+	__u32		flags;
+	__u64		__resv[15];
+};
+
+enum {
+	IORING_MOCK_MGR_CMD_PROBE,
+	IORING_MOCK_MGR_CMD_CREATE,
+};
+
+#endif
-- 
2.49.0

[PATCH v4 2/6] io_uring/mock: add cmd using vectored regbufs

[Pavel Begunkov]

There is a command api allowing to import vectored registered buffers,
add a new mock command that uses the feature and simply copies the
specified registered buffer into user space or vice versa.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/mock_file.c | 70 +++++++++++++++++++++++++++++++++++++++++++-
 io_uring/mock_file.h | 14 +++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/io_uring/mock_file.c b/io_uring/mock_file.c
index 3bb5614c85f1..4982f71a3a9b 100644
--- a/io_uring/mock_file.c
+++ b/io_uring/mock_file.c
@@ -9,8 +9,76 @@
 #include <linux/io_uring_types.h>
 #include "mock_file.h"
 
+#define IO_VALID_COPY_CMD_FLAGS		IORING_MOCK_COPY_FROM
+
+static int io_copy_regbuf(struct iov_iter *reg_iter, void __user *ubuf)
+{
+	size_t ret, copied = 0;
+	size_t buflen = PAGE_SIZE;
+	void *tmp_buf;
+
+	tmp_buf = kzalloc(buflen, GFP_KERNEL);
+	if (!tmp_buf)
+		return -ENOMEM;
+
+	while (iov_iter_count(reg_iter)) {
+		size_t len = min(iov_iter_count(reg_iter), buflen);
+
+		if (iov_iter_rw(reg_iter) == ITER_SOURCE) {
+			ret = copy_from_iter(tmp_buf, len, reg_iter);
+			if (ret <= 0)
+				break;
+			if (copy_to_user(ubuf, tmp_buf, ret))
+				break;
+		} else {
+			if (copy_from_user(tmp_buf, ubuf, len))
+				break;
+			ret = copy_to_iter(tmp_buf, len, reg_iter);
+			if (ret <= 0)
+				break;
+		}
+		ubuf += ret;
+		copied += ret;
+	}
+
+	kfree(tmp_buf);
+	return copied;
+}
+
+static int io_cmd_copy_regbuf(struct io_uring_cmd *cmd, unsigned int issue_flags)
+{
+	const struct io_uring_sqe *sqe = cmd->sqe;
+	const struct iovec __user *iovec;
+	unsigned flags, iovec_len;
+	struct iov_iter iter;
+	void __user *ubuf;
+	int dir, ret;
+
+	ubuf = u64_to_user_ptr(READ_ONCE(sqe->addr3));
+	iovec = u64_to_user_ptr(READ_ONCE(sqe->addr));
+	iovec_len = READ_ONCE(sqe->len);
+	flags = READ_ONCE(sqe->file_index);
+
+	if (unlikely(sqe->ioprio || sqe->__pad1))
+		return -EINVAL;
+	if (flags & ~IO_VALID_COPY_CMD_FLAGS)
+		return -EINVAL;
+
+	dir = (flags & IORING_MOCK_COPY_FROM) ? ITER_SOURCE : ITER_DEST;
+	ret = io_uring_cmd_import_fixed_vec(cmd, iovec, iovec_len, dir, &iter,
+					    issue_flags);
+	if (ret)
+		return ret;
+	ret = io_copy_regbuf(&iter, ubuf);
+	return ret ? ret : -EFAULT;
+}
+
 static int io_mock_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
+	switch (cmd->cmd_op) {
+	case IORING_MOCK_CMD_COPY_REGBUF:
+		return io_cmd_copy_regbuf(cmd, issue_flags);
+	}
 	return -ENOTSUPP;
 }
 
@@ -85,7 +153,7 @@ static int io_probe_mock(struct io_uring_cmd *cmd)
 	if (!mem_is_zero(&mp, sizeof(mp)))
 		return -EINVAL;
 
-	mp.features = 0;
+	mp.features = IORING_MOCK_FEAT_END;
 
 	if (copy_to_user(uarg, &mp, uarg_size))
 		return -EFAULT;
diff --git a/io_uring/mock_file.h b/io_uring/mock_file.h
index 8b00045480cd..8475dfd827e9 100644
--- a/io_uring/mock_file.h
+++ b/io_uring/mock_file.h
@@ -3,6 +3,12 @@
 
 #include <linux/types.h>
 
+enum {
+	IORING_MOCK_FEAT_CMD_COPY,
+
+	IORING_MOCK_FEAT_END,
+};
+
 struct io_uring_mock_probe {
 	__u64		features;
 	__u64		__resv[9];
@@ -19,4 +25,12 @@ enum {
 	IORING_MOCK_MGR_CMD_CREATE,
 };
 
+enum {
+	IORING_MOCK_CMD_COPY_REGBUF,
+};
+
+enum {
+	IORING_MOCK_COPY_FROM			= 1,
+};
+
 #endif
-- 
2.49.0

[PATCH v4 3/6] io_uring/mock: add sync read/write

[Pavel Begunkov]

Add support for synchronous zero read/write for mock files.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/mock_file.c | 67 ++++++++++++++++++++++++++++++++++++++++----
 io_uring/mock_file.h |  4 ++-
 2 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/io_uring/mock_file.c b/io_uring/mock_file.c
index 4982f71a3a9b..7dd87d0b83b2 100644
--- a/io_uring/mock_file.c
+++ b/io_uring/mock_file.c
@@ -9,6 +9,10 @@
 #include <linux/io_uring_types.h>
 #include "mock_file.h"
 
+struct io_mock_file {
+	size_t size;
+};
+
 #define IO_VALID_COPY_CMD_FLAGS		IORING_MOCK_COPY_FROM
 
 static int io_copy_regbuf(struct iov_iter *reg_iter, void __user *ubuf)
@@ -82,18 +86,59 @@ static int io_mock_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 	return -ENOTSUPP;
 }
 
+static ssize_t io_mock_read_iter(struct kiocb *iocb, struct iov_iter *to)
+{
+	struct io_mock_file *mf = iocb->ki_filp->private_data;
+	size_t len = iov_iter_count(to);
+
+	if (iocb->ki_pos + len > mf->size)
+		return -EINVAL;
+	return iov_iter_zero(len, to);
+}
+
+static ssize_t io_mock_write_iter(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct io_mock_file *mf = iocb->ki_filp->private_data;
+	size_t len = iov_iter_count(from);
+
+	if (iocb->ki_pos + len > mf->size)
+		return -EINVAL;
+	iov_iter_advance(from, len);
+	return len;
+}
+
+static loff_t io_mock_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct io_mock_file *mf = file->private_data;
+
+	return fixed_size_llseek(file, offset, whence, mf->size);
+}
+
+static int io_mock_release(struct inode *inode, struct file *file)
+{
+	struct io_mock_file *mf = file->private_data;
+
+	kfree(mf);
+	return 0;
+}
+
 static const struct file_operations io_mock_fops = {
 	.owner		= THIS_MODULE,
+	.release	= io_mock_release,
 	.uring_cmd	= io_mock_cmd,
+	.read_iter	= io_mock_read_iter,
+	.write_iter	= io_mock_write_iter,
+	.llseek		= io_mock_llseek,
 };
 
 static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
 	const struct io_uring_sqe *sqe = cmd->sqe;
 	struct io_uring_mock_create mc, __user *uarg;
+	struct io_mock_file *mf = NULL;
 	struct file *file = NULL;
 	size_t uarg_size;
-	int fd, ret;
+	int fd = -1, ret;
 
 	uarg = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	uarg_size = READ_ONCE(sqe->len);
@@ -108,18 +153,28 @@ static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flag
 		return -EFAULT;
 	if (!mem_is_zero(mc.__resv, sizeof(mc.__resv)) || mc.flags)
 		return -EINVAL;
+	if (mc.file_size > SZ_1G)
+		return -EINVAL;
+	mf = kzalloc(sizeof(*mf), GFP_KERNEL_ACCOUNT);
+	if (!mf)
+		return -ENOMEM;
 
-	fd = get_unused_fd_flags(O_RDWR | O_CLOEXEC);
+	ret = fd = get_unused_fd_flags(O_RDWR | O_CLOEXEC);
 	if (fd < 0)
-		return fd;
+		goto fail;
 
+	mf->size = mc.file_size;
 	file = anon_inode_create_getfile("[io_uring_mock]", &io_mock_fops,
-					 NULL, O_RDWR | O_CLOEXEC, NULL);
+					 mf, O_RDWR | O_CLOEXEC, NULL);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
 		goto fail;
 	}
 
+	file->f_mode |= FMODE_READ | FMODE_CAN_READ |
+			FMODE_WRITE | FMODE_CAN_WRITE |
+			FMODE_LSEEK;
+
 	mc.out_fd = fd;
 	if (copy_to_user(uarg, &mc, uarg_size)) {
 		fput(file);
@@ -130,7 +185,9 @@ static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flag
 	fd_install(fd, file);
 	return 0;
 fail:
-	put_unused_fd(fd);
+	if (fd >= 0)
+		put_unused_fd(fd);
+	kfree(mf);
 	return ret;
 }
 
diff --git a/io_uring/mock_file.h b/io_uring/mock_file.h
index 8475dfd827e9..dc6cc343410d 100644
--- a/io_uring/mock_file.h
+++ b/io_uring/mock_file.h
@@ -5,6 +5,7 @@
 
 enum {
 	IORING_MOCK_FEAT_CMD_COPY,
+	IORING_MOCK_FEAT_RW_ZERO,
 
 	IORING_MOCK_FEAT_END,
 };
@@ -17,7 +18,8 @@ struct io_uring_mock_probe {
 struct io_uring_mock_create {
 	__u32		out_fd;
 	__u32		flags;
-	__u64		__resv[15];
+	__u64		file_size;
+	__u64		__resv[14];
 };
 
 enum {
-- 
2.49.0

[PATCH v4 4/6] io_uring/mock: allow to choose FMODE_NOWAIT

[Pavel Begunkov]

Add an option to choose whether the file supports FMODE_NOWAIT, that
changes the execution path io_uring request takes.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/mock_file.c | 8 +++++++-
 io_uring/mock_file.h | 5 +++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/io_uring/mock_file.c b/io_uring/mock_file.c
index 7dd87d0b83b2..517d75fac888 100644
--- a/io_uring/mock_file.c
+++ b/io_uring/mock_file.c
@@ -131,6 +131,8 @@ static const struct file_operations io_mock_fops = {
 	.llseek		= io_mock_llseek,
 };
 
+#define IO_VALID_CREATE_FLAGS (IORING_MOCK_CREATE_F_SUPPORT_NOWAIT)
+
 static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
 	const struct io_uring_sqe *sqe = cmd->sqe;
@@ -151,7 +153,9 @@ static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flag
 	memset(&mc, 0, sizeof(mc));
 	if (copy_from_user(&mc, uarg, uarg_size))
 		return -EFAULT;
-	if (!mem_is_zero(mc.__resv, sizeof(mc.__resv)) || mc.flags)
+	if (!mem_is_zero(mc.__resv, sizeof(mc.__resv)))
+		return -EINVAL;
+	if (mc.flags & ~IO_VALID_CREATE_FLAGS)
 		return -EINVAL;
 	if (mc.file_size > SZ_1G)
 		return -EINVAL;
@@ -174,6 +178,8 @@ static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flag
 	file->f_mode |= FMODE_READ | FMODE_CAN_READ |
 			FMODE_WRITE | FMODE_CAN_WRITE |
 			FMODE_LSEEK;
+	if (mc.flags & IORING_MOCK_CREATE_F_SUPPORT_NOWAIT)
+		file->f_mode |= FMODE_NOWAIT;
 
 	mc.out_fd = fd;
 	if (copy_to_user(uarg, &mc, uarg_size)) {
diff --git a/io_uring/mock_file.h b/io_uring/mock_file.h
index dc6cc343410d..b2b669f7621f 100644
--- a/io_uring/mock_file.h
+++ b/io_uring/mock_file.h
@@ -6,6 +6,7 @@
 enum {
 	IORING_MOCK_FEAT_CMD_COPY,
 	IORING_MOCK_FEAT_RW_ZERO,
+	IORING_MOCK_FEAT_RW_NOWAIT,
 
 	IORING_MOCK_FEAT_END,
 };
@@ -15,6 +16,10 @@ struct io_uring_mock_probe {
 	__u64		__resv[9];
 };
 
+enum {
+	IORING_MOCK_CREATE_F_SUPPORT_NOWAIT			= 1,
+};
+
 struct io_uring_mock_create {
 	__u32		out_fd;
 	__u32		flags;
-- 
2.49.0

[PATCH v4 5/6] io_uring/mock: support for async read/write

[Pavel Begunkov]

Let the user to specify a delay to read/write request. io_uring will
start a timer, return -EIOCBQUEUED and complete the request
asynchronously after the delay pass.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/mock_file.c | 59 +++++++++++++++++++++++++++++++++++++++++---
 io_uring/mock_file.h |  4 ++-
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/io_uring/mock_file.c b/io_uring/mock_file.c
index 517d75fac888..2f9a0269eedd 100644
--- a/io_uring/mock_file.c
+++ b/io_uring/mock_file.c
@@ -4,13 +4,22 @@
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/anon_inodes.h>
+#include <linux/ktime.h>
+#include <linux/hrtimer.h>
 
 #include <linux/io_uring/cmd.h>
 #include <linux/io_uring_types.h>
 #include "mock_file.h"
 
+struct io_mock_iocb {
+	struct kiocb		*iocb;
+	struct hrtimer		timer;
+	int			res;
+};
+
 struct io_mock_file {
-	size_t size;
+	size_t			size;
+	u64			rw_delay_ns;
 };
 
 #define IO_VALID_COPY_CMD_FLAGS		IORING_MOCK_COPY_FROM
@@ -86,14 +95,48 @@ static int io_mock_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 	return -ENOTSUPP;
 }
 
+static enum hrtimer_restart io_mock_rw_timer_expired(struct hrtimer *timer)
+{
+	struct io_mock_iocb *mio = container_of(timer, struct io_mock_iocb, timer);
+	struct kiocb *iocb = mio->iocb;
+
+	WRITE_ONCE(iocb->private, NULL);
+	iocb->ki_complete(iocb, mio->res);
+	kfree(mio);
+	return HRTIMER_NORESTART;
+}
+
+static ssize_t io_mock_delay_rw(struct kiocb *iocb, size_t len)
+{
+	struct io_mock_file *mf = iocb->ki_filp->private_data;
+	struct io_mock_iocb *mio;
+
+	mio = kzalloc(sizeof(*mio), GFP_KERNEL);
+	if (!mio)
+		return -ENOMEM;
+
+	mio->iocb = iocb;
+	mio->res = len;
+	hrtimer_setup(&mio->timer, io_mock_rw_timer_expired,
+		      CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hrtimer_start(&mio->timer, ns_to_ktime(mf->rw_delay_ns),
+		      HRTIMER_MODE_REL);
+	return -EIOCBQUEUED;
+}
+
 static ssize_t io_mock_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	struct io_mock_file *mf = iocb->ki_filp->private_data;
 	size_t len = iov_iter_count(to);
+	size_t nr_zeroed;
 
 	if (iocb->ki_pos + len > mf->size)
 		return -EINVAL;
-	return iov_iter_zero(len, to);
+	nr_zeroed = iov_iter_zero(len, to);
+	if (!mf->rw_delay_ns || nr_zeroed != len)
+		return nr_zeroed;
+
+	return io_mock_delay_rw(iocb, len);
 }
 
 static ssize_t io_mock_write_iter(struct kiocb *iocb, struct iov_iter *from)
@@ -103,8 +146,12 @@ static ssize_t io_mock_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	if (iocb->ki_pos + len > mf->size)
 		return -EINVAL;
-	iov_iter_advance(from, len);
-	return len;
+	if (!mf->rw_delay_ns) {
+		iov_iter_advance(from, len);
+		return len;
+	}
+
+	return io_mock_delay_rw(iocb, len);
 }
 
 static loff_t io_mock_llseek(struct file *file, loff_t offset, int whence)
@@ -159,6 +206,9 @@ static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flag
 		return -EINVAL;
 	if (mc.file_size > SZ_1G)
 		return -EINVAL;
+	if (mc.rw_delay_ns > NSEC_PER_SEC)
+		return -EINVAL;
+
 	mf = kzalloc(sizeof(*mf), GFP_KERNEL_ACCOUNT);
 	if (!mf)
 		return -ENOMEM;
@@ -168,6 +218,7 @@ static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flag
 		goto fail;
 
 	mf->size = mc.file_size;
+	mf->rw_delay_ns = mc.rw_delay_ns;
 	file = anon_inode_create_getfile("[io_uring_mock]", &io_mock_fops,
 					 mf, O_RDWR | O_CLOEXEC, NULL);
 	if (IS_ERR(file)) {
diff --git a/io_uring/mock_file.h b/io_uring/mock_file.h
index b2b669f7621f..0ca03562359f 100644
--- a/io_uring/mock_file.h
+++ b/io_uring/mock_file.h
@@ -7,6 +7,7 @@ enum {
 	IORING_MOCK_FEAT_CMD_COPY,
 	IORING_MOCK_FEAT_RW_ZERO,
 	IORING_MOCK_FEAT_RW_NOWAIT,
+	IORING_MOCK_FEAT_RW_ASYNC,
 
 	IORING_MOCK_FEAT_END,
 };
@@ -24,7 +25,8 @@ struct io_uring_mock_create {
 	__u32		out_fd;
 	__u32		flags;
 	__u64		file_size;
-	__u64		__resv[14];
+	__u64		rw_delay_ns;
+	__u64		__resv[13];
 };
 
 enum {
-- 
2.49.0

[PATCH v4 6/6] io_uring/mock: add trivial poll handler

[Pavel Begunkov]

Add a flag that enables polling on the mock file. For now it's trivially
says that there is always data available, it'll be extended in the
future.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/mock_file.c | 37 +++++++++++++++++++++++++++++++++++--
 io_uring/mock_file.h |  2 ++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/io_uring/mock_file.c b/io_uring/mock_file.c
index 2f9a0269eedd..81401e9a1a6b 100644
--- a/io_uring/mock_file.c
+++ b/io_uring/mock_file.c
@@ -6,6 +6,7 @@
 #include <linux/anon_inodes.h>
 #include <linux/ktime.h>
 #include <linux/hrtimer.h>
+#include <linux/poll.h>
 
 #include <linux/io_uring/cmd.h>
 #include <linux/io_uring_types.h>
@@ -20,6 +21,8 @@ struct io_mock_iocb {
 struct io_mock_file {
 	size_t			size;
 	u64			rw_delay_ns;
+	bool			pollable;
+	struct wait_queue_head	poll_wq;
 };
 
 #define IO_VALID_COPY_CMD_FLAGS		IORING_MOCK_COPY_FROM
@@ -161,6 +164,18 @@ static loff_t io_mock_llseek(struct file *file, loff_t offset, int whence)
 	return fixed_size_llseek(file, offset, whence, mf->size);
 }
 
+static __poll_t io_mock_poll(struct file *file, struct poll_table_struct *pt)
+{
+	struct io_mock_file *mf = file->private_data;
+	__poll_t mask = 0;
+
+	poll_wait(file, &mf->poll_wq, pt);
+
+	mask |= EPOLLOUT | EPOLLWRNORM;
+	mask |= EPOLLIN | EPOLLRDNORM;
+	return mask;
+}
+
 static int io_mock_release(struct inode *inode, struct file *file)
 {
 	struct io_mock_file *mf = file->private_data;
@@ -178,10 +193,22 @@ static const struct file_operations io_mock_fops = {
 	.llseek		= io_mock_llseek,
 };
 
-#define IO_VALID_CREATE_FLAGS (IORING_MOCK_CREATE_F_SUPPORT_NOWAIT)
+static const struct file_operations io_mock_poll_fops = {
+	.owner		= THIS_MODULE,
+	.release	= io_mock_release,
+	.uring_cmd	= io_mock_cmd,
+	.read_iter	= io_mock_read_iter,
+	.write_iter	= io_mock_write_iter,
+	.llseek		= io_mock_llseek,
+	.poll		= io_mock_poll,
+};
+
+#define IO_VALID_CREATE_FLAGS (IORING_MOCK_CREATE_F_SUPPORT_NOWAIT | \
+				IORING_MOCK_CREATE_F_POLL)
 
 static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
+	const struct file_operations *fops = &io_mock_fops;
 	const struct io_uring_sqe *sqe = cmd->sqe;
 	struct io_uring_mock_create mc, __user *uarg;
 	struct io_mock_file *mf = NULL;
@@ -217,9 +244,15 @@ static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flag
 	if (fd < 0)
 		goto fail;
 
+	init_waitqueue_head(&mf->poll_wq);
 	mf->size = mc.file_size;
 	mf->rw_delay_ns = mc.rw_delay_ns;
-	file = anon_inode_create_getfile("[io_uring_mock]", &io_mock_fops,
+	if (mc.flags & IORING_MOCK_CREATE_F_POLL) {
+		fops = &io_mock_poll_fops;
+		mf->pollable = true;
+	}
+
+	file = anon_inode_create_getfile("[io_uring_mock]", fops,
 					 mf, O_RDWR | O_CLOEXEC, NULL);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
diff --git a/io_uring/mock_file.h b/io_uring/mock_file.h
index 0ca03562359f..30456ea71d54 100644
--- a/io_uring/mock_file.h
+++ b/io_uring/mock_file.h
@@ -8,6 +8,7 @@ enum {
 	IORING_MOCK_FEAT_RW_ZERO,
 	IORING_MOCK_FEAT_RW_NOWAIT,
 	IORING_MOCK_FEAT_RW_ASYNC,
+	IORING_MOCK_FEAT_POLL,
 
 	IORING_MOCK_FEAT_END,
 };
@@ -19,6 +20,7 @@ struct io_uring_mock_probe {
 
 enum {
 	IORING_MOCK_CREATE_F_SUPPORT_NOWAIT			= 1,
+	IORING_MOCK_CREATE_F_POLL				= 2,
 };
 
 struct io_uring_mock_create {
-- 
2.49.0

Re: [PATCH v4 2/6] io_uring/mock: add cmd using vectored regbufs

[Jens Axboe]

On 5/30/25 6:51 AM, Pavel Begunkov wrote:
> +static int io_copy_regbuf(struct iov_iter *reg_iter, void __user *ubuf)
> +{
> +	size_t ret, copied = 0;
> +	size_t buflen = PAGE_SIZE;
> +	void *tmp_buf;
> +
> +	tmp_buf = kzalloc(buflen, GFP_KERNEL);
> +	if (!tmp_buf)
> +		return -ENOMEM;
> +
> +	while (iov_iter_count(reg_iter)) {
> +		size_t len = min(iov_iter_count(reg_iter), buflen);
> +
> +		if (iov_iter_rw(reg_iter) == ITER_SOURCE) {
> +			ret = copy_from_iter(tmp_buf, len, reg_iter);
> +			if (ret <= 0)
> +				break;
> +			if (copy_to_user(ubuf, tmp_buf, ret))
> +				break;
> +		} else {
> +			if (copy_from_user(tmp_buf, ubuf, len))
> +				break;
> +			ret = copy_to_iter(tmp_buf, len, reg_iter);
> +			if (ret <= 0)
> +				break;
> +		}

Do copy_{to,from}_iter() not follow the same "bytes not copied" return
value that the copy_{to,from}_user() do? From a quick look, looks like
they do.

Minor thing, no need for a respin just for that.

-- 
Jens Axboe

Re: [PATCH v4 5/6] io_uring/mock: support for async read/write

[Jens Axboe]

On 5/30/25 6:52 AM, Pavel Begunkov wrote:
> Let the user to specify a delay to read/write request. io_uring will
> start a timer, return -EIOCBQUEUED and complete the request
> asynchronously after the delay pass.

This is nifty. Just one question, do we want to have this delay_ns be
some kind of range? Eg complete within min_comp_ns and max_comp_ns or
something, to get some variation in there (if desired, you could set
them the same too, of course).

-- 
Jens Axboe

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Jens Axboe]

On 5/30/25 6:51 AM, Pavel Begunkov wrote:
> diff --git a/init/Kconfig b/init/Kconfig
> index 63f5974b9fa6..9e8a5b810804 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>  	  the io_uring subsystem, hence this should only be enabled for
>  	  specific test purposes.
>  
> +config IO_URING_MOCK_FILE
> +	tristate "Enable io_uring mock files (Experimental)" if EXPERT
> +	default n
> +	depends on IO_URING && KASAN
> +	help
> +	  Enable mock files for io_uring subststem testing. The ABI might
> +	  still change, so it's still experimental and should only be enabled
> +	  for specific test purposes.
> +
> +	  If unsure, say N.

As mentioned in the other email, I don't think we should include KASAN
here.

> +struct io_uring_mock_create {
> +	__u32		out_fd;
> +	__u32		flags;
> +	__u64		__resv[15];
> +};

Do we want to have a type here for this? Eg regular file, pipe, socket,
etc?

-- 
Jens Axboe

Re: [PATCH v4 2/6] io_uring/mock: add cmd using vectored regbufs

[Pavel Begunkov]

On 5/30/25 14:25, Jens Axboe wrote:
> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>> +static int io_copy_regbuf(struct iov_iter *reg_iter, void __user *ubuf)
>> +{
>> +	size_t ret, copied = 0;
>> +	size_t buflen = PAGE_SIZE;
>> +	void *tmp_buf;
>> +
>> +	tmp_buf = kzalloc(buflen, GFP_KERNEL);
>> +	if (!tmp_buf)
>> +		return -ENOMEM;
>> +
>> +	while (iov_iter_count(reg_iter)) {
>> +		size_t len = min(iov_iter_count(reg_iter), buflen);
>> +
>> +		if (iov_iter_rw(reg_iter) == ITER_SOURCE) {
>> +			ret = copy_from_iter(tmp_buf, len, reg_iter);
>> +			if (ret <= 0)
>> +				break;
>> +			if (copy_to_user(ubuf, tmp_buf, ret))
>> +				break;
>> +		} else {
>> +			if (copy_from_user(tmp_buf, ubuf, len))
>> +				break;
>> +			ret = copy_to_iter(tmp_buf, len, reg_iter);
>> +			if (ret <= 0)
>> +				break;
>> +		}
> 
> Do copy_{to,from}_iter() not follow the same "bytes not copied" return
> value that the copy_{to,from}_user() do? From a quick look, looks like
> they do.
> 
> Minor thing, no need for a respin just for that.

One returns 0 on success the other the number of processed bytes.


-- 
Pavel Begunkov

Re: [PATCH v4 5/6] io_uring/mock: support for async read/write

[Pavel Begunkov]

On 5/30/25 14:27, Jens Axboe wrote:
> On 5/30/25 6:52 AM, Pavel Begunkov wrote:
>> Let the user to specify a delay to read/write request. io_uring will
>> start a timer, return -EIOCBQUEUED and complete the request
>> asynchronously after the delay pass.
> 
> This is nifty. Just one question, do we want to have this delay_ns be
> some kind of range? Eg complete within min_comp_ns and max_comp_ns or
> something, to get some variation in there (if desired, you could set
> them the same too, of course).

That's left out for later. You can always invent dozens of such
parameters: introducing a choice of distributions, mixing,
interaction and parameters for polling, but that should rather
go in tandem with more thorough discussions on which edge cases
it needs to test.

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

On 5/30/25 14:28, Jens Axboe wrote:
> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 63f5974b9fa6..9e8a5b810804 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>   	  the io_uring subsystem, hence this should only be enabled for
>>   	  specific test purposes.
>>   
>> +config IO_URING_MOCK_FILE
>> +	tristate "Enable io_uring mock files (Experimental)" if EXPERT
>> +	default n
>> +	depends on IO_URING && KASAN
>> +	help
>> +	  Enable mock files for io_uring subststem testing. The ABI might
>> +	  still change, so it's still experimental and should only be enabled
>> +	  for specific test purposes.
>> +
>> +	  If unsure, say N.
> 
> As mentioned in the other email, I don't think we should include KASAN
> here.
> 
>> +struct io_uring_mock_create {
>> +	__u32		out_fd;
>> +	__u32		flags;
>> +	__u64		__resv[15];
>> +};
> 
> Do we want to have a type here for this? Eg regular file, pipe, socket,
> etc?

That can be added when/if they're mocked. This set tries to atomize
file rw execution characteristics, pipes should be just a particular
set of those (e.g. +option stream vs offset based file). There might
be some interest to test some interactions like with page cache, but
that's beyond the scope, at least for now.

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

On 5/30/25 14:28, Jens Axboe wrote:
> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 63f5974b9fa6..9e8a5b810804 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>   	  the io_uring subsystem, hence this should only be enabled for
>>   	  specific test purposes.
>>   
>> +config IO_URING_MOCK_FILE
>> +	tristate "Enable io_uring mock files (Experimental)" if EXPERT
>> +	default n
>> +	depends on IO_URING && KASAN
>> +	help
>> +	  Enable mock files for io_uring subststem testing. The ABI might
>> +	  still change, so it's still experimental and should only be enabled
>> +	  for specific test purposes.
>> +
>> +	  If unsure, say N.
> 
> As mentioned in the other email, I don't think we should include KASAN
> here.

I disagree. It's supposed to give a superset of coverage, if not,
mocking should be improved. It might be seen as a nuisance that you
can't run it with a stock kernel, but that desire is already half
step from "let's enable it for prod kernels for testing", and then
distributions will start forcing it on, because as you said "People
do all sorts of weird stuff".

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

On 5/30/25 15:09, Pavel Begunkov wrote:
> On 5/30/25 14:28, Jens Axboe wrote:
>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>> diff --git a/init/Kconfig b/init/Kconfig
>>> index 63f5974b9fa6..9e8a5b810804 100644
>>> --- a/init/Kconfig
>>> +++ b/init/Kconfig
>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>         the io_uring subsystem, hence this should only be enabled for
>>>         specific test purposes.
>>> +config IO_URING_MOCK_FILE
>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>> +    default n
>>> +    depends on IO_URING && KASAN
>>> +    help
>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>> +      still change, so it's still experimental and should only be enabled
>>> +      for specific test purposes.
>>> +
>>> +      If unsure, say N.
>>
>> As mentioned in the other email, I don't think we should include KASAN
>> here.
> 
> I disagree. It's supposed to give a superset of coverage, if not,
> mocking should be improved. It might be seen as a nuisance that you
> can't run it with a stock kernel, but that desire is already half
> step from "let's enable it for prod kernels for testing", and then
> distributions will start forcing it on, because as you said "People
> do all sorts of weird stuff".

The purpose is to get the project even more hardened / secure through
elaborate testing, that would defeat the purpose if non test systems
will start getting errors because of some mess up, let's say in the
driver.

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

On 5/30/25 15:12, Pavel Begunkov wrote:
> On 5/30/25 15:09, Pavel Begunkov wrote:
>> On 5/30/25 14:28, Jens Axboe wrote:
>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>> diff --git a/init/Kconfig b/init/Kconfig
>>>> index 63f5974b9fa6..9e8a5b810804 100644
>>>> --- a/init/Kconfig
>>>> +++ b/init/Kconfig
>>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>>         the io_uring subsystem, hence this should only be enabled for
>>>>         specific test purposes.
>>>> +config IO_URING_MOCK_FILE
>>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>>> +    default n
>>>> +    depends on IO_URING && KASAN
>>>> +    help
>>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>>> +      still change, so it's still experimental and should only be enabled
>>>> +      for specific test purposes.
>>>> +
>>>> +      If unsure, say N.
>>>
>>> As mentioned in the other email, I don't think we should include KASAN
>>> here.
>>
>> I disagree. It's supposed to give a superset of coverage, if not,
>> mocking should be improved. It might be seen as a nuisance that you
>> can't run it with a stock kernel, but that desire is already half
>> step from "let's enable it for prod kernels for testing", and then
>> distributions will start forcing it on, because as you said "People
>> do all sorts of weird stuff".
> 
> The purpose is to get the project even more hardened / secure through
> elaborate testing, that would defeat the purpose if non test systems
> will start getting errors because of some mess up, let's say in the
> driver.

Alternatively, it doesn't help with bloating, but tainting the kernel
might be enough to serve the purpose.

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Jens Axboe]

On 5/30/25 7:57 AM, Pavel Begunkov wrote:
> On 5/30/25 14:28, Jens Axboe wrote:
>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>> diff --git a/init/Kconfig b/init/Kconfig
>>> index 63f5974b9fa6..9e8a5b810804 100644
>>> --- a/init/Kconfig
>>> +++ b/init/Kconfig
>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>         the io_uring subsystem, hence this should only be enabled for
>>>         specific test purposes.
>>>   +config IO_URING_MOCK_FILE
>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>> +    default n
>>> +    depends on IO_URING && KASAN
>>> +    help
>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>> +      still change, so it's still experimental and should only be enabled
>>> +      for specific test purposes.
>>> +
>>> +      If unsure, say N.
>>
>> As mentioned in the other email, I don't think we should include KASAN
>> here.
>>
>>> +struct io_uring_mock_create {
>>> +    __u32        out_fd;
>>> +    __u32        flags;
>>> +    __u64        __resv[15];
>>> +};
>>
>> Do we want to have a type here for this? Eg regular file, pipe, socket,
>> etc?
> 
> That can be added when/if they're mocked. This set tries to atomize
> file rw execution characteristics, pipes should be just a particular
> set of those (e.g. +option stream vs offset based file). There might
> be some interest to test some interactions like with page cache, but
> that's beyond the scope, at least for now.

That's fine, guess it's easy to add a type when everything is checked
for zeroes upfront.

-- 
Jens Axboe

Re: [PATCH v4 2/6] io_uring/mock: add cmd using vectored regbufs

[Jens Axboe]

On 5/30/25 7:40 AM, Pavel Begunkov wrote:
> On 5/30/25 14:25, Jens Axboe wrote:
>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>> +static int io_copy_regbuf(struct iov_iter *reg_iter, void __user *ubuf)
>>> +{
>>> +    size_t ret, copied = 0;
>>> +    size_t buflen = PAGE_SIZE;
>>> +    void *tmp_buf;
>>> +
>>> +    tmp_buf = kzalloc(buflen, GFP_KERNEL);
>>> +    if (!tmp_buf)
>>> +        return -ENOMEM;
>>> +
>>> +    while (iov_iter_count(reg_iter)) {
>>> +        size_t len = min(iov_iter_count(reg_iter), buflen);
>>> +
>>> +        if (iov_iter_rw(reg_iter) == ITER_SOURCE) {
>>> +            ret = copy_from_iter(tmp_buf, len, reg_iter);
>>> +            if (ret <= 0)
>>> +                break;
>>> +            if (copy_to_user(ubuf, tmp_buf, ret))
>>> +                break;
>>> +        } else {
>>> +            if (copy_from_user(tmp_buf, ubuf, len))
>>> +                break;
>>> +            ret = copy_to_iter(tmp_buf, len, reg_iter);
>>> +            if (ret <= 0)
>>> +                break;
>>> +        }
>>
>> Do copy_{to,from}_iter() not follow the same "bytes not copied" return
>> value that the copy_{to,from}_user() do? From a quick look, looks like
>> they do.
>>
>> Minor thing, no need for a respin just for that.
> 
> One returns 0 on success the other the number of processed bytes.

copy_{to,from}_user() returns bytes NOT processed, and I guess the iter
versions return bytes processed. Guess the code is fine, it's more so
the API that's a bit wonky on the copy/iter side.

-- 
Jens Axboe

Re: [PATCH v4 5/6] io_uring/mock: support for async read/write

[Jens Axboe]

On 5/30/25 7:49 AM, Pavel Begunkov wrote:
> On 5/30/25 14:27, Jens Axboe wrote:
>> On 5/30/25 6:52 AM, Pavel Begunkov wrote:
>>> Let the user to specify a delay to read/write request. io_uring will
>>> start a timer, return -EIOCBQUEUED and complete the request
>>> asynchronously after the delay pass.
>>
>> This is nifty. Just one question, do we want to have this delay_ns be
>> some kind of range? Eg complete within min_comp_ns and max_comp_ns or
>> something, to get some variation in there (if desired, you could set
>> them the same too, of course).
> 
> That's left out for later. You can always invent dozens of such
> parameters: introducing a choice of distributions, mixing,
> interaction and parameters for polling, but that should rather
> go in tandem with more thorough discussions on which edge cases
> it needs to test.

Sure, that's fine.

-- 
Jens Axboe

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Jens Axboe]

On 5/30/25 8:26 AM, Pavel Begunkov wrote:
> On 5/30/25 15:12, Pavel Begunkov wrote:
>> On 5/30/25 15:09, Pavel Begunkov wrote:
>>> On 5/30/25 14:28, Jens Axboe wrote:
>>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>>> diff --git a/init/Kconfig b/init/Kconfig
>>>>> index 63f5974b9fa6..9e8a5b810804 100644
>>>>> --- a/init/Kconfig
>>>>> +++ b/init/Kconfig
>>>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>>>         the io_uring subsystem, hence this should only be enabled for
>>>>>         specific test purposes.
>>>>> +config IO_URING_MOCK_FILE
>>>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>>>> +    default n
>>>>> +    depends on IO_URING && KASAN
>>>>> +    help
>>>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>>>> +      still change, so it's still experimental and should only be enabled
>>>>> +      for specific test purposes.
>>>>> +
>>>>> +      If unsure, say N.
>>>>
>>>> As mentioned in the other email, I don't think we should include KASAN
>>>> here.
>>>
>>> I disagree. It's supposed to give a superset of coverage, if not,
>>> mocking should be improved. It might be seen as a nuisance that you
>>> can't run it with a stock kernel, but that desire is already half
>>> step from "let's enable it for prod kernels for testing", and then
>>> distributions will start forcing it on, because as you said "People
>>> do all sorts of weird stuff".
>>
>> The purpose is to get the project even more hardened / secure through
>> elaborate testing, that would defeat the purpose if non test systems
>> will start getting errors because of some mess up, let's say in the
>> driver.
> 
> Alternatively, it doesn't help with bloating, but tainting the kernel
> might be enough to serve the purpose.

I think taint or KASAN dependencies is over-reaching. It has nothing to
do with KASAN, and there's absolutely zero reason for it to be gated on
KASAN (or lockdep, or whatever). You're never going to prevent people
from running this in odd cases, and I think it's a mistake to try and do
that. If the thing is gated on CAP_SYS_ADMIN, then that's Good Enough
imho.

It'll make my life harder for coverage testing, which I think is reason
enough alone to not have a KASAN dependency. No other test code in the
kernel has unrelated dependencies like KASAN, unless they are related to
KASAN. We should not add one here for some notion of preventing people
from running it on prod stuff, in fact it should be totally fine to run
on a prod kernel. Might actually be useful in some cases, to verify or
test some behavior on that specific kernel, without needing to build a
new kernel for it.

-- 
Jens Axboe

Re: [PATCH v4 2/6] io_uring/mock: add cmd using vectored regbufs

[Pavel Begunkov]

On 5/30/25 15:37, Jens Axboe wrote:
> On 5/30/25 7:40 AM, Pavel Begunkov wrote:
>> On 5/30/25 14:25, Jens Axboe wrote:
>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>> +static int io_copy_regbuf(struct iov_iter *reg_iter, void __user *ubuf)
>>>> +{
>>>> +    size_t ret, copied = 0;
>>>> +    size_t buflen = PAGE_SIZE;
>>>> +    void *tmp_buf;
>>>> +
>>>> +    tmp_buf = kzalloc(buflen, GFP_KERNEL);
>>>> +    if (!tmp_buf)
>>>> +        return -ENOMEM;
>>>> +
>>>> +    while (iov_iter_count(reg_iter)) {
>>>> +        size_t len = min(iov_iter_count(reg_iter), buflen);
>>>> +
>>>> +        if (iov_iter_rw(reg_iter) == ITER_SOURCE) {
>>>> +            ret = copy_from_iter(tmp_buf, len, reg_iter);
>>>> +            if (ret <= 0)
>>>> +                break;
>>>> +            if (copy_to_user(ubuf, tmp_buf, ret))
>>>> +                break;
>>>> +        } else {
>>>> +            if (copy_from_user(tmp_buf, ubuf, len))
>>>> +                break;
>>>> +            ret = copy_to_iter(tmp_buf, len, reg_iter);
>>>> +            if (ret <= 0)
>>>> +                break;
>>>> +        }
>>>
>>> Do copy_{to,from}_iter() not follow the same "bytes not copied" return
>>> value that the copy_{to,from}_user() do? From a quick look, looks like
>>> they do.
>>>
>>> Minor thing, no need for a respin just for that.
>>
>> One returns 0 on success the other the number of processed bytes.
> 
> copy_{to,from}_user() returns bytes NOT processed, and I guess the iter

Sure, it doesn't contradict it, they follow different semantics.

> versions return bytes processed. Guess the code is fine, it's more so
> the API that's a bit wonky on the copy/iter side.

Which API? This command or copy helpers? copy_{to,from}_user are used
here in the way they're always used in the kernel, and that's fine,
they're not supposed to fail for valid input. That's unlike iter
helpers, which may return a partial result.

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

On 5/30/25 15:41, Jens Axboe wrote:
> On 5/30/25 8:26 AM, Pavel Begunkov wrote:
>> On 5/30/25 15:12, Pavel Begunkov wrote:
>>> On 5/30/25 15:09, Pavel Begunkov wrote:
>>>> On 5/30/25 14:28, Jens Axboe wrote:
>>>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>>>> diff --git a/init/Kconfig b/init/Kconfig
>>>>>> index 63f5974b9fa6..9e8a5b810804 100644
>>>>>> --- a/init/Kconfig
>>>>>> +++ b/init/Kconfig
>>>>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>>>>          the io_uring subsystem, hence this should only be enabled for
>>>>>>          specific test purposes.
>>>>>> +config IO_URING_MOCK_FILE
>>>>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>>>>> +    default n
>>>>>> +    depends on IO_URING && KASAN
>>>>>> +    help
>>>>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>>>>> +      still change, so it's still experimental and should only be enabled
>>>>>> +      for specific test purposes.
>>>>>> +
>>>>>> +      If unsure, say N.
>>>>>
>>>>> As mentioned in the other email, I don't think we should include KASAN
>>>>> here.
>>>>
>>>> I disagree. It's supposed to give a superset of coverage, if not,
>>>> mocking should be improved. It might be seen as a nuisance that you
>>>> can't run it with a stock kernel, but that desire is already half
>>>> step from "let's enable it for prod kernels for testing", and then
>>>> distributions will start forcing it on, because as you said "People
>>>> do all sorts of weird stuff".
>>>
>>> The purpose is to get the project even more hardened / secure through
>>> elaborate testing, that would defeat the purpose if non test systems
>>> will start getting errors because of some mess up, let's say in the
>>> driver.
>>
>> Alternatively, it doesn't help with bloating, but tainting the kernel
>> might be enough to serve the purpose.
> 
> I think taint or KASAN dependencies is over-reaching. It has nothing to
> do with KASAN, and there's absolutely zero reason for it to be gated on
> KASAN (or lockdep, or whatever). You're never going to prevent people
> from running this in odd cases, and I think it's a mistake to try and do
> that. If the thing is gated on CAP_SYS_ADMIN, then that's Good Enough
> imho.
> 
> It'll make my life harder for coverage testing, which I think is reason
> enough alone to not have a KASAN dependency. No other test code in the
> kernel has unrelated dependencies like KASAN, unless they are related to
> KASAN. We should not add one here for some notion of preventing people
> from running it on prod stuff, in fact it should be totally fine to run
> on a prod kernel. Might actually be useful in some cases, to verify or
> test some behavior on that specific kernel, without needing to build a
> new kernel for it.

commit 2852ca7fba9f77b204f0fe953b31fadd0057c936
Author: David Gow <davidgow@google.com>
Date:   Fri Jul 1 16:47:41 2022 +0800

     panic: Taint kernel if tests are run
     
     Most in-kernel tests (such as KUnit tests) are not supposed to run on
     production systems: they may do deliberately illegal things to trigger
     errors, and have security implications (for example, KUnit assertions
     will often deliberately leak kernel addresses).
     
     Add a new taint type, TAINT_TEST to signal that a test has been run.
     This will be printed as 'N' (originally for kuNit, as every other
     sensible letter was taken.)
     
     This should discourage people from running these tests on production
     systems, and to make it easier to tell if tests have been run
     accidentally (by loading the wrong configuration, etc.)
     
     Acked-by: Luis Chamberlain <mcgrof@kernel.org>
     Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
     Signed-off-by: David Gow <davidgow@google.com>
     Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>


The same situation, it's a special TAINT_TEST, and set for a good
reason. And there is also a case of TAINT_CRAP for staging.

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Jens Axboe]

On 5/30/25 9:11 AM, Pavel Begunkov wrote:
> On 5/30/25 15:41, Jens Axboe wrote:
>> On 5/30/25 8:26 AM, Pavel Begunkov wrote:
>>> On 5/30/25 15:12, Pavel Begunkov wrote:
>>>> On 5/30/25 15:09, Pavel Begunkov wrote:
>>>>> On 5/30/25 14:28, Jens Axboe wrote:
>>>>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>>>>> diff --git a/init/Kconfig b/init/Kconfig
>>>>>>> index 63f5974b9fa6..9e8a5b810804 100644
>>>>>>> --- a/init/Kconfig
>>>>>>> +++ b/init/Kconfig
>>>>>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>>>>>          the io_uring subsystem, hence this should only be enabled for
>>>>>>>          specific test purposes.
>>>>>>> +config IO_URING_MOCK_FILE
>>>>>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>>>>>> +    default n
>>>>>>> +    depends on IO_URING && KASAN
>>>>>>> +    help
>>>>>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>>>>>> +      still change, so it's still experimental and should only be enabled
>>>>>>> +      for specific test purposes.
>>>>>>> +
>>>>>>> +      If unsure, say N.
>>>>>>
>>>>>> As mentioned in the other email, I don't think we should include KASAN
>>>>>> here.
>>>>>
>>>>> I disagree. It's supposed to give a superset of coverage, if not,
>>>>> mocking should be improved. It might be seen as a nuisance that you
>>>>> can't run it with a stock kernel, but that desire is already half
>>>>> step from "let's enable it for prod kernels for testing", and then
>>>>> distributions will start forcing it on, because as you said "People
>>>>> do all sorts of weird stuff".
>>>>
>>>> The purpose is to get the project even more hardened / secure through
>>>> elaborate testing, that would defeat the purpose if non test systems
>>>> will start getting errors because of some mess up, let's say in the
>>>> driver.
>>>
>>> Alternatively, it doesn't help with bloating, but tainting the kernel
>>> might be enough to serve the purpose.
>>
>> I think taint or KASAN dependencies is over-reaching. It has nothing to
>> do with KASAN, and there's absolutely zero reason for it to be gated on
>> KASAN (or lockdep, or whatever). You're never going to prevent people
>> from running this in odd cases, and I think it's a mistake to try and do
>> that. If the thing is gated on CAP_SYS_ADMIN, then that's Good Enough
>> imho.
>>
>> It'll make my life harder for coverage testing, which I think is reason
>> enough alone to not have a KASAN dependency. No other test code in the
>> kernel has unrelated dependencies like KASAN, unless they are related to
>> KASAN. We should not add one here for some notion of preventing people
>> from running it on prod stuff, in fact it should be totally fine to run
>> on a prod kernel. Might actually be useful in some cases, to verify or
>> test some behavior on that specific kernel, without needing to build a
>> new kernel for it.
> 
> commit 2852ca7fba9f77b204f0fe953b31fadd0057c936
> Author: David Gow <davidgow@google.com>
> Date:   Fri Jul 1 16:47:41 2022 +0800
> 
>     panic: Taint kernel if tests are run
>         Most in-kernel tests (such as KUnit tests) are not supposed to run on
>     production systems: they may do deliberately illegal things to trigger
>     errors, and have security implications (for example, KUnit assertions
>     will often deliberately leak kernel addresses).
>         Add a new taint type, TAINT_TEST to signal that a test has been run.
>     This will be printed as 'N' (originally for kuNit, as every other
>     sensible letter was taken.)
>         This should discourage people from running these tests on production
>     systems, and to make it easier to tell if tests have been run
>     accidentally (by loading the wrong configuration, etc.)
>         Acked-by: Luis Chamberlain <mcgrof@kernel.org>
>     Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
>     Signed-off-by: David Gow <davidgow@google.com>
>     Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> 
> 
> The same situation, it's a special TAINT_TEST, and set for a good
> reason. And there is also a case of TAINT_CRAP for staging.

TAINT is fine, I don't care about that. So we can certainly do that. My
main objection is just to gating it on lockdep/kasan or something like
that.

-- 
Jens Axboe

Re: [PATCH v4 2/6] io_uring/mock: add cmd using vectored regbufs

[Jens Axboe]

On 5/30/25 8:53 AM, Pavel Begunkov wrote:
> On 5/30/25 15:37, Jens Axboe wrote:
>> On 5/30/25 7:40 AM, Pavel Begunkov wrote:
>>> On 5/30/25 14:25, Jens Axboe wrote:
>>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>>> +static int io_copy_regbuf(struct iov_iter *reg_iter, void __user *ubuf)
>>>>> +{
>>>>> +    size_t ret, copied = 0;
>>>>> +    size_t buflen = PAGE_SIZE;
>>>>> +    void *tmp_buf;
>>>>> +
>>>>> +    tmp_buf = kzalloc(buflen, GFP_KERNEL);
>>>>> +    if (!tmp_buf)
>>>>> +        return -ENOMEM;
>>>>> +
>>>>> +    while (iov_iter_count(reg_iter)) {
>>>>> +        size_t len = min(iov_iter_count(reg_iter), buflen);
>>>>> +
>>>>> +        if (iov_iter_rw(reg_iter) == ITER_SOURCE) {
>>>>> +            ret = copy_from_iter(tmp_buf, len, reg_iter);
>>>>> +            if (ret <= 0)
>>>>> +                break;
>>>>> +            if (copy_to_user(ubuf, tmp_buf, ret))
>>>>> +                break;
>>>>> +        } else {
>>>>> +            if (copy_from_user(tmp_buf, ubuf, len))
>>>>> +                break;
>>>>> +            ret = copy_to_iter(tmp_buf, len, reg_iter);
>>>>> +            if (ret <= 0)
>>>>> +                break;
>>>>> +        }
>>>>
>>>> Do copy_{to,from}_iter() not follow the same "bytes not copied" return
>>>> value that the copy_{to,from}_user() do? From a quick look, looks like
>>>> they do.
>>>>
>>>> Minor thing, no need for a respin just for that.
>>>
>>> One returns 0 on success the other the number of processed bytes.
>>
>> copy_{to,from}_user() returns bytes NOT processed, and I guess the iter
> 
> Sure, it doesn't contradict it, they follow different semantics.
> 
>> versions return bytes processed. Guess the code is fine, it's more so
>> the API that's a bit wonky on the copy/iter side.
> 
> Which API? This command or copy helpers? copy_{to,from}_user are used
> here in the way they're always used in the kernel, and that's fine,
> they're not supposed to fail for valid input. That's unlike iter
> helpers, which may return a partial result.

Not this command, the copy helpers in the kernel.

-- 
Jens Axboe

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Keith Busch]

On Fri, May 30, 2025 at 01:51:58PM +0100, Pavel Begunkov wrote:
> +++ b/io_uring/mock_file.h
> @@ -0,0 +1,22 @@
> +#ifndef IOU_MOCK_H
> +#define IOU_MOCK_H
> +
> +#include <linux/types.h>
> +
> +struct io_uring_mock_probe {
> +	__u64		features;
> +	__u64		__resv[9];
> +};
> +
> +struct io_uring_mock_create {
> +	__u32		out_fd;
> +	__u32		flags;
> +	__u64		__resv[15];
> +};
> +
> +enum {
> +	IORING_MOCK_MGR_CMD_PROBE,
> +	IORING_MOCK_MGR_CMD_CREATE,
> +};
> +
> +#endif

Shouldn't this go in a linux/uapi header instead?

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

On 5/30/25 16:30, Jens Axboe wrote:
> On 5/30/25 9:11 AM, Pavel Begunkov wrote:
>> On 5/30/25 15:41, Jens Axboe wrote:
>>> On 5/30/25 8:26 AM, Pavel Begunkov wrote:
>>>> On 5/30/25 15:12, Pavel Begunkov wrote:
>>>>> On 5/30/25 15:09, Pavel Begunkov wrote:
>>>>>> On 5/30/25 14:28, Jens Axboe wrote:
>>>>>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>>>>>> diff --git a/init/Kconfig b/init/Kconfig
>>>>>>>> index 63f5974b9fa6..9e8a5b810804 100644
>>>>>>>> --- a/init/Kconfig
>>>>>>>> +++ b/init/Kconfig
>>>>>>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>>>>>>           the io_uring subsystem, hence this should only be enabled for
>>>>>>>>           specific test purposes.
>>>>>>>> +config IO_URING_MOCK_FILE
>>>>>>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>>>>>>> +    default n
>>>>>>>> +    depends on IO_URING && KASAN
>>>>>>>> +    help
>>>>>>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>>>>>>> +      still change, so it's still experimental and should only be enabled
>>>>>>>> +      for specific test purposes.
>>>>>>>> +
>>>>>>>> +      If unsure, say N.
>>>>>>>
>>>>>>> As mentioned in the other email, I don't think we should include KASAN
>>>>>>> here.
>>>>>>
>>>>>> I disagree. It's supposed to give a superset of coverage, if not,
>>>>>> mocking should be improved. It might be seen as a nuisance that you
>>>>>> can't run it with a stock kernel, but that desire is already half
>>>>>> step from "let's enable it for prod kernels for testing", and then
>>>>>> distributions will start forcing it on, because as you said "People
>>>>>> do all sorts of weird stuff".
>>>>>
>>>>> The purpose is to get the project even more hardened / secure through
>>>>> elaborate testing, that would defeat the purpose if non test systems
>>>>> will start getting errors because of some mess up, let's say in the
>>>>> driver.
>>>>
>>>> Alternatively, it doesn't help with bloating, but tainting the kernel
>>>> might be enough to serve the purpose.
>>>
>>> I think taint or KASAN dependencies is over-reaching. It has nothing to
>>> do with KASAN, and there's absolutely zero reason for it to be gated on
>>> KASAN (or lockdep, or whatever). You're never going to prevent people
>>> from running this in odd cases, and I think it's a mistake to try and do
>>> that. If the thing is gated on CAP_SYS_ADMIN, then that's Good Enough
>>> imho.
>>>
>>> It'll make my life harder for coverage testing, which I think is reason
>>> enough alone to not have a KASAN dependency. No other test code in the
>>> kernel has unrelated dependencies like KASAN, unless they are related to
>>> KASAN. We should not add one here for some notion of preventing people
>>> from running it on prod stuff, in fact it should be totally fine to run
>>> on a prod kernel. Might actually be useful in some cases, to verify or
>>> test some behavior on that specific kernel, without needing to build a
>>> new kernel for it.
>>
>> commit 2852ca7fba9f77b204f0fe953b31fadd0057c936
>> Author: David Gow <davidgow@google.com>
>> Date:   Fri Jul 1 16:47:41 2022 +0800
>>
>>      panic: Taint kernel if tests are run
>>          Most in-kernel tests (such as KUnit tests) are not supposed to run on
>>      production systems: they may do deliberately illegal things to trigger
>>      errors, and have security implications (for example, KUnit assertions
>>      will often deliberately leak kernel addresses).
>>          Add a new taint type, TAINT_TEST to signal that a test has been run.
>>      This will be printed as 'N' (originally for kuNit, as every other
>>      sensible letter was taken.)
>>          This should discourage people from running these tests on production
>>      systems, and to make it easier to tell if tests have been run
>>      accidentally (by loading the wrong configuration, etc.)
>>          Acked-by: Luis Chamberlain <mcgrof@kernel.org>
>>      Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
>>      Signed-off-by: David Gow <davidgow@google.com>
>>      Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
>>
>>
>> The same situation, it's a special TAINT_TEST, and set for a good
>> reason. And there is also a case of TAINT_CRAP for staging.
> 
> TAINT is fine, I don't care about that. So we can certainly do that. My

Good you changed your mind

> main objection is just to gating it on lockdep/kasan or something like
> that.
> 

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

On 5/30/25 19:04, Keith Busch wrote:
> On Fri, May 30, 2025 at 01:51:58PM +0100, Pavel Begunkov wrote:
>> +++ b/io_uring/mock_file.h
>> @@ -0,0 +1,22 @@
>> +#ifndef IOU_MOCK_H
>> +#define IOU_MOCK_H
>> +
>> +#include <linux/types.h>
>> +
>> +struct io_uring_mock_probe {
>> +	__u64		features;
>> +	__u64		__resv[9];
>> +};
>> +
>> +struct io_uring_mock_create {
>> +	__u32		out_fd;
>> +	__u32		flags;
>> +	__u64		__resv[15];
>> +};
>> +
>> +enum {
>> +	IORING_MOCK_MGR_CMD_PROBE,
>> +	IORING_MOCK_MGR_CMD_CREATE,
>> +};
>> +
>> +#endif
> 
> Shouldn't this go in a linux/uapi header instead?

Depends on whether we want distribute it to machines where it's
not supposed to be enabled ever. I couldn't find any notion of
debug headers in header installation, what people usually do
in this case?

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Jens Axboe]

On 5/30/25 12:21 PM, Pavel Begunkov wrote:
> On 5/30/25 19:04, Keith Busch wrote:
>> On Fri, May 30, 2025 at 01:51:58PM +0100, Pavel Begunkov wrote:
>>> +++ b/io_uring/mock_file.h
>>> @@ -0,0 +1,22 @@
>>> +#ifndef IOU_MOCK_H
>>> +#define IOU_MOCK_H
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +struct io_uring_mock_probe {
>>> +    __u64        features;
>>> +    __u64        __resv[9];
>>> +};
>>> +
>>> +struct io_uring_mock_create {
>>> +    __u32        out_fd;
>>> +    __u32        flags;
>>> +    __u64        __resv[15];
>>> +};
>>> +
>>> +enum {
>>> +    IORING_MOCK_MGR_CMD_PROBE,
>>> +    IORING_MOCK_MGR_CMD_CREATE,
>>> +};
>>> +
>>> +#endif
>>
>> Shouldn't this go in a linux/uapi header instead?
> 
> Depends on whether we want distribute it to machines where it's
> not supposed to be enabled ever. I couldn't find any notion of
> debug headers in header installation, what people usually do
> in this case?

It still needs to go in uapi, as it is a user api of sorts. Otherwise
we'd need to duplicate it in test programs.

-- 
Jens Axboe

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Jens Axboe]

On 5/30/25 12:14 PM, Pavel Begunkov wrote:
> On 5/30/25 16:30, Jens Axboe wrote:
>> On 5/30/25 9:11 AM, Pavel Begunkov wrote:
>>> On 5/30/25 15:41, Jens Axboe wrote:
>>>> On 5/30/25 8:26 AM, Pavel Begunkov wrote:
>>>>> On 5/30/25 15:12, Pavel Begunkov wrote:
>>>>>> On 5/30/25 15:09, Pavel Begunkov wrote:
>>>>>>> On 5/30/25 14:28, Jens Axboe wrote:
>>>>>>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>>>>>>> diff --git a/init/Kconfig b/init/Kconfig
>>>>>>>>> index 63f5974b9fa6..9e8a5b810804 100644
>>>>>>>>> --- a/init/Kconfig
>>>>>>>>> +++ b/init/Kconfig
>>>>>>>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>>>>>>>           the io_uring subsystem, hence this should only be enabled for
>>>>>>>>>           specific test purposes.
>>>>>>>>> +config IO_URING_MOCK_FILE
>>>>>>>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>>>>>>>> +    default n
>>>>>>>>> +    depends on IO_URING && KASAN
>>>>>>>>> +    help
>>>>>>>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>>>>>>>> +      still change, so it's still experimental and should only be enabled
>>>>>>>>> +      for specific test purposes.
>>>>>>>>> +
>>>>>>>>> +      If unsure, say N.
>>>>>>>>
>>>>>>>> As mentioned in the other email, I don't think we should include KASAN
>>>>>>>> here.
>>>>>>>
>>>>>>> I disagree. It's supposed to give a superset of coverage, if not,
>>>>>>> mocking should be improved. It might be seen as a nuisance that you
>>>>>>> can't run it with a stock kernel, but that desire is already half
>>>>>>> step from "let's enable it for prod kernels for testing", and then
>>>>>>> distributions will start forcing it on, because as you said "People
>>>>>>> do all sorts of weird stuff".
>>>>>>
>>>>>> The purpose is to get the project even more hardened / secure through
>>>>>> elaborate testing, that would defeat the purpose if non test systems
>>>>>> will start getting errors because of some mess up, let's say in the
>>>>>> driver.
>>>>>
>>>>> Alternatively, it doesn't help with bloating, but tainting the kernel
>>>>> might be enough to serve the purpose.
>>>>
>>>> I think taint or KASAN dependencies is over-reaching. It has nothing to
>>>> do with KASAN, and there's absolutely zero reason for it to be gated on
>>>> KASAN (or lockdep, or whatever). You're never going to prevent people
>>>> from running this in odd cases, and I think it's a mistake to try and do
>>>> that. If the thing is gated on CAP_SYS_ADMIN, then that's Good Enough
>>>> imho.
>>>>
>>>> It'll make my life harder for coverage testing, which I think is reason
>>>> enough alone to not have a KASAN dependency. No other test code in the
>>>> kernel has unrelated dependencies like KASAN, unless they are related to
>>>> KASAN. We should not add one here for some notion of preventing people
>>>> from running it on prod stuff, in fact it should be totally fine to run
>>>> on a prod kernel. Might actually be useful in some cases, to verify or
>>>> test some behavior on that specific kernel, without needing to build a
>>>> new kernel for it.
>>>
>>> commit 2852ca7fba9f77b204f0fe953b31fadd0057c936
>>> Author: David Gow <davidgow@google.com>
>>> Date:   Fri Jul 1 16:47:41 2022 +0800
>>>
>>>      panic: Taint kernel if tests are run
>>>          Most in-kernel tests (such as KUnit tests) are not supposed to run on
>>>      production systems: they may do deliberately illegal things to trigger
>>>      errors, and have security implications (for example, KUnit assertions
>>>      will often deliberately leak kernel addresses).
>>>          Add a new taint type, TAINT_TEST to signal that a test has been run.
>>>      This will be printed as 'N' (originally for kuNit, as every other
>>>      sensible letter was taken.)
>>>          This should discourage people from running these tests on production
>>>      systems, and to make it easier to tell if tests have been run
>>>      accidentally (by loading the wrong configuration, etc.)
>>>          Acked-by: Luis Chamberlain <mcgrof@kernel.org>
>>>      Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
>>>      Signed-off-by: David Gow <davidgow@google.com>
>>>      Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
>>>
>>>
>>> The same situation, it's a special TAINT_TEST, and set for a good
>>> reason. And there is also a case of TAINT_CRAP for staging.
>>
>> TAINT is fine, I don't care about that. So we can certainly do that. My
> 
> Good you changed your mind

Yes, my main objection was (and is) having nonsensical dependencies.

-- 
Jens Axboe

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov]

On 6/2/25 16:19, Jens Axboe wrote:
> On 5/30/25 12:14 PM, Pavel Begunkov wrote:
>> On 5/30/25 16:30, Jens Axboe wrote:
...>>>> The same situation, it's a special TAINT_TEST, and set for a good
>>>> reason. And there is also a case of TAINT_CRAP for staging.
>>>
>>> TAINT is fine, I don't care about that. So we can certainly do that. My
>>
>> Good you changed your mind
> 
> Yes, my main objection was (and is) having nonsensical dependencies.

"I think taint (snip) is over-reaching" says otherwise. Anyway, it's
regrettable you don't take security arguments into consideration, but
that's your choice.

-- 
Pavel Begunkov

Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Jens Axboe]

On 6/2/25 9:31 AM, Pavel Begunkov wrote:
> On 6/2/25 16:19, Jens Axboe wrote:
>> On 5/30/25 12:14 PM, Pavel Begunkov wrote:
>>> On 5/30/25 16:30, Jens Axboe wrote:
> ...>>>> The same situation, it's a special TAINT_TEST, and set for a good
>>>>> reason. And there is also a case of TAINT_CRAP for staging.
>>>>
>>>> TAINT is fine, I don't care about that. So we can certainly do that. My
>>>
>>> Good you changed your mind
>>
>> Yes, my main objection was (and is) having nonsensical dependencies.
> 
> "I think taint (snip) is over-reaching" says otherwise. 

Since you clearly want to keep harping on this, I said "yes" and clarified
that the main objection was bogus dependencies.

Can we stop wasting time on this?

> Anyway, it's
> regrettable you don't take security arguments into consideration, but
> that's your choice.

??

-- 
Jens Axboe