Xuan Zhuo
2023-Feb-14 07:26 UTC
[PATCH vhost 01/10] virtio_ring: split: refactor virtqueue_add_split() for premapped
DMA-related logic is separated from the virtqueue_add_split to prepare
for subsequent support for premapped.
Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com>
---
drivers/virtio/virtio_ring.c | 219 ++++++++++++++++++++++++-----------
1 file changed, 152 insertions(+), 67 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 41144b5246a8..560ee30d942c 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -520,29 +520,83 @@ static inline unsigned int virtqueue_add_desc_split(struct
virtqueue *vq,
return next;
}
-static inline int virtqueue_add_split(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 int virtqueue_map_sgs(struct vring_virtqueue *vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs)
{
- struct vring_virtqueue *vq = to_vvq(_vq);
struct scatterlist *sg;
- struct vring_desc *desc;
- unsigned int i, n, avail, descs_used, prev, err_idx;
- int head;
- bool indirect;
+ unsigned int n;
- START_USE(vq);
+ for (n = 0; n < out_sgs; n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_TO_DEVICE);
+
+ if (vring_mapping_error(vq, addr))
+ return -ENOMEM;
+
+ sg->dma_address = addr;
+ }
+ }
+ for (; n < (out_sgs + in_sgs); n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_FROM_DEVICE);
+
+ if (vring_mapping_error(vq, addr))
+ return -ENOMEM;
+
+ sg->dma_address = addr;
+ }
+ }
+
+ return 0;
+}
+
+static void virtqueue_unmap_sgs(struct vring_virtqueue *vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs)
+{
+ struct scatterlist *sg;
+ unsigned int n;
+
+ for (n = 0; n < out_sgs; n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ if (!sg->dma_address)
+ return;
+
+ dma_unmap_single(vring_dma_dev(vq), sg->dma_address,
+ sg->length, DMA_TO_DEVICE);
+ }
+ }
+ for (; n < (out_sgs + in_sgs); n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ if (!sg->dma_address)
+ return;
+
+ dma_unmap_single(vring_dma_dev(vq), sg->dma_address,
+ sg->length, DMA_FROM_DEVICE);
+ }
+ }
+}
+
+static inline int virtqueue_add_split_prepare(struct vring_virtqueue *vq,
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ void *data,
+ void *ctx,
+ gfp_t gfp,
+ struct vring_desc **pdesc)
+{
+ struct vring_desc *desc;
+ unsigned int descs_used;
BUG_ON(data == NULL);
BUG_ON(ctx && vq->indirect);
if (unlikely(vq->broken)) {
- END_USE(vq);
return -EIO;
}
@@ -550,27 +604,17 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
BUG_ON(total_sg == 0);
- head = vq->free_head;
-
if (virtqueue_use_indirect(vq, total_sg))
- desc = alloc_indirect_split(_vq, total_sg, gfp);
+ desc = alloc_indirect_split(&vq->vq, total_sg, gfp);
else {
desc = NULL;
WARN_ON_ONCE(total_sg > vq->split.vring.num &&
!vq->indirect);
}
- if (desc) {
- /* Use a single buffer which doesn't continue */
- indirect = true;
- /* Set up rest to use this indirect table. */
- i = 0;
+ if (desc)
descs_used = 1;
- } else {
- indirect = false;
- desc = vq->split.vring.desc;
- i = head;
+ else
descs_used = total_sg;
- }
if (unlikely(vq->vq.num_free < descs_used)) {
pr_debug("Can't add buf len %i - avail = %i\n",
@@ -580,38 +624,64 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
* host should service the ring ASAP. */
if (out_sgs)
vq->notify(&vq->vq);
- if (indirect)
- kfree(desc);
- END_USE(vq);
+ kfree(desc);
return -ENOSPC;
}
+ *pdesc = desc;
+
+ return 0;
+}
+
+static inline int virtqueue_add_split_vring(struct vring_virtqueue *vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ struct vring_desc *desc)
+{
+ unsigned int n, i, avail, descs_used, prev;
+ struct virtqueue *_vq = &vq->vq;
+ struct scatterlist *sg;
+ bool indirect;
+ int head;
+
+ head = vq->free_head;
+
+ if (desc) {
+ /* Use a single buffer which doesn't continue */
+ indirect = true;
+ /* Set up rest to use this indirect table. */
+ i = 0;
+ descs_used = 1;
+ } else {
+ indirect = false;
+ desc = vq->split.vring.desc;
+ i = head;
+ descs_used = total_sg;
+ }
+
for (n = 0; n < out_sgs; n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
- dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_TO_DEVICE);
- if (vring_mapping_error(vq, addr))
- goto unmap_release;
-
prev = i;
/* Note that we trust indirect descriptor
* table since it use stream DMA mapping.
*/
- i = virtqueue_add_desc_split(_vq, desc, i, addr, sg->length,
+ i = virtqueue_add_desc_split(_vq, desc, i,
+ sg->dma_address,
+ sg->length,
VRING_DESC_F_NEXT,
indirect);
}
}
for (; n < (out_sgs + in_sgs); n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
- dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_FROM_DEVICE);
- if (vring_mapping_error(vq, addr))
- goto unmap_release;
-
prev = i;
/* Note that we trust indirect descriptor
* table since it use stream DMA mapping.
*/
- i = virtqueue_add_desc_split(_vq, desc, i, addr,
+ i = virtqueue_add_desc_split(_vq, desc, i,
+ sg->dma_address,
sg->length,
VRING_DESC_F_NEXT |
VRING_DESC_F_WRITE,
@@ -630,7 +700,7 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
vq, desc, total_sg * sizeof(struct vring_desc),
DMA_TO_DEVICE);
if (vring_mapping_error(vq, addr))
- goto unmap_release;
+ return -ENOMEM;
virtqueue_add_desc_split(_vq, vq->split.vring.desc,
head, addr,
@@ -648,13 +718,6 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
else
vq->free_head = i;
- /* Store token and indirect buffer state. */
- vq->split.desc_state[head].data = data;
- if (indirect)
- vq->split.desc_state[head].indir_desc = desc;
- else
- vq->split.desc_state[head].indir_desc = ctx;
-
/* Put entry in available array (but don't update avail->idx until they
* do sync). */
avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1);
@@ -677,30 +740,52 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
virtqueue_kick(_vq);
return 0;
+}
-unmap_release:
- err_idx = i;
+static inline int virtqueue_add_split(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_desc *desc;
+ int head;
+ int err;
- if (indirect)
- i = 0;
- else
- i = head;
+ START_USE(vq);
- for (n = 0; n < total_sg; n++) {
- if (i == err_idx)
- break;
- if (indirect) {
- vring_unmap_one_split_indirect(vq, &desc[i]);
- i = virtio16_to_cpu(_vq->vdev, desc[i].next);
- } else
- i = vring_unmap_one_split(vq, i);
- }
+ /* check vq state and try to alloc desc for indirect. */
+ err = virtqueue_add_split_prepare(vq, total_sg, out_sgs, data, ctx, gfp,
&desc);
+ if (err)
+ goto end;
- if (indirect)
- kfree(desc);
+ err = virtqueue_map_sgs(vq, sgs, total_sg, out_sgs, in_sgs);
+ if (err)
+ goto err;
+ head = vq->free_head;
+ err = virtqueue_add_split_vring(vq, sgs, total_sg, out_sgs, in_sgs, desc);
+ if (err)
+ goto err;
+
+ /* Store token and indirect buffer state. */
+ vq->split.desc_state[head].data = data;
+ vq->split.desc_state[head].indir_desc = desc ? desc : ctx;
+
+ goto end;
+
+err:
+ virtqueue_unmap_sgs(vq, sgs, total_sg, out_sgs, in_sgs);
+
+ kfree(desc);
+
+end:
END_USE(vq);
- return -ENOMEM;
+ return err;
}
static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
--
2.32.0.3.g01195cf9f
Jason Wang
2023-Feb-20 05:37 UTC
[PATCH vhost 01/10] virtio_ring: split: refactor virtqueue_add_split() for premapped
On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo <xuanzhuo at linux.alibaba.com> wrote:> > DMA-related logic is separated from the virtqueue_add_split to prepare > for subsequent support for premapped.The patch seems to do more than what is described here. To simplify reviewers, I'd suggest to split this patch into three: 1) virtqueue_add_split_prepare() (could we have a better name?) 2) virtqueue_map_sgs() 3) virtqueue_add_split_vring() (Or only factor DMA parts out, I haven't gone through the reset of the patches) Thanks> > Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com> > --- > drivers/virtio/virtio_ring.c | 219 ++++++++++++++++++++++++----------- > 1 file changed, 152 insertions(+), 67 deletions(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 41144b5246a8..560ee30d942c 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -520,29 +520,83 @@ static inline unsigned int virtqueue_add_desc_split(struct virtqueue *vq, > return next; > } > > -static inline int virtqueue_add_split(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 int virtqueue_map_sgs(struct vring_virtqueue *vq, > + struct scatterlist *sgs[], > + unsigned int total_sg, > + unsigned int out_sgs, > + unsigned int in_sgs) > { > - struct vring_virtqueue *vq = to_vvq(_vq); > struct scatterlist *sg; > - struct vring_desc *desc; > - unsigned int i, n, avail, descs_used, prev, err_idx; > - int head; > - bool indirect; > + unsigned int n; > > - START_USE(vq); > + for (n = 0; n < out_sgs; n++) { > + for (sg = sgs[n]; sg; sg = sg_next(sg)) { > + dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_TO_DEVICE); > + > + if (vring_mapping_error(vq, addr)) > + return -ENOMEM; > + > + sg->dma_address = addr; > + } > + } > + for (; n < (out_sgs + in_sgs); n++) { > + for (sg = sgs[n]; sg; sg = sg_next(sg)) { > + dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_FROM_DEVICE); > + > + if (vring_mapping_error(vq, addr)) > + return -ENOMEM; > + > + sg->dma_address = addr; > + } > + } > + > + return 0; > +} > + > +static void virtqueue_unmap_sgs(struct vring_virtqueue *vq, > + struct scatterlist *sgs[], > + unsigned int total_sg, > + unsigned int out_sgs, > + unsigned int in_sgs) > +{ > + struct scatterlist *sg; > + unsigned int n; > + > + for (n = 0; n < out_sgs; n++) { > + for (sg = sgs[n]; sg; sg = sg_next(sg)) { > + if (!sg->dma_address) > + return; > + > + dma_unmap_single(vring_dma_dev(vq), sg->dma_address, > + sg->length, DMA_TO_DEVICE); > + } > + } > + for (; n < (out_sgs + in_sgs); n++) { > + for (sg = sgs[n]; sg; sg = sg_next(sg)) { > + if (!sg->dma_address) > + return; > + > + dma_unmap_single(vring_dma_dev(vq), sg->dma_address, > + sg->length, DMA_FROM_DEVICE); > + } > + } > +} > + > +static inline int virtqueue_add_split_prepare(struct vring_virtqueue *vq, > + unsigned int total_sg, > + unsigned int out_sgs, > + void *data, > + void *ctx, > + gfp_t gfp, > + struct vring_desc **pdesc) > +{ > + struct vring_desc *desc; > + unsigned int descs_used; > > BUG_ON(data == NULL); > BUG_ON(ctx && vq->indirect); > > if (unlikely(vq->broken)) { > - END_USE(vq); > return -EIO; > } > > @@ -550,27 +604,17 @@ static inline int virtqueue_add_split(struct virtqueue *_vq, > > BUG_ON(total_sg == 0); > > - head = vq->free_head; > - > if (virtqueue_use_indirect(vq, total_sg)) > - desc = alloc_indirect_split(_vq, total_sg, gfp); > + desc = alloc_indirect_split(&vq->vq, total_sg, gfp); > else { > desc = NULL; > WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect); > } > > - if (desc) { > - /* Use a single buffer which doesn't continue */ > - indirect = true; > - /* Set up rest to use this indirect table. */ > - i = 0; > + if (desc) > descs_used = 1; > - } else { > - indirect = false; > - desc = vq->split.vring.desc; > - i = head; > + else > descs_used = total_sg; > - } > > if (unlikely(vq->vq.num_free < descs_used)) { > pr_debug("Can't add buf len %i - avail = %i\n", > @@ -580,38 +624,64 @@ static inline int virtqueue_add_split(struct virtqueue *_vq, > * host should service the ring ASAP. */ > if (out_sgs) > vq->notify(&vq->vq); > - if (indirect) > - kfree(desc); > - END_USE(vq); > + kfree(desc); > return -ENOSPC; > } > > + *pdesc = desc; > + > + return 0; > +} > + > +static inline int virtqueue_add_split_vring(struct vring_virtqueue *vq, > + struct scatterlist *sgs[], > + unsigned int total_sg, > + unsigned int out_sgs, > + unsigned int in_sgs, > + struct vring_desc *desc) > +{ > + unsigned int n, i, avail, descs_used, prev; > + struct virtqueue *_vq = &vq->vq; > + struct scatterlist *sg; > + bool indirect; > + int head; > + > + head = vq->free_head; > + > + if (desc) { > + /* Use a single buffer which doesn't continue */ > + indirect = true; > + /* Set up rest to use this indirect table. */ > + i = 0; > + descs_used = 1; > + } else { > + indirect = false; > + desc = vq->split.vring.desc; > + i = head; > + descs_used = total_sg; > + } > + > for (n = 0; n < out_sgs; n++) { > for (sg = sgs[n]; sg; sg = sg_next(sg)) { > - dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_TO_DEVICE); > - if (vring_mapping_error(vq, addr)) > - goto unmap_release; > - > prev = i; > /* Note that we trust indirect descriptor > * table since it use stream DMA mapping. > */ > - i = virtqueue_add_desc_split(_vq, desc, i, addr, sg->length, > + i = virtqueue_add_desc_split(_vq, desc, i, > + sg->dma_address, > + sg->length, > VRING_DESC_F_NEXT, > indirect); > } > } > for (; n < (out_sgs + in_sgs); n++) { > for (sg = sgs[n]; sg; sg = sg_next(sg)) { > - dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_FROM_DEVICE); > - if (vring_mapping_error(vq, addr)) > - goto unmap_release; > - > prev = i; > /* Note that we trust indirect descriptor > * table since it use stream DMA mapping. > */ > - i = virtqueue_add_desc_split(_vq, desc, i, addr, > + i = virtqueue_add_desc_split(_vq, desc, i, > + sg->dma_address, > sg->length, > VRING_DESC_F_NEXT | > VRING_DESC_F_WRITE, > @@ -630,7 +700,7 @@ static inline int virtqueue_add_split(struct virtqueue *_vq, > vq, desc, total_sg * sizeof(struct vring_desc), > DMA_TO_DEVICE); > if (vring_mapping_error(vq, addr)) > - goto unmap_release; > + return -ENOMEM; > > virtqueue_add_desc_split(_vq, vq->split.vring.desc, > head, addr, > @@ -648,13 +718,6 @@ static inline int virtqueue_add_split(struct virtqueue *_vq, > else > vq->free_head = i; > > - /* Store token and indirect buffer state. */ > - vq->split.desc_state[head].data = data; > - if (indirect) > - vq->split.desc_state[head].indir_desc = desc; > - else > - vq->split.desc_state[head].indir_desc = ctx; > - > /* Put entry in available array (but don't update avail->idx until they > * do sync). */ > avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1); > @@ -677,30 +740,52 @@ static inline int virtqueue_add_split(struct virtqueue *_vq, > virtqueue_kick(_vq); > > return 0; > +} > > -unmap_release: > - err_idx = i; > +static inline int virtqueue_add_split(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_desc *desc; > + int head; > + int err; > > - if (indirect) > - i = 0; > - else > - i = head; > + START_USE(vq); > > - for (n = 0; n < total_sg; n++) { > - if (i == err_idx) > - break; > - if (indirect) { > - vring_unmap_one_split_indirect(vq, &desc[i]); > - i = virtio16_to_cpu(_vq->vdev, desc[i].next); > - } else > - i = vring_unmap_one_split(vq, i); > - } > + /* check vq state and try to alloc desc for indirect. */ > + err = virtqueue_add_split_prepare(vq, total_sg, out_sgs, data, ctx, gfp, &desc); > + if (err) > + goto end; > > - if (indirect) > - kfree(desc); > + err = virtqueue_map_sgs(vq, sgs, total_sg, out_sgs, in_sgs); > + if (err) > + goto err; > > + head = vq->free_head; > + err = virtqueue_add_split_vring(vq, sgs, total_sg, out_sgs, in_sgs, desc); > + if (err) > + goto err; > + > + /* Store token and indirect buffer state. */ > + vq->split.desc_state[head].data = data; > + vq->split.desc_state[head].indir_desc = desc ? desc : ctx; > + > + goto end; > + > +err: > + virtqueue_unmap_sgs(vq, sgs, total_sg, out_sgs, in_sgs); > + > + kfree(desc); > + > +end: > END_USE(vq); > - return -ENOMEM; > + return err; > } > > static bool virtqueue_kick_prepare_split(struct virtqueue *_vq) > -- > 2.32.0.3.g01195cf9f >
Possibly Parallel Threads
- [PATCH vhost 00/10] virtio core prepares for AF_XDP
- [PATCH vhost v3 01/11] virtio_ring: split: separate dma codes
- [PATCH vhost v1 02/12] virtio_ring: split: separate DMA codes
- [PATCH vhost v1 00/12] virtio core prepares for AF_XDP
- [PATCH vhost v3 00/11] virtio core prepares for AF_XDP