Re: [LTP] [PATCH v2 1/4] io_uring: Redesign common helpers to follow liburing API conventions

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi Sebastian,

> +/*
> + * Generic SQE fill for read/write family operations.
> + * Does not touch user_data - caller sets it via io_uring_sqe_set_data64().
> + */
> +static inline void io_uring_prep_rw(struct io_uring_sqe *sqe, int opcode,
> +				    int fd, const void *addr, unsigned int len,
> +				    off_t offset)
> +{
> +	sqe->opcode = opcode;
> +	sqe->fd = fd;
> +	sqe->addr = (unsigned long)addr;
> +	sqe->len = len;
> +	sqe->off = offset;
> +}
> +
> +static inline void io_uring_prep_read(struct io_uring_sqe *sqe, int fd,
> +				      void *buf, size_t len, off_t offset)
> +{
> +	io_uring_prep_rw(sqe, IORING_OP_READ, fd, buf, len, offset);
> +}
> +
> +static inline void io_uring_prep_write(struct io_uring_sqe *sqe, int fd,
> +				       const void *buf, size_t len, off_t offset)
> +{
> +	io_uring_prep_rw(sqe, IORING_OP_WRITE, fd, buf, len, offset);
> +}
> +
> +static inline void io_uring_prep_readv(struct io_uring_sqe *sqe, int fd,
> +				       struct iovec *iovs, int nr_vecs,
> +				       off_t offset)
> +{
> +	io_uring_prep_rw(sqe, IORING_OP_READV, fd, iovs, nr_vecs, offset);
> +}
> +
> +static inline void io_uring_prep_writev(struct io_uring_sqe *sqe, int fd,
> +					struct iovec *iovs, int nr_vecs,
> +					off_t offset)
> +{
> +	io_uring_prep_rw(sqe, IORING_OP_WRITEV, fd, iovs, nr_vecs, offset);
> +}

I'm not an expert of liburing or io_uring in general, but aren't all
these functions quite redundant? They mostly do what we could simply
do inside test with 1 call and 1 argument difference.

> +
> +/*
> + * Set the user_data field of an SQE. user_data is returned verbatim in the
> + * corresponding CQE and must be unique per in-flight request to allow correct
> + * correlation of completions.
> + */
> +static inline void io_uring_sqe_set_data64(struct io_uring_sqe *sqe,
> +					   uint64_t data)
> +{
> +	sqe->user_data = data;
> +}

Uh? :-) same comment.

As I said, I'm not an expert on this technology, but I see that we are
overengineering something that is so simple as a 1-argument-difference
or 1 pointer assignment.

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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