Re: [PATCH v2 liburing] Test consistent file position updates

[Jens Axboe <axboe@kernel.dk>]

On 2/21/22 7:18 AM, Dylan Yudaken wrote:
> read(2)/write(2) and friends support sequential reads without giving an
> explicit offset. The result of these should leave the file with an
> incremented offset.
> 
> Add tests for both read and write to check that io_uring behaves
> consistently in these scenarios. Expect that if you queue many
> reads/writes, and set the IOSQE_IO_LINK flag, that they will behave
> similarly to calling read(2)/write(2) in sequence.
> 
> Set IOSQE_ASYNC as well in a set of tests. This exacerbates the problem by
> forcing work to happen in different threads to submission.
> 
> Also add tests for not setting IOSQE_IO_LINK, but allow the file offset to
> progress past the end of the file.

A few style and test output comments below.

> diff --git a/test/fpos.c b/test/fpos.c
> new file mode 100644
> index 0000000..2c6c139
> --- /dev/null
> +++ b/test/fpos.c
> @@ -0,0 +1,270 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Description: test io_uring fpos handling
> + *
> + */
> +#include <errno.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <assert.h>
> +
> +#include "helpers.h"
> +#include "liburing.h"
> +
> +#define FILE_SIZE 10000
> +#define QUEUE_SIZE 4096

Seems huge? 

> +static int
> +test_read(struct io_uring *ring, bool async, bool link, int blocksize)

liburing and the kernel tend to use:

static int test_read(struct io_uring *ring, bool async, bool link,
		     int blocksize)
{
}

if we go over 80 chars, with the reasoning being that a grep will show
you the return type at least.

> +{
> +	int ret, fd, i;
> +	bool done = false;
> +	struct io_uring_sqe *sqe;
> +	struct io_uring_cqe *cqe;
> +	loff_t current, expected = 0;
> +	int count_ok;
> +	int count_0 = 0, count_1 = 0;
> +	unsigned char buff[QUEUE_SIZE * blocksize];
> +	unsigned char reordered[QUEUE_SIZE * blocksize];
> +
> +	create_file(".test_read", FILE_SIZE);
> +	fd = open(".test_read", O_RDONLY);
> +	unlink(".test_read");
> +	assert(fd >= 0);
> +
> +	while (!done) {
> +		for (i = 0; i < QUEUE_SIZE; ++i) {
> +			sqe = io_uring_get_sqe(ring);
> +			if (!sqe) {
> +				fprintf(stderr, "no sqe\n");
> +				return -1;
> +			}
> +			io_uring_prep_read(sqe, fd,
> +					buff + i * blocksize,
> +					blocksize, -1);
> +			sqe->user_data = i;
> +			if (async)
> +				sqe->flags |= IOSQE_ASYNC;
> +			if (link && i != QUEUE_SIZE - 1)
> +				sqe->flags |= IOSQE_IO_LINK;
> +		}
> +		ret = io_uring_submit_and_wait(ring, QUEUE_SIZE);
> +		if (ret != QUEUE_SIZE) {
> +			fprintf(stderr, "submit failed: %d\n", ret);
> +			return 1;
> +		}
> +		count_ok  = 0;
> +		for (i = 0; i < QUEUE_SIZE; ++i) {
> +			int res;
> +
> +			ret = io_uring_peek_cqe(ring, &cqe);
> +			if (ret) {
> +				fprintf(stderr, "peek failed: %d\n", ret);
> +				return ret;
> +			}
> +			assert(cqe->user_data < QUEUE_SIZE);
> +			memcpy(reordered + count_ok
> +				, buff + cqe->user_data * blocksize
> +				, blocksize);

			memcpy(reordered + count_ok,
				buff + cqe->user_data * blocksize, blocksize);

> +static int
> +test_write(struct io_uring *ring, bool async, bool link, int blocksize)
> +{
> +	int ret, fd, i;
> +	struct io_uring_sqe *sqe;
> +	struct io_uring_cqe *cqe;
> +	bool fail = false;
> +	loff_t current;
> +	char data[blocksize+1];
> +	char readbuff[QUEUE_SIZE*blocksize+1];
> +
> +	fd = open(".test_write", O_RDWR | O_CREAT, 0644);
> +	unlink(".test_write");
> +	assert(fd >= 0);
> +
> +	for(i = 0; i < blocksize; i++) {
> +		data[i] = 'A' + i;
> +	}
> +	data[blocksize] = '\0';
> +
> +	for (i = 0; i < QUEUE_SIZE; ++i) {
> +		sqe = io_uring_get_sqe(ring);
> +		if (!sqe) {
> +			fprintf(stderr, "no sqe\n");
> +			return -1;
> +		}
> +		io_uring_prep_write(sqe, fd, data + (i % blocksize), 1, -1);
> +		sqe->user_data = 1;
> +		if (async)
> +			sqe->flags |= IOSQE_ASYNC;
> +		if (link && i != QUEUE_SIZE - 1)
> +			sqe->flags |= IOSQE_IO_LINK;
> +	}
> +	ret = io_uring_submit_and_wait(ring, QUEUE_SIZE);
> +	if (ret != QUEUE_SIZE) {
> +		fprintf(stderr, "submit failed: %d\n", ret);
> +		return 1;
> +	}
> +	for (i = 0; i < QUEUE_SIZE; ++i) {
> +		int res;
> +
> +		ret = io_uring_peek_cqe(ring, &cqe);
> +		res = cqe->res;
> +		if (ret) {
> +			fprintf(stderr, "peek failed: %d\n", ret);
> +			return ret;
> +		}
> +		io_uring_cqe_seen(ring, cqe);
> +		if (!fail && res != 1) {
> +			fprintf(stderr, "bad result %d\n", res);
> +			fail = true;
> +		}
> +	}
> +	current = lseek(fd, 0, SEEK_CUR);
> +	if (current != QUEUE_SIZE) {
> +		fprintf(stderr,
> +			"f_pos incorrect, expected %ld have %d\n",
> +			current,
> +			QUEUE_SIZE);
> +		fail = true;

		fprintf(stderr, "f_pos incorrect, expected %ld have %d\n",
				current, QUEUE_SIZE);

> +int main(int argc, char *argv[])
> +{
> +	struct io_uring ring;
> +	int ret;
> +	int failed = 0;
> +	int blocksizes[] = {1, 8, 15, 0};
> +
> +	if (argc > 1)
> +		return 0;
> +
> +	ret = io_uring_queue_init(QUEUE_SIZE, &ring, 0);
> +	if (ret) {
> +		fprintf(stderr, "ring setup failed\n");
> +		return 1;
> +	}
> +
> +	for (int *blocksize = blocksizes; *blocksize; blocksize++) {
> +	for (int async = 0; async < 2; async++) {
> +	for (int link = 0; link < 2; link++) {
> +	for (int write = 0; write < 2; write++) {
> +		fprintf(stderr, "*********\n");
> +		ret = write
> +			? test_write(&ring, !!async, !!link, *blocksize)
> +			: test_read(&ring, !!async, !!link, *blocksize);
> +		fprintf(stderr, "test %s async=%d link=%d blocksize=%d:\t%s\n",
> +			write ? "write":"read",
> +			async, link,
> +			*blocksize,
> +			ret ? "failed" : "passed");

The normal procedure for tests and printing output is:

- Be silent on success, otherwise it's just noise to look over when
  doing runs. There are some ancient tests that don't honor this, but
  generally the rest of them do.

- If one test fails, stop further tests.

> +	}
> +	}
> +	}

Indentation is wonky here.

-- 
Jens Axboe