Re: [PATCH v9 12/32] virtio_ring: packed: extract the logic of alloc queue
On Wed, Apr 13, 2022 at 11:26 AM Xuan Zhuo wrote: > > On Tue, 12 Apr 2022 14:28:24 +0800, Jason Wang 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 > > > --- > > > 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. Yes, it's better to not introduce new structures if it's possible. > > I'd be more than happy to revise this. Good to know this. Thanks > > 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, , , , > > > +_dma_addr, _event_dma_addr, > > > +_event_dma_addr, > > > +_size_in_bytes, > > > +
Re: [PATCH v9 12/32] virtio_ring: packed: extract the logic of alloc queue
On Tue, 12 Apr 2022 14:28:24 +0800, Jason Wang 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 > > --- > > 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, , , , > > +_dma_addr, _event_dma_addr, > > +_event_dma_addr, > > +_size_in_bytes, > > +_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,
Re: [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 --- 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, , , , +_dma_addr, _event_dma_addr, +_event_dma_addr, +_size_in_bytes, +_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; ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization