Re: [PATCH 1/2] io_uring: add support for truncate

[Jens Axboe <axboe@kernel.dk>]

On 1/22/24 12:37 PM, Tony Solomonik wrote:
> Libraries that are built on io_uring currently need to maintain a
> separate thread pool implementation when they want to truncate a file.

In general, I think we should just make this one opcode, and then
require fd to be -1 for the truncate case (with path in addr, like you
have, or have a valid fd and NULL addr for the fruncate case. Then at
least we don't waste two opcodes on essentially the same functionality.
One minor code comment, which applies to both patches:

> +int io_truncate_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> +	struct io_trunc *tr = io_kiocb_to_cmd(req, struct io_trunc);
> +
> +	if (sqe->off || sqe->rw_flags || sqe->buf_index || sqe->splice_fd_in)
> +		return -EINVAL;
> +	if (unlikely(req->flags & REQ_F_FIXED_FILE))
> +		return -EBADF;
> +
> +	tr->pathname = u64_to_user_ptr(READ_ONCE(sqe->addr));
> +	tr->len = READ_ONCE(sqe->len);

This should use offset rather than len, as Breno pointed out.

> +	req->flags |= REQ_F_NEED_CLEANUP;

Why is this being set? There's nothing to clean up post completion, so
this just slows req completion down for no particularly reason. And the
you can kill the same flag force clear in io_truncate as well.

-- 
Jens Axboe