On Thu, Mar 2, 2023 at 7:59 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
>
> This commit splits virtqueue_add_packed() to two functions. The purpose
> of such splitting is to separate DMA operations.
>
> The first function includes all codes that may fail before the DMA
> operation. The subsequent part is used as the second function.
>
> In this way, we can perform DMA operations in the middle of the two
> functions. If the first function fails, we do not need to perform DMA
> operations. If it is premapped, we can pass the DMA operation.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> ---
> drivers/virtio/virtio_ring.c | 120 +++++++++++++++++++++++------------
> 1 file changed, 81 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 178edf1171e2..6796cbee0207 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1347,7 +1347,6 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
> unsigned int total_sg,
> unsigned int out_sgs,
> unsigned int in_sgs,
> - void *data,
> struct vring_packed_desc *desc)
> {
> struct scatterlist *sg;
> @@ -1422,14 +1421,12 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
>
> /* Store token and indirect buffer state. */
> vq->packed.desc_state[id].num = 1;
> - vq->packed.desc_state[id].data = data;
> vq->packed.desc_state[id].indir_desc = desc;
> vq->packed.desc_state[id].last = id;
>
> vq->num_added += 1;
>
> pr_debug("Added buffer head %i to %p\n", head, vq);
> - END_USE(vq);
>
> return 0;
>
> @@ -1441,74 +1438,76 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
>
> kfree(desc);
>
> - END_USE(vq);
> return -ENOMEM;
> }
>
> -static inline int virtqueue_add_packed(struct virtqueue *_vq,
> - struct scatterlist *sgs[],
> - unsigned int total_sg,
> - unsigned int out_sgs,
> - unsigned int in_sgs,
> - void *data,
> - void *ctx,
> - gfp_t gfp)
> +static inline struct vring_packed_desc *virtqueue_get_desc_packed(struct vring_virtqueue *vq,
> + unsigned int total_sg,
> + void *data,
> + void *ctx,
> + gfp_t gfp)
> {
> - struct vring_virtqueue *vq = to_vvq(_vq);
> struct vring_packed_desc *desc;
> - struct scatterlist *sg;
> - unsigned int i, n, c, descs_used, err_idx;
> - __le16 head_flags, flags;
> - u16 head, id, prev, curr, avail_used_flags;
> - int err;
> -
> - START_USE(vq);
> + unsigned int descs_used;
>
> BUG_ON(data == NULL);
> BUG_ON(ctx && vq->indirect);
>
> - if (unlikely(vq->broken)) {
> - END_USE(vq);
> - return -EIO;
> - }
> + if (unlikely(vq->broken))
> + return ERR_PTR(-EIO);
>
> LAST_ADD_TIME_UPDATE(vq);
>
> BUG_ON(total_sg == 0);
>
> + desc = NULL;
> +
> if (virtqueue_use_indirect(vq, total_sg)) {
> desc = alloc_indirect_packed(total_sg, gfp);
> if (desc) {
> if (unlikely(vq->vq.num_free < 1)) {
> pr_debug("Can't add buf len 1 - avail = 0\n");
> kfree(desc);
> - END_USE(vq);
> - return -ENOSPC;
> + return ERR_PTR(-ENOSPC);
> }
>
> - return virtqueue_add_indirect_packed(vq, sgs, total_sg, out_sgs,
> - in_sgs, data, desc);
> + return NULL;
> }
>
> /* fall back on direct */
> }
>
> - head = vq->packed.next_avail_idx;
> - avail_used_flags = vq->packed.avail_used_flags;
> -
> WARN_ON_ONCE(total_sg > vq->packed.vring.num && !vq->indirect);
>
> - desc = vq->packed.vring.desc;
> - i = head;
> descs_used = total_sg;
>
> if (unlikely(vq->vq.num_free < descs_used)) {
> pr_debug("Can't add buf len %i - avail = %i\n",
> descs_used, vq->vq.num_free);
> - END_USE(vq);
> - return -ENOSPC;
> + return ERR_PTR(-ENOSPC);
> }
>
> + return desc;
> +}
> +
> +static inline int virtqueue_add_vring_packed(struct vring_virtqueue *vq,
> + struct scatterlist *sgs[],
> + unsigned int total_sg,
> + unsigned int out_sgs,
> + unsigned int in_sgs)
> +{
> + struct vring_packed_desc *desc;
> + struct scatterlist *sg;
> + unsigned int i, n, c, descs_used, err_idx;
> + __le16 head_flags, flags;
> + u16 head, id, prev, curr, avail_used_flags;
> +
> + desc = vq->packed.vring.desc;
> + head = vq->packed.next_avail_idx;
> + i = head;
> + descs_used = total_sg;
> + avail_used_flags = vq->packed.avail_used_flags;
> +
> id = vq->free_head;
> BUG_ON(id == vq->packed.vring.num);
>
> @@ -1563,8 +1562,6 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
>
> /* Store token. */
> vq->packed.desc_state[id].num = descs_used;
> - vq->packed.desc_state[id].data = data;
> - vq->packed.desc_state[id].indir_desc = ctx;
> vq->packed.desc_state[id].last = prev;
>
> /*
> @@ -1577,7 +1574,6 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
> vq->num_added += descs_used;
>
> pr_debug("Added buffer head %i to %p\n", head, vq);
> - END_USE(vq);
>
> return 0;
>
> @@ -1598,10 +1594,56 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
> i = 0;
> }
>
> - END_USE(vq);
> return -EIO;
> }
>
> +static inline int virtqueue_add_packed(struct virtqueue *_vq,
> + struct scatterlist *sgs[],
> + unsigned int total_sg,
> + unsigned int out_sgs,
> + unsigned int in_sgs,
> + void *data,
> + void *ctx,
> + gfp_t gfp)
> +{
> +
> + struct vring_virtqueue *vq = to_vvq(_vq);
> + struct vring_packed_desc *desc;
> + u16 id;
> + int err;
> +
> + START_USE(vq);
> +
> + /* check vq state and try to alloc desc for indirect. */
> + desc = virtqueue_get_desc_packed(vq, total_sg, data, ctx, gfp);
> + if (IS_ERR(desc)) {
> + err = PTR_ERR(desc);
> + goto end;
> + }
> +
> + id = vq->free_head;
> +
> + if (desc) {
> + err = virtqueue_add_indirect_packed(vq, sgs, total_sg, out_sgs, in_sgs, desc);
> + if (err)
> + goto err;
> + } else {
> + virtqueue_add_vring_packed(vq, sgs, total_sg, out_sgs, in_sgs);
While at it, can we unify the logic of virtqueue_add_indirect_packed()
and virtqueue_add_vring_packed()?
> + vq->packed.desc_state[id].indir_desc = ctx;
> + }
> +
> + vq->packed.desc_state[id].data = data;
> +
> + goto end;
Similar to split, I'd rather duplicate the END_USE() and return.
Thanks
> +
> +err:
> + kfree(desc);
> +
> +end:
> + END_USE(vq);
> + return err;
> +}
> +
> static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
> {
> struct vring_virtqueue *vq = to_vvq(_vq);
> --
> 2.32.0.3.g01195cf9f
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
