Re: [PATCH v1 2/4] RDMA/core: use IOVA-based DMA mapping for bvec RDMA operations

[Christoph Hellwig <hch@lst.de>]

> +	/* Calculate total transfer length */
> +	for (i = 0; i < nr_bvec; i++) {
> +		size_t len = (i == 0 && offset) ?
> +			     bvec[i].bv_len - offset : bvec[i].bv_len;
> +
> +		if (check_add_overflow(total_len, len, &total_len))
> +			return -EINVAL;
> +	}

The caller should usually have that value, so maybe pass it in?
(also using a bvec_iter would fix that)

> +	ctx->nr_ops = DIV_ROUND_UP(nr_bvec, max_sge);

I don't think this part is correct, but see more below.

> +	/* Link all bvecs into the IOVA space */
> +	for (i = 0; i < nr_bvec; i++) {
> +		const struct bio_vec *bv = &bvec[i];
> +		phys_addr_t phys = bvec_phys(bv);
> +		size_t len = bv->bv_len;
> +
> +		if (i == 0 && offset) {
> +			phys += offset;
> +			len -= offset;
> +		}
> +
> +		ret = dma_iova_link(dma_dev, &ctx->iova.state, phys,
> +				    mapped_len, len, dir, 0);
> +		if (ret)
> +			goto out_destroy;
> +
> +		mapped_len += len;

This creates a single contiguous IOVA for all the passed in bvecs,
even if they had non-contiguous host physical addresses.

> +	/* Build SGEs using offsets into the contiguous IOVA range */
> +	mapped_len = 0;
> +	for (i = 0; i < ctx->nr_ops; i++) {
> +		struct ib_rdma_wr *rdma_wr = &ctx->iova.wrs[i];
> +		u32 nr_sge = min(nr_bvec - bvec_idx, max_sge);
> +
> +		if (dir == DMA_TO_DEVICE)
> +			rdma_wr->wr.opcode = IB_WR_RDMA_WRITE;
> +		else
> +			rdma_wr->wr.opcode = IB_WR_RDMA_READ;
> +		rdma_wr->remote_addr = remote_addr + mapped_len;
> +		rdma_wr->rkey = rkey;
> +		rdma_wr->wr.num_sge = nr_sge;
> +		rdma_wr->wr.sg_list = sge;

... which means that here you just need a single WR and SGE to register
all of them.  No need to split the IOVA space up again.