Re: [LTP] [RFC] [PATCH 1/2] io_uring: Test IORING READ and WRITE operations

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
>  # Tests below may cause kernel memory leak
>  perf_event_open03 perf_event_open03
> diff --git a/testcases/kernel/syscalls/io_uring/.gitignore b/testcases/kernel/syscalls/io_uring/.gitignore
> index 749db17db..9382ae413 100644
> --- a/testcases/kernel/syscalls/io_uring/.gitignore
> +++ b/testcases/kernel/syscalls/io_uring/.gitignore
> @@ -1,2 +1,3 @@
>  /io_uring01
>  /io_uring02
> +/io_uring03
> diff --git a/testcases/kernel/syscalls/io_uring/io_uring03.c b/testcases/kernel/syscalls/io_uring/io_uring03.c
> new file mode 100644
> index 000000000..53d4feae5
> --- /dev/null
> +++ b/testcases/kernel/syscalls/io_uring/io_uring03.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 IBM
> + * Author: Sachin Sant <sachinp@linux.ibm.com>
> + *
> + * Test IORING_OP_READ and IORING_OP_WRITE operations.
> + *
> + * This test validates basic read and write operations using io_uring.
> + * It tests:
> + * 1. IORING_OP_WRITE - Writing data to a file
> + * 2. IORING_OP_READ - Reading data from a file
> + * 3. Data integrity verification
> + */

The second half of the comment should be doc comment (starts with /*\)
so that it's picked up by the documentation parser and exported into the
online documentation.

> +#include "io_uring_common.h"
> +
> +#define TEST_FILE "io_uring_test_file"
> +#define QUEUE_DEPTH 2
> +#define BLOCK_SZ 4096
> +
> +static char write_buf[BLOCK_SZ];
> +static char read_buf[BLOCK_SZ];
> +static struct io_uring_submit s;
> +static sigset_t sig;
> +
> +static void test_write_read(void)
> +{
> +	int fd;
> +	size_t i;
> +
> +	/* Prepare write buffer with pattern */

There a lot of comments like this that are commenting the
obvious. Comments that comment obvious does not add any value and
shouldn't be added.

> +	for (i = 0; i < BLOCK_SZ; i++)
> +		write_buf[i] = 'A' + (i % 26);

This should be done only once in the test setup.

> +	/* Open file for writing */
> +	fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT | O_TRUNC, 0644);
> +
> +	/* Test IORING_OP_WRITE */
> +	tst_res(TINFO, "Testing IORING_OP_WRITE");
> +	io_uring_submit_sqe(&s, fd, IORING_OP_WRITE, write_buf, BLOCK_SZ, 0);



> +	if (io_uring_wait_cqe(&s, BLOCK_SZ, IORING_OP_WRITE, &sig) == 0)
> +		tst_res(TPASS, "IORING_OP_WRITE completed successfully");
> +
> +	/* Sync to ensure data is written */
> +	SAFE_FSYNC(fd);
> +
> +	/* Test IORING_OP_READ */
> +	tst_res(TINFO, "Testing IORING_OP_READ");
> +	memset(read_buf, 0, BLOCK_SZ);
> +	io_uring_submit_sqe(&s, fd, IORING_OP_READ, read_buf, BLOCK_SZ, 0);
> +
> +	if (io_uring_wait_cqe(&s, BLOCK_SZ, IORING_OP_READ, &sig) == 0)
> +		tst_res(TPASS, "IORING_OP_READ completed successfully");
> +
> +	/* Verify data integrity */
> +	if (memcmp(write_buf, read_buf, BLOCK_SZ) == 0) {
> +		tst_res(TPASS, "Data integrity verified");
> +	} else {
> +		tst_res(TFAIL, "Data mismatch after read");
> +		for (i = 0; i < BLOCK_SZ && i < 64; i++) {
> +			if (write_buf[i] != read_buf[i]) {
> +				tst_res(TINFO, "First mismatch at offset %zu: "
> +					"wrote 0x%02x, read 0x%02x",
> +					i, write_buf[i], read_buf[i]);
> +				break;
> +			}
> +		}
> +	}

This should really be put into a function and used in both test cases.

> +	SAFE_CLOSE(fd);
> +}
> +
> +static void test_partial_io(void)
> +{
> +	int fd;
> +	size_t half = BLOCK_SZ / 2;
> +	size_t i;
> +
> +	tst_res(TINFO, "Testing partial I/O operations");
> +
> +	/* Prepare buffer */
> +	for (i = 0; i < BLOCK_SZ; i++)
> +		write_buf[i] = 'a' + (i % 26);
> +
> +	fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT | O_TRUNC, 0644);
> +
> +	/* Write first half */
> +	io_uring_submit_sqe(&s, fd, IORING_OP_WRITE, write_buf, half, 0);
> +	if (io_uring_wait_cqe(&s, half, IORING_OP_WRITE, &sig) == 0)
> +		tst_res(TPASS, "Partial write (first half) succeeded");
> +
> +	/* Write second half */
> +	io_uring_submit_sqe(&s, fd, IORING_OP_WRITE, write_buf + half,
> +			    half, half);
> +	if (io_uring_wait_cqe(&s, half, IORING_OP_WRITE, &sig) == 0)
> +		tst_res(TPASS, "Partial write (second half) succeeded");
> +
> +	SAFE_FSYNC(fd);
> +
> +	/* Read back in one operation */
> +	memset(read_buf, 0, BLOCK_SZ);
> +	io_uring_submit_sqe(&s, fd, IORING_OP_READ, read_buf, BLOCK_SZ, 0);
> +	if (io_uring_wait_cqe(&s, BLOCK_SZ, IORING_OP_READ, &sig) == 0)
> +		tst_res(TPASS, "Full read after partial writes succeeded");
> +
> +	/* Verify */
> +	if (memcmp(write_buf, read_buf, BLOCK_SZ) == 0)
> +		tst_res(TPASS, "Partial I/O data integrity verified");
> +	else
> +		tst_res(TFAIL, "Partial I/O data mismatch");
> +
> +	SAFE_CLOSE(fd);
> +}
> +
> +static void run(void)
> +{
> +	io_uring_setup_queue(&s, QUEUE_DEPTH);
> +	test_write_read();
> +	test_partial_io();
> +	io_uring_cleanup_queue(&s, QUEUE_DEPTH);
> +}
> +
> +static void setup(void)
> +{
> +	io_uring_setup_supported_by_kernel();
> +	sigemptyset(&sig);
> +	memset(&s, 0, sizeof(s));
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.needs_tmpdir = 1,
> +	.save_restore = (const struct tst_path_val[]) {
> +		{"/proc/sys/kernel/io_uring_disabled", "0",
> +			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
> +		{}
> +	},
> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "5d17b4a4b48c"},

This commit does not appear to exist in linux kernel git tree.

> +		{"linux-git", "2b188cc1bb85"},

And this is a commit that added io_uring. Commit hashes in tests are
used only for cases where the test is a regression test for some bug and
point to a commit that fixed the problem.

> +		{}
> +	}
> +};
> diff --git a/testcases/kernel/syscalls/io_uring/io_uring_common.h b/testcases/kernel/syscalls/io_uring/io_uring_common.h

Since you are pulling the common code into header it would be nice to
convert the io_uring01.c so that it uses the common header as well.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp