Re: Deduplicate io_*_prep calls?

[Jens Axboe <axboe@kernel.dk>]

On 2/23/20 6:07 PM, Andres Freund wrote:
> Hi,
> 
> There's currently two places that know to call io_*_prep() for
> sqes. io_req_defer_prep() and io_issue_sqe(). E.g. for READV there's:
> 
> static int io_req_defer_prep(struct io_kiocb *req,
> 			     const struct io_uring_sqe *sqe)
> ...
> 	case IORING_OP_READV:
> 	case IORING_OP_READ_FIXED:
> 	case IORING_OP_READ:
> 		ret = io_read_prep(req, sqe, true);
> 		break;
> 
> and
> 
> static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe,
> 			struct io_kiocb **nxt, bool force_nonblock)
> {
> ...
> 	case IORING_OP_READV:
> 	case IORING_OP_READ_FIXED:
> 	case IORING_OP_READ:
> 		if (sqe) {
> 			ret = io_read_prep(req, sqe, force_nonblock);
> 			if (ret < 0)
> 				break;
> 		}
> 		ret = io_read(req, nxt, force_nonblock);
> 		break;
> 
> that seems a bit unnecessary. How about breaking that out into a
> separate function?  I can write up a patch, just didn't want to do so if
> there's a reason for the current split.
> 
> 
> Alternatively it'd could all be just be dispatches via io_op_defs, but
> that'd be a bigger change with potential performance implications. And
> it'd benefit from prior deduplication anyway.

The reason for the split is that if we defer a request, it has to be
prepared up front. If the request has been deferred, then the
io_issue_sqe() invocation has sqe == NULL. Hence we only run the prep
handler once, and read the sqe just once.

This could of course be compacted with some indirect function calls, but
I didn't want to pay the overhead of doing so... The downside is that
the code is a bit bigger.

-- 
Jens Axboe