Jason Wang
2022-Apr-12 06:28 UTC
[PATCH v9 12/32] virtio_ring: packed: extract the logic of alloc queue
? 2022/4/6 ??11:43, Xuan Zhuo ??:> 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> > --- > drivers/virtio/virtio_ring.c | 70 ++++++++++++++++++++++++++++-------- > 1 file changed, 56 insertions(+), 14 deletions(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 33864134a744..ea451ae2aaef 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -1817,19 +1817,17 @@ static struct vring_desc_extra *vring_alloc_desc_extra(unsigned int num) > 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 int vring_alloc_queue_packed(struct virtio_device *vdev, > + 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 *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; > @@ -1857,6 +1855,52 @@ static struct virtqueue *vring_create_virtqueue_packed( > if (!device) > goto err_device; > > + *_ring = ring; > + *_driver = driver; > + *_device = device; > + *_ring_dma_addr = ring_dma_addr; > + *_driver_event_dma_addr = driver_event_dma_addr; > + *_device_event_dma_addr = device_event_dma_addr; > + *_ring_size_in_bytes = ring_size_in_bytes; > + *_event_size_in_bytes = event_size_in_bytes;I wonder if we can simply factor out split and packed from struct vring_virtqueue: struct vring_virtqueue { ??? union { ??? ??? struct {} split; ??? ??? struct {} packed; ??? }; }; to struct vring_virtqueue_split {}; struct vring_virtqueue_packed {}; Then we can do things like: vring_create_virtqueue_packed(struct virtio_device *vdev, u32 num, struct vring_virtqueue_packed *packed); and vring_vritqueue_attach_packed(struct vring_virtqueue *vq, struct vring_virtqueue_packed packed); Thanks> + > + return 0; > + > +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: > + 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) > +{ > + 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; > + struct vring_virtqueue *vq; > + > + if (vring_alloc_queue_packed(vdev, num, &ring, &driver, &device, > + &ring_dma_addr, &driver_event_dma_addr, > + &device_event_dma_addr, > + &ring_size_in_bytes, > + &event_size_in_bytes)) > + goto err_ring; > + > vq = kmalloc(sizeof(*vq), GFP_KERNEL); > if (!vq) > goto err_vq; > @@ -1939,9 +1983,7 @@ static struct virtqueue *vring_create_virtqueue_packed( > 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: > return NULL;
Xuan Zhuo
2022-Apr-13 03:23 UTC
[PATCH v9 12/32] virtio_ring: packed: extract the logic of alloc queue
On Tue, 12 Apr 2022 14:28:24 +0800, Jason Wang <jasowang at redhat.com> wrote:> > ? 2022/4/6 ??11:43, Xuan Zhuo ??: > > 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> > > --- > > drivers/virtio/virtio_ring.c | 70 ++++++++++++++++++++++++++++-------- > > 1 file changed, 56 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > > index 33864134a744..ea451ae2aaef 100644 > > --- a/drivers/virtio/virtio_ring.c > > +++ b/drivers/virtio/virtio_ring.c > > @@ -1817,19 +1817,17 @@ static struct vring_desc_extra *vring_alloc_desc_extra(unsigned int num) > > 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 int vring_alloc_queue_packed(struct virtio_device *vdev, > > + 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 *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; > > @@ -1857,6 +1855,52 @@ static struct virtqueue *vring_create_virtqueue_packed( > > if (!device) > > goto err_device; > > > > + *_ring = ring; > > + *_driver = driver; > > + *_device = device; > > + *_ring_dma_addr = ring_dma_addr; > > + *_driver_event_dma_addr = driver_event_dma_addr; > > + *_device_event_dma_addr = device_event_dma_addr; > > + *_ring_size_in_bytes = ring_size_in_bytes; > > + *_event_size_in_bytes = event_size_in_bytes; > > > I wonder if we can simply factor out split and packed from struct > vring_virtqueue: > > struct vring_virtqueue { > ??? union { > ??? ??? struct {} split; > ??? ??? struct {} packed; > ??? }; > }; > > to > > struct vring_virtqueue_split {}; > struct vring_virtqueue_packed {}; > > Then we can do things like: > > vring_create_virtqueue_packed(struct virtio_device *vdev, u32 num, > struct vring_virtqueue_packed *packed); > > and > > vring_vritqueue_attach_packed(struct vring_virtqueue *vq, struct > vring_virtqueue_packed packed);This idea is very similar to my previous idea, just without introducing a new structure. I'd be more than happy to revise this. Thanks.> > Thanks > > > > + > > + return 0; > > + > > +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: > > + 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) > > +{ > > + 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; > > + struct vring_virtqueue *vq; > > + > > + if (vring_alloc_queue_packed(vdev, num, &ring, &driver, &device, > > + &ring_dma_addr, &driver_event_dma_addr, > > + &device_event_dma_addr, > > + &ring_size_in_bytes, > > + &event_size_in_bytes)) > > + goto err_ring; > > + > > vq = kmalloc(sizeof(*vq), GFP_KERNEL); > > if (!vq) > > goto err_vq; > > @@ -1939,9 +1983,7 @@ static struct virtqueue *vring_create_virtqueue_packed( > > 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: > > return NULL; >