On Fri, Jun 30, 2023 at 04:25:16PM +0100, David Howells wrote:
> A number of places that generate kiocbs didn't use init_sync_kiocb() to
> initialise the new kiocb. Fix these to always use init_kiocb().
>
> Note that aio and io_uring pass information in through ki_filp through an
> overlaid union before I can call init_kiocb(), so that gets reinitialised.
> I don't think it clobbers anything else.
>
> After this point, IOCB_WRITE is only set by init_kiocb().
Nothing in this patch touches the VFS, so the subject line is
wrong. And I think we're better off splitting it into one per
subsystem, which also allows documenting the exact changes.
Which includes now setting the flags from f_iocb_flags and setting
and I/O priority. Please explain why this is harmless or even useful.
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 37511d2b2caf..ea92235c5ba2 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -439,16 +439,17 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
> }
> atomic_set(&cmd->ref, 2);
>
> - iov_iter_bvec(&iter, rw, bvec, nr_bvec, blk_rq_bytes(rq));
> + iov_iter_bvec(&iter, rw == WRITE ? ITER_SOURCE : ITER_DEST,
> + bvec, nr_bvec, blk_rq_bytes(rq));
Given the cover letter I expect this is going to go away, but the
changes would probably a lot more readable if you had a helper
to convert from READ/WRITE to the iter flags inbetween.
Or maybe do it the other way - add a helper to init the
> @@ -490,12 +491,12 @@ static int do_req_filebacked(struct loop_device *lo, struct request *rq)
> return lo_fallocate(lo, rq, pos, FALLOC_FL_PUNCH_HOLE);
> case REQ_OP_WRITE:
> if (cmd->use_aio)
> - return lo_rw_aio(lo, cmd, pos, ITER_SOURCE);
> + return lo_rw_aio(lo, cmd, pos, WRITE);
> else
> return lo_write_simple(lo, rq, pos);
> case REQ_OP_READ:
> if (cmd->use_aio)
> - return lo_rw_aio(lo, cmd, pos, ITER_DEST);
> + return lo_rw_aio(lo, cmd, pos, READ);
I don't think there is any need to pass the rw argument at all,
lo_rw_aio can just do an op_is_write(req_op(rq))
> -static int io_rw_init_file(struct io_kiocb *req, fmode_t mode)
> +static int io_rw_init_file(struct io_kiocb *req, unsigned int io_direction)
> {
> struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
> struct kiocb *kiocb = &rw->kiocb;
> struct io_ring_ctx *ctx = req->ctx;
> struct file *file = req->file;
> + fmode_t mode = (io_direction == WRITE) ? FMODE_WRITE : FMODE_READ;
> int ret;
>
> if (unlikely(!file || !(file->f_mode & mode)))
I'd just move this check into the two callers, that way you can hard
code the mode instead of adding a conversion.
