Xuan Zhuo
2023-Mar-02 11:59 UTC
[PATCH vhost v1 05/12] virtio_ring: packed: refactor virtqueue_add_packed() for premapped
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 at linux.alibaba.com>
---
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);
+ vq->packed.desc_state[id].indir_desc = ctx;
+ }
+
+ vq->packed.desc_state[id].data = data;
+
+ goto end;
+
+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
Jason Wang
2023-Mar-07 06:43 UTC
[PATCH vhost v1 05/12] virtio_ring: packed: refactor virtqueue_add_packed() for premapped
On Thu, Mar 2, 2023 at 7:59?PM Xuan Zhuo <xuanzhuo at linux.alibaba.com> 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 at linux.alibaba.com> > --- > 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 >