[LTP] [PATCH 1/3] io_uring: Test IORING READ and WRITE operations

[Sachin Sant]

>> +
>> +static char write_buf[BLOCK_SZ];
>> +static char read_buf[BLOCK_SZ];
> Can we please allocate these as a guarded buffers?
>
> https://linux-test-project.readthedocs.io/en/latest/developers/api_c_tests.html#guarded-buffers
Sure, will modify accordingly.
>> +
>> +static void run(void)
>> +{
>> +	io_uring_setup_queue(&s, QUEUE_DEPTH);
>> +	test_write_read();
>> +	test_partial_io();
>> +	io_uring_cleanup_queue(&s, QUEUE_DEPTH);
> I suppose that we need to setup and cleanup the queue only once in test
> setup() and cleanup() functions (when the test is executed with -i 2 on
> command line).
Good point. Will change it accordingly.

> +static inline void io_uring_do_io_op(struct io_uring_submit *s, int fd,
> +				     int op, void *buf, size_t len,
> +				     off_t offset, sigset_t *sig,
> +				     const char *msg)
> +{
> +	io_uring_submit_sqe(s, fd, op, buf, len, offset);
> +
> +	if (io_uring_wait_cqe(s, len, op, sig) == 0)
> +		tst_res(TPASS, "%s", msg);
>
> Rather than passing the description passed from the caller I would print
> the parameters passed to the function, something as:
>
> 		tst_res(TPASS, "OP=%2x fd=%i buf=%p len=%zu offset=%jd",
> 		        op, fd, buf, len, (intmax_t)offset);
>
> And we can add a function to map the OP to the enum name if we want to
> have fancy messages:
>
> static const char *ioring_op_name(int op)
> {
> 	switch (op) {
> 	case IORING_READ:
> 		return "IORING_READ";
> 	...
> 	defaut:
> 		return "UNKNOWN"
> }
Okay. Will include the changes in next version.

> Then we can print the OP as:
>
> tst_res(TPASS, "OP=%s (%2x) ...", ioring_op_name(op), op, ...);
>
> With this approach we will avoid copy&paste mistakes, it's way too easy
> for messages and parameters written manually to get out of sync.
>
> Also we should either propagate the failure to the test by returning
> non-zero from this function if waiting for completion failed (so that
> the test can abort) or call tst_brk(TBROK, ...) in the
> io_uring_wait_cqe() which will abort the test when something fails
> automatically.
Yes, will make the required changes to address this.

Thanks again for the review comments.

-- 
Thanks
- Sachin