[PATCH v4 07/11] io_uring/rw: add support to send meta along with read/write
Christoph Hellwig
hch at lst.de
Thu Oct 17 01:10:57 PDT 2024
s/meta/metadata/ in the subject.
> + const struct io_uring_meta *md = (struct io_uring_meta *)sqe->big_sqe_cmd;
Overly long line.
> + if (!meta_type)
> + return 0;
> + if (!(meta_type & META_TYPE_INTEGRITY))
> + return -EINVAL;
What is the meta_type for? To distintinguish PI from non-PI metadata?
Why doesn't this support non-PI metadata? Also PI or TO_PI might be
a better name than the rather generic integrity. (but I'll defer to
Martin if he has any good arguments for naming here).
> static bool need_complete_io(struct io_kiocb *req)
> {
> + struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
> +
> + /* Exclude meta IO as we don't support partial completion for that */
> return req->flags & REQ_F_ISREG ||
> - S_ISBLK(file_inode(req->file)->i_mode);
> + S_ISBLK(file_inode(req->file)->i_mode) ||
> + !(rw->kiocb.ki_flags & IOCB_HAS_METADATA);
> }
What partial ocmpletions aren't supported? Note that this would
trigger easily as right now metadata is only added for block devices
anyway.
> + if (unlikely(kiocb->ki_flags & IOCB_HAS_METADATA)) {
For a workload using metadata this is everything but unlikely. Is
there a specific reason you're trying to override the existing
branch predictor here (although on at least x86_64 gcc these kinds
of unlikely calls tend to be no-ops anyway).
> + struct io_async_rw *io = req->async_data;
> +
> + if (!(req->file->f_flags & O_DIRECT))
> + return -EOPNOTSUPP;
I guess you are forcing this here rather in the file oerations instance
because the union of the field in struct io_async_rw. Please add comment
about that.
> + /* wpq is for buffered io, while meta fields are used with direct io*/
missing whitespace before the closing of the comment.
More information about the Linux-nvme
mailing list