Xuan Zhuo
2022-Mar-08 07:01 UTC
[PATCH v6 06/26] virtio_ring: packed: extrace the logic of creating vring
On Mon, 7 Mar 2022 17:17:51 -0500, "Michael S. Tsirkin" <mst at redhat.com> wrote:> On Thu, Feb 24, 2022 at 04:10:42PM +0800, Xuan Zhuo wrote: > > Separate the logic of packed to create vring queue. > > > > For the convenience of passing parameters, add a structure > > vring_packed. > > > > This feature is required for subsequent virtuqueue reset vring. > > > > Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com> > > Subject has a typo.I will fix it.> Besides: > > > --- > > drivers/virtio/virtio_ring.c | 121 ++++++++++++++++++++++++++--------- > > 1 file changed, 92 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > > index dc6313b79305..41864c5e665f 100644 > > --- a/drivers/virtio/virtio_ring.c > > +++ b/drivers/virtio/virtio_ring.c > > @@ -92,6 +92,18 @@ struct vring_split { > > struct vring vring; > > }; > > > > +struct vring_packed { > > + u32 num; > > + struct vring_packed_desc *ring; > > + struct vring_packed_desc_event *driver; > > + struct vring_packed_desc_event *device; > > + dma_addr_t ring_dma_addr; > > + dma_addr_t driver_event_dma_addr; > > + dma_addr_t device_event_dma_addr; > > + size_t ring_size_in_bytes; > > + size_t event_size_in_bytes; > > +}; > > + > > struct vring_virtqueue { > > struct virtqueue vq; > > > > @@ -1683,45 +1695,101 @@ static struct vring_desc_extra *vring_alloc_desc_extra(struct vring_virtqueue *v > > return desc_extra; > > } > > > > -static struct virtqueue *vring_create_virtqueue_packed( > > - unsigned int index, > > - unsigned int num, > > - unsigned int vring_align, > > - struct virtio_device *vdev, > > - bool weak_barriers, > > - bool may_reduce_num, > > - bool context, > > - bool (*notify)(struct virtqueue *), > > - void (*callback)(struct virtqueue *), > > - const char *name) > > +static void vring_free_vring_packed(struct vring_packed *vring, > > + struct virtio_device *vdev) > > +{ > > + dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr; > > + struct vring_packed_desc_event *driver, *device; > > + size_t ring_size_in_bytes, event_size_in_bytes; > > + struct vring_packed_desc *ring; > > + > > + ring = vring->ring; > > + driver = vring->driver; > > + device = vring->device; > > + ring_dma_addr = vring->ring_size_in_bytes; > > + event_size_in_bytes = vring->event_size_in_bytes; > > + ring_dma_addr = vring->ring_dma_addr; > > + driver_event_dma_addr = vring->driver_event_dma_addr; > > + device_event_dma_addr = vring->device_event_dma_addr; > > + > > + if (device) > > + vring_free_queue(vdev, event_size_in_bytes, device, device_event_dma_addr); > > + > > + if (driver) > > + vring_free_queue(vdev, event_size_in_bytes, driver, driver_event_dma_addr); > > + > > + if (ring) > > + vring_free_queue(vdev, ring_size_in_bytes, ring, ring_dma_addr); > > ring_size_in_bytes is uninitialized here. > > Which begs the question how was this tested patchset generally and > this patch in particular. > Please add note on tested configurations and tests run to the patchset.Sorry, my environment is running in split mode. I did not retest the packed mode before sending patches. Because my dpdk vhost-user is not easy to use, I need to change the kernel of the host. I would like to ask if there are other lightweight environments that can be used to test packed mode. Thanks.> > > +} > > + > > +static int vring_create_vring_packed(struct vring_packed *vring, > > + struct virtio_device *vdev, > > + u32 num) > > { > > - struct vring_virtqueue *vq; > > struct vring_packed_desc *ring; > > struct vring_packed_desc_event *driver, *device; > > dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr; > > size_t ring_size_in_bytes, event_size_in_bytes; > > > > + memset(vring, 0, sizeof(*vring)); > > + > > ring_size_in_bytes = num * sizeof(struct vring_packed_desc); > > > > ring = vring_alloc_queue(vdev, ring_size_in_bytes, > > &ring_dma_addr, > > GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO); > > if (!ring) > > - goto err_ring; > > + goto err; > > + > > + vring->num = num; > > + vring->ring = ring; > > + vring->ring_size_in_bytes = ring_size_in_bytes; > > + vring->ring_dma_addr = ring_dma_addr; > > > > event_size_in_bytes = sizeof(struct vring_packed_desc_event); > > + vring->event_size_in_bytes = event_size_in_bytes; > > > > driver = vring_alloc_queue(vdev, event_size_in_bytes, > > &driver_event_dma_addr, > > GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO); > > if (!driver) > > - goto err_driver; > > + goto err; > > + > > + vring->driver = driver; > > + vring->driver_event_dma_addr = driver_event_dma_addr; > > > > device = vring_alloc_queue(vdev, event_size_in_bytes, > > &device_event_dma_addr, > > GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO); > > if (!device) > > - goto err_device; > > + goto err; > > + > > + vring->device = device; > > + vring->device_event_dma_addr = device_event_dma_addr; > > + return 0; > > + > > +err: > > + vring_free_vring_packed(vring, vdev); > > + return -ENOMEM; > > +} > > + > > +static struct virtqueue *vring_create_virtqueue_packed( > > + unsigned int index, > > + unsigned int num, > > + unsigned int vring_align, > > + struct virtio_device *vdev, > > + bool weak_barriers, > > + bool may_reduce_num, > > + bool context, > > + bool (*notify)(struct virtqueue *), > > + void (*callback)(struct virtqueue *), > > + const char *name) > > +{ > > + struct vring_virtqueue *vq; > > + struct vring_packed vring; > > + > > + if (vring_create_vring_packed(&vring, vdev, num)) > > + goto err_vq; > > > > vq = kmalloc(sizeof(*vq), GFP_KERNEL); > > if (!vq) > > @@ -1753,17 +1821,17 @@ static struct virtqueue *vring_create_virtqueue_packed( > > if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM)) > > vq->weak_barriers = false; > > > > - vq->packed.ring_dma_addr = ring_dma_addr; > > - vq->packed.driver_event_dma_addr = driver_event_dma_addr; > > - vq->packed.device_event_dma_addr = device_event_dma_addr; > > + vq->packed.ring_dma_addr = vring.ring_dma_addr; > > + vq->packed.driver_event_dma_addr = vring.driver_event_dma_addr; > > + vq->packed.device_event_dma_addr = vring.device_event_dma_addr; > > > > - vq->packed.ring_size_in_bytes = ring_size_in_bytes; > > - vq->packed.event_size_in_bytes = event_size_in_bytes; > > + vq->packed.ring_size_in_bytes = vring.ring_size_in_bytes; > > + vq->packed.event_size_in_bytes = vring.event_size_in_bytes; > > > > vq->packed.vring.num = num; > > - vq->packed.vring.desc = ring; > > - vq->packed.vring.driver = driver; > > - vq->packed.vring.device = device; > > + vq->packed.vring.desc = vring.ring; > > + vq->packed.vring.driver = vring.driver; > > + vq->packed.vring.device = vring.device; > > > > vq->packed.next_avail_idx = 0; > > vq->packed.avail_wrap_counter = 1; > > @@ -1804,12 +1872,7 @@ static struct virtqueue *vring_create_virtqueue_packed( > > err_desc_state: > > kfree(vq); > > err_vq: > > - vring_free_queue(vdev, event_size_in_bytes, device, device_event_dma_addr); > > -err_device: > > - vring_free_queue(vdev, event_size_in_bytes, driver, driver_event_dma_addr); > > -err_driver: > > - vring_free_queue(vdev, ring_size_in_bytes, ring, ring_dma_addr); > > -err_ring: > > + vring_free_vring_packed(&vring, vdev); > > return NULL; > > } > > > > -- > > 2.31.0 >
Jason Wang
2022-Mar-08 07:28 UTC
[PATCH v6 06/26] virtio_ring: packed: extrace the logic of creating vring
? 2022/3/8 ??3:01, Xuan Zhuo ??:> On Mon, 7 Mar 2022 17:17:51 -0500, "Michael S. Tsirkin" <mst at redhat.com> wrote: >> On Thu, Feb 24, 2022 at 04:10:42PM +0800, Xuan Zhuo wrote: >>> Separate the logic of packed to create vring queue. >>> >>> For the convenience of passing parameters, add a structure >>> vring_packed. >>> >>> This feature is required for subsequent virtuqueue reset vring. >>> >>> Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com> >> Subject has a typo. > I will fix it. > >> Besides: >> >>> --- >>> drivers/virtio/virtio_ring.c | 121 ++++++++++++++++++++++++++--------- >>> 1 file changed, 92 insertions(+), 29 deletions(-) >>> >>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >>> index dc6313b79305..41864c5e665f 100644 >>> --- a/drivers/virtio/virtio_ring.c >>> +++ b/drivers/virtio/virtio_ring.c >>> @@ -92,6 +92,18 @@ struct vring_split { >>> struct vring vring; >>> }; >>> >>> +struct vring_packed { >>> + u32 num; >>> + struct vring_packed_desc *ring; >>> + struct vring_packed_desc_event *driver; >>> + struct vring_packed_desc_event *device; >>> + dma_addr_t ring_dma_addr; >>> + dma_addr_t driver_event_dma_addr; >>> + dma_addr_t device_event_dma_addr; >>> + size_t ring_size_in_bytes; >>> + size_t event_size_in_bytes; >>> +}; >>> + >>> struct vring_virtqueue { >>> struct virtqueue vq; >>> >>> @@ -1683,45 +1695,101 @@ static struct vring_desc_extra *vring_alloc_desc_extra(struct vring_virtqueue *v >>> return desc_extra; >>> } >>> >>> -static struct virtqueue *vring_create_virtqueue_packed( >>> - unsigned int index, >>> - unsigned int num, >>> - unsigned int vring_align, >>> - struct virtio_device *vdev, >>> - bool weak_barriers, >>> - bool may_reduce_num, >>> - bool context, >>> - bool (*notify)(struct virtqueue *), >>> - void (*callback)(struct virtqueue *), >>> - const char *name) >>> +static void vring_free_vring_packed(struct vring_packed *vring, >>> + struct virtio_device *vdev) >>> +{ >>> + dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr; >>> + struct vring_packed_desc_event *driver, *device; >>> + size_t ring_size_in_bytes, event_size_in_bytes; >>> + struct vring_packed_desc *ring; >>> + >>> + ring = vring->ring; >>> + driver = vring->driver; >>> + device = vring->device; >>> + ring_dma_addr = vring->ring_size_in_bytes; >>> + event_size_in_bytes = vring->event_size_in_bytes; >>> + ring_dma_addr = vring->ring_dma_addr; >>> + driver_event_dma_addr = vring->driver_event_dma_addr; >>> + device_event_dma_addr = vring->device_event_dma_addr; >>> + >>> + if (device) >>> + vring_free_queue(vdev, event_size_in_bytes, device, device_event_dma_addr); >>> + >>> + if (driver) >>> + vring_free_queue(vdev, event_size_in_bytes, driver, driver_event_dma_addr); >>> + >>> + if (ring) >>> + vring_free_queue(vdev, ring_size_in_bytes, ring, ring_dma_addr); >> ring_size_in_bytes is uninitialized here. >> >> Which begs the question how was this tested patchset generally and >> this patch in particular. >> Please add note on tested configurations and tests run to the patchset. > Sorry, my environment is running in split mode. I did not retest the packed mode > before sending patches. Because my dpdk vhost-user is not easy to use, I > need to change the kernel of the host. > > I would like to ask if there are other lightweight environments that can be used > to test packed mode.You can use Qemu's dataplane. It has support for packed virtqueue. Thanks> > > Thanks. > > >>> +} >>> + >>> +static int vring_create_vring_packed(struct vring_packed *vring, >>> + struct virtio_device *vdev, >>> + u32 num) >>> { >>> - struct vring_virtqueue *vq; >>> struct vring_packed_desc *ring; >>> struct vring_packed_desc_event *driver, *device; >>> dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr; >>> size_t ring_size_in_bytes, event_size_in_bytes; >>> >>> + memset(vring, 0, sizeof(*vring)); >>> + >>> ring_size_in_bytes = num * sizeof(struct vring_packed_desc); >>> >>> ring = vring_alloc_queue(vdev, ring_size_in_bytes, >>> &ring_dma_addr, >>> GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO); >>> if (!ring) >>> - goto err_ring; >>> + goto err; >>> + >>> + vring->num = num; >>> + vring->ring = ring; >>> + vring->ring_size_in_bytes = ring_size_in_bytes; >>> + vring->ring_dma_addr = ring_dma_addr; >>> >>> event_size_in_bytes = sizeof(struct vring_packed_desc_event); >>> + vring->event_size_in_bytes = event_size_in_bytes; >>> >>> driver = vring_alloc_queue(vdev, event_size_in_bytes, >>> &driver_event_dma_addr, >>> GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO); >>> if (!driver) >>> - goto err_driver; >>> + goto err; >>> + >>> + vring->driver = driver; >>> + vring->driver_event_dma_addr = driver_event_dma_addr; >>> >>> device = vring_alloc_queue(vdev, event_size_in_bytes, >>> &device_event_dma_addr, >>> GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO); >>> if (!device) >>> - goto err_device; >>> + goto err; >>> + >>> + vring->device = device; >>> + vring->device_event_dma_addr = device_event_dma_addr; >>> + return 0; >>> + >>> +err: >>> + vring_free_vring_packed(vring, vdev); >>> + return -ENOMEM; >>> +} >>> + >>> +static struct virtqueue *vring_create_virtqueue_packed( >>> + unsigned int index, >>> + unsigned int num, >>> + unsigned int vring_align, >>> + struct virtio_device *vdev, >>> + bool weak_barriers, >>> + bool may_reduce_num, >>> + bool context, >>> + bool (*notify)(struct virtqueue *), >>> + void (*callback)(struct virtqueue *), >>> + const char *name) >>> +{ >>> + struct vring_virtqueue *vq; >>> + struct vring_packed vring; >>> + >>> + if (vring_create_vring_packed(&vring, vdev, num)) >>> + goto err_vq; >>> >>> vq = kmalloc(sizeof(*vq), GFP_KERNEL); >>> if (!vq) >>> @@ -1753,17 +1821,17 @@ static struct virtqueue *vring_create_virtqueue_packed( >>> if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM)) >>> vq->weak_barriers = false; >>> >>> - vq->packed.ring_dma_addr = ring_dma_addr; >>> - vq->packed.driver_event_dma_addr = driver_event_dma_addr; >>> - vq->packed.device_event_dma_addr = device_event_dma_addr; >>> + vq->packed.ring_dma_addr = vring.ring_dma_addr; >>> + vq->packed.driver_event_dma_addr = vring.driver_event_dma_addr; >>> + vq->packed.device_event_dma_addr = vring.device_event_dma_addr; >>> >>> - vq->packed.ring_size_in_bytes = ring_size_in_bytes; >>> - vq->packed.event_size_in_bytes = event_size_in_bytes; >>> + vq->packed.ring_size_in_bytes = vring.ring_size_in_bytes; >>> + vq->packed.event_size_in_bytes = vring.event_size_in_bytes; >>> >>> vq->packed.vring.num = num; >>> - vq->packed.vring.desc = ring; >>> - vq->packed.vring.driver = driver; >>> - vq->packed.vring.device = device; >>> + vq->packed.vring.desc = vring.ring; >>> + vq->packed.vring.driver = vring.driver; >>> + vq->packed.vring.device = vring.device; >>> >>> vq->packed.next_avail_idx = 0; >>> vq->packed.avail_wrap_counter = 1; >>> @@ -1804,12 +1872,7 @@ static struct virtqueue *vring_create_virtqueue_packed( >>> err_desc_state: >>> kfree(vq); >>> err_vq: >>> - vring_free_queue(vdev, event_size_in_bytes, device, device_event_dma_addr); >>> -err_device: >>> - vring_free_queue(vdev, event_size_in_bytes, driver, driver_event_dma_addr); >>> -err_driver: >>> - vring_free_queue(vdev, ring_size_in_bytes, ring, ring_dma_addr); >>> -err_ring: >>> + vring_free_vring_packed(&vring, vdev); >>> return NULL; >>> } >>> >>> -- >>> 2.31.0