In the scenario where indirect is not used, each desc corresponds to an
extra, which is used to record information such as dma, flags, and
next.
In the scenario of using indirect, the assigned desc does not have the
corresponding extra record dma information, and the dma information must
be obtained from the desc when unmap.
This patch allocates the corresponding extra array when indirect desc is
allocated. This has these advantages:
1. Record the dma information of desc, no need to read desc when unmap
2. It will be more convenient and unified in processing
3. Some additional information can be recorded in extra, which will be
used in subsequent patches.
Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com>
---
drivers/virtio/virtio_ring.c | 197 ++++++++++++++++-------------------
1 file changed, 91 insertions(+), 106 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 81531cbb08a7..64b4d2b03016 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -66,9 +66,21 @@
#define LAST_ADD_TIME_INVALID(vq)
#endif
+struct vring_desc_extra {
+ dma_addr_t addr; /* Descriptor DMA addr. */
+ u32 len; /* Descriptor length. */
+ u16 flags; /* Descriptor flags. */
+ u16 next; /* The next desc state in a list. */
+};
+
+struct vring_indirect_split {
+ struct vring_desc_extra *extra;
+ struct vring_desc desc[];
+};
+
struct vring_desc_state_split {
void *data; /* Data for callback. */
- struct vring_desc *indir_desc; /* Indirect descriptor, if any. */
+ struct vring_indirect_split *in;/* Indirect descriptor, if any. */
};
struct vring_desc_state_packed {
@@ -78,13 +90,6 @@ struct vring_desc_state_packed {
u16 last; /* The last desc state in a list. */
};
-struct vring_desc_extra {
- dma_addr_t addr; /* Descriptor DMA addr. */
- u32 len; /* Descriptor length. */
- u16 flags; /* Descriptor flags. */
- u16 next; /* The next desc state in a list. */
-};
-
struct vring_virtqueue {
struct virtqueue vq;
@@ -369,66 +374,40 @@ static int vring_mapping_error(const struct
vring_virtqueue *vq,
* Split ring specific functions - *_split().
*/
-static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
- struct vring_desc *desc)
-{
- u16 flags;
-
- if (!vq->use_dma_api)
- return;
-
- flags = virtio16_to_cpu(vq->vq.vdev, desc->flags);
-
- if (flags & VRING_DESC_F_INDIRECT) {
- dma_unmap_single(vring_dma_dev(vq),
- virtio64_to_cpu(vq->vq.vdev, desc->addr),
- virtio32_to_cpu(vq->vq.vdev, desc->len),
- (flags & VRING_DESC_F_WRITE) ?
- DMA_FROM_DEVICE : DMA_TO_DEVICE);
- } else {
- dma_unmap_page(vring_dma_dev(vq),
- virtio64_to_cpu(vq->vq.vdev, desc->addr),
- virtio32_to_cpu(vq->vq.vdev, desc->len),
- (flags & VRING_DESC_F_WRITE) ?
- DMA_FROM_DEVICE : DMA_TO_DEVICE);
- }
-}
-
static unsigned int vring_unmap_one_split(const struct vring_virtqueue *vq,
- unsigned int i)
+ struct vring_desc_extra *extra)
{
- struct vring_desc_extra *extra = vq->split.desc_extra;
u16 flags;
if (!vq->use_dma_api)
goto out;
- flags = extra[i].flags;
+ flags = extra->flags;
if (flags & VRING_DESC_F_INDIRECT) {
dma_unmap_single(vring_dma_dev(vq),
- extra[i].addr,
- extra[i].len,
+ extra->addr,
+ extra->len,
(flags & VRING_DESC_F_WRITE) ?
DMA_FROM_DEVICE : DMA_TO_DEVICE);
} else {
dma_unmap_page(vring_dma_dev(vq),
- extra[i].addr,
- extra[i].len,
+ extra->addr,
+ extra->len,
(flags & VRING_DESC_F_WRITE) ?
DMA_FROM_DEVICE : DMA_TO_DEVICE);
}
out:
- return extra[i].next;
+ return extra->next;
}
-static struct vring_desc *alloc_indirect_split(struct virtqueue *_vq,
- unsigned int total_sg,
- gfp_t gfp)
+static struct vring_indirect_split *alloc_indirect_split(struct virtqueue *_vq,
+ unsigned int total_sg,
+ gfp_t gfp)
{
- struct vring_desc *desc;
- unsigned int i;
+ struct vring_indirect_split *in;
+ unsigned int i, size;
/*
* We require lowmem mappings for the descriptors because
@@ -437,40 +416,52 @@ static struct vring_desc *alloc_indirect_split(struct
virtqueue *_vq,
*/
gfp &= ~__GFP_HIGHMEM;
- desc = kmalloc_array(total_sg, sizeof(struct vring_desc), gfp);
- if (!desc)
+ size = sizeof(struct vring_desc) + sizeof(struct vring_desc_extra);
+ size = size * total_sg + sizeof(*in);
+
+ in = kmalloc(size, gfp);
+ if (!in)
return NULL;
+ in->extra = (struct vring_desc_extra *)(in->desc + total_sg);
+
for (i = 0; i < total_sg; i++)
- desc[i].next = cpu_to_virtio16(_vq->vdev, i + 1);
- return desc;
+ in->extra[i].next = i + 1;
+
+ return in;
}
static inline unsigned int virtqueue_add_desc_split(struct virtqueue *vq,
- struct vring_desc *desc,
+ struct vring_indirect_split *in,
unsigned int i,
dma_addr_t addr,
unsigned int len,
- u16 flags,
- bool indirect)
+ u16 flags)
{
struct vring_virtqueue *vring = to_vvq(vq);
- struct vring_desc_extra *extra = vring->split.desc_extra;
+ struct vring_desc_extra *extra;
+ struct vring_desc *desc;
u16 next;
- desc[i].flags = cpu_to_virtio16(vq->vdev, flags);
- desc[i].addr = cpu_to_virtio64(vq->vdev, addr);
- desc[i].len = cpu_to_virtio32(vq->vdev, len);
+ if (!in) {
+ desc = vring->split.vring.desc + i;
+ extra = vring->split.desc_extra + i;
+
+ } else {
+ desc = in->desc + i;
+ extra = in->extra + i;
+ }
+
+ next = extra->next;
- if (!indirect) {
- next = extra[i].next;
- desc[i].next = cpu_to_virtio16(vq->vdev, next);
+ desc->flags = cpu_to_virtio16(vq->vdev, flags);
+ desc->addr = cpu_to_virtio64(vq->vdev, addr);
+ desc->len = cpu_to_virtio32(vq->vdev, len);
+ desc->next = cpu_to_virtio16(vq->vdev, next);
- extra[i].addr = addr;
- extra[i].len = len;
- extra[i].flags = flags;
- } else
- next = virtio16_to_cpu(vq->vdev, desc[i].next);
+ extra->addr = addr;
+ extra->len = len;
+ extra->flags = flags;
return next;
}
@@ -485,11 +476,11 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
gfp_t gfp)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ struct vring_indirect_split *in = NULL;
struct scatterlist *sg;
struct vring_desc *desc;
unsigned int i, n, avail, descs_used, prev, err_idx;
int head;
- bool indirect;
START_USE(vq);
@@ -507,21 +498,21 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
head = vq->free_head;
- if (virtqueue_use_indirect(_vq, total_sg))
- desc = alloc_indirect_split(_vq, total_sg, gfp);
- else {
+ if (virtqueue_use_indirect(_vq, total_sg)) {
+ in = alloc_indirect_split(_vq, total_sg, gfp);
+ if (in)
+ desc = in->desc;
+ } 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;
descs_used = 1;
} else {
- indirect = false;
desc = vq->split.vring.desc;
i = head;
descs_used = total_sg;
@@ -535,8 +526,7 @@ 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);
+ kfree(in);
END_USE(vq);
return -ENOSPC;
}
@@ -551,9 +541,8 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
/* Note that we trust indirect descriptor
* table since it use stream DMA mapping.
*/
- i = virtqueue_add_desc_split(_vq, desc, i, addr, sg->length,
- VRING_DESC_F_NEXT,
- indirect);
+ i = virtqueue_add_desc_split(_vq, in, i, addr, sg->length,
+ VRING_DESC_F_NEXT);
}
}
for (; n < (out_sgs + in_sgs); n++) {
@@ -566,20 +555,19 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
/* 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, in, i, addr,
sg->length,
VRING_DESC_F_NEXT |
- VRING_DESC_F_WRITE,
- indirect);
+ VRING_DESC_F_WRITE);
}
}
/* Last one doesn't continue. */
desc[prev].flags &= cpu_to_virtio16(_vq->vdev, ~VRING_DESC_F_NEXT);
- if (!indirect && vq->use_dma_api)
+ if (!in && vq->use_dma_api)
vq->split.desc_extra[prev & (vq->split.vring.num - 1)].flags &
~VRING_DESC_F_NEXT;
- if (indirect) {
+ if (in) {
/* Now that the indirect table is filled in, map it. */
dma_addr_t addr = vring_map_single(
vq, desc, total_sg * sizeof(struct vring_desc),
@@ -587,28 +575,26 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
if (vring_mapping_error(vq, addr))
goto unmap_release;
- virtqueue_add_desc_split(_vq, vq->split.vring.desc,
- head, addr,
+ virtqueue_add_desc_split(_vq, NULL, head, addr,
total_sg * sizeof(struct vring_desc),
- VRING_DESC_F_INDIRECT,
- false);
+ VRING_DESC_F_INDIRECT);
}
/* We're using some buffers from the free list. */
vq->vq.num_free -= descs_used;
/* Update free pointer */
- if (indirect)
+ if (in)
vq->free_head = vq->split.desc_extra[head].next;
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;
+ if (in)
+ vq->split.desc_state[head].in = in;
else
- vq->split.desc_state[head].indir_desc = ctx;
+ vq->split.desc_state[head].in = ctx;
/* Put entry in available array (but don't update avail->idx until they
* do sync). */
@@ -636,7 +622,7 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
unmap_release:
err_idx = i;
- if (indirect)
+ if (in)
i = 0;
else
i = head;
@@ -644,15 +630,13 @@ static inline int virtqueue_add_split(struct virtqueue
*_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);
+ if (in)
+ i = vring_unmap_one_split(vq, in->extra + i);
+ else
+ i = vring_unmap_one_split(vq, vq->split.desc_extra + i);
}
- if (indirect)
- kfree(desc);
+ kfree(in);
END_USE(vq);
return -ENOMEM;
@@ -702,12 +686,12 @@ static void detach_buf_split(struct vring_virtqueue *vq,
unsigned int head,
i = head;
while (vq->split.vring.desc[i].flags & nextflag) {
- vring_unmap_one_split(vq, i);
+ vring_unmap_one_split(vq, vq->split.desc_extra + i);
i = vq->split.desc_extra[i].next;
vq->vq.num_free++;
}
- vring_unmap_one_split(vq, i);
+ vring_unmap_one_split(vq, vq->split.desc_extra + i);
vq->split.desc_extra[i].next = vq->free_head;
vq->free_head = head;
@@ -715,12 +699,13 @@ static void detach_buf_split(struct vring_virtqueue *vq,
unsigned int head,
vq->vq.num_free++;
if (vq->indirect) {
- struct vring_desc *indir_desc - vq->split.desc_state[head].indir_desc;
+ struct vring_indirect_split *in;
u32 len;
+ in = vq->split.desc_state[head].in;
+
/* Free the indirect table, if any, now that it's unmapped. */
- if (!indir_desc)
+ if (!in)
return;
len = vq->split.desc_extra[head].len;
@@ -730,12 +715,12 @@ static void detach_buf_split(struct vring_virtqueue *vq,
unsigned int head,
BUG_ON(len == 0 || len % sizeof(struct vring_desc));
for (j = 0; j < len / sizeof(struct vring_desc); j++)
- vring_unmap_one_split_indirect(vq, &indir_desc[j]);
+ vring_unmap_one_split(vq, in->extra + j);
- kfree(indir_desc);
- vq->split.desc_state[head].indir_desc = NULL;
+ kfree(in);
+ vq->split.desc_state[head].in = NULL;
} else if (ctx) {
- *ctx = vq->split.desc_state[head].indir_desc;
+ *ctx = vq->split.desc_state[head].in;
}
}
--
2.31.0
Jason Wang
2022-Jan-10 06:43 UTC
[PATCH 2/6] virtio: split: alloc indirect desc with extra
? 2022/1/7 ??2:33, Xuan Zhuo ??:> In the scenario where indirect is not used, each desc corresponds to an > extra, which is used to record information such as dma, flags, and > next. > > In the scenario of using indirect, the assigned desc does not have the > corresponding extra record dma information, and the dma information must > be obtained from the desc when unmap. > > This patch allocates the corresponding extra array when indirect desc is > allocated. This has these advantages: > 1. Record the dma information of desc, no need to read desc when unmap > 2. It will be more convenient and unified in processing > 3. Some additional information can be recorded in extra, which will be > used in subsequent patches.Two questions: 1) Is there any performance number for this change? I guess it gives more stress on the cache. 2) Is there a requirement to mix the pre mapped sg with unmapped sg? If not, a per virtqueue flag looks sufficient Thanks