Re: [RESEND PATCH 2/5] loop: cleanup lo_rw_aio()

[Christoph Hellwig <hch@infradead.org>]

On Sun, Mar 09, 2025 at 12:23:06AM +0800, Ming Lei wrote:
> +	if (rq->bio != rq->biotail) {

This would probably be more self-explaining by checking for cmd->bdev
here.

> +		bvec = cmd->bvec;
> +		offset = 0;
> +	} else {
> +		struct bio *bio = rq->bio;
> +
> +		offset = bio->bi_iter.bi_bvec_done;
> +		bvec = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
> +	}
> +	iov_iter_bvec(&iter, dir, bvec, nr_bvec, blk_rq_bytes(rq));
> +	iter.iov_offset = offset;

And given that bvec and offset are only used here I'd just move the
iov_iter_bvec into the branches and do away with the two variables,
and kill the bio variable as well while at it.

> +static inline unsigned lo_cmd_nr_bvec(struct loop_cmd *cmd)
> +{
> +	struct req_iterator rq_iter;
> +	struct request *rq = blk_mq_rq_from_pdu(cmd);
> +	struct bio_vec tmp;
> +	int nr_bvec = 0;
> +
>  	rq_for_each_bvec(tmp, rq, rq_iter)
>  		nr_bvec++;
>  
> +	return nr_bvec;
> +}
> +
> +static int lo_rw_aio_prep(struct loop_device *lo, struct loop_cmd *cmd,
> +			  unsigned nr_bvec)

The function order is a bit weird.  I would expect them to appear in
the rough order that they are called, e.g. lo_cmd_nr_bvec first, then
lo_rw_aio_prep, then the submit helper.

> -		/*
> -		 * Same here, this bio may be started from the middle of the
> -		 * 'bvec' because of bio splitting, so offset from the bvec
> -		 * must be passed to iov iterator
> -		 */

It would be good if this comment didn't get lost.