Xuan Zhuo
2022-Mar-08 12:34 UTC
[PATCH v7 05/26] virtio_ring: split: extract the logic of init vq and attach vring
Split the logic of split assignment vq into three parts.
1. The assignment passed from the function parameter
2. The part that attaches vring to vq. -- __vring_virtqueue_attach_split()
3. The part that initializes vq to a fixed value --
__vring_virtqueue_init_split()
This feature is required for subsequent virtuqueue reset vring
Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com>
---
drivers/virtio/virtio_ring.c | 111 +++++++++++++++++++++--------------
1 file changed, 67 insertions(+), 44 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index d32793615451..dc6313b79305 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2196,34 +2196,40 @@ irqreturn_t vring_interrupt(int irq, void *_vq)
}
EXPORT_SYMBOL_GPL(vring_interrupt);
-/* Only available for split ring */
-struct virtqueue *__vring_new_virtqueue(unsigned int index,
- struct vring vring,
- struct virtio_device *vdev,
- bool weak_barriers,
- bool context,
- bool (*notify)(struct virtqueue *),
- void (*callback)(struct virtqueue *),
- const char *name)
+static int __vring_virtqueue_attach_split(struct vring_virtqueue *vq,
+ struct virtio_device *vdev,
+ struct vring vring)
{
- struct vring_virtqueue *vq;
+ vq->vq.num_free = vring.num;
- if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED))
- return NULL;
+ vq->split.vring = vring;
+ vq->split.queue_dma_addr = 0;
+ vq->split.queue_size_in_bytes = 0;
- vq = kmalloc(sizeof(*vq), GFP_KERNEL);
- if (!vq)
- return NULL;
+ vq->split.desc_state = kmalloc_array(vring.num,
+ sizeof(struct vring_desc_state_split), GFP_KERNEL);
+ if (!vq->split.desc_state)
+ goto err_state;
+ vq->split.desc_extra = vring_alloc_desc_extra(vq, vring.num);
+ if (!vq->split.desc_extra)
+ goto err_extra;
+
+ memset(vq->split.desc_state, 0, vring.num *
+ sizeof(struct vring_desc_state_split));
+ return 0;
+
+err_extra:
+ kfree(vq->split.desc_state);
+err_state:
+ return -ENOMEM;
+}
+
+static void __vring_virtqueue_init_split(struct vring_virtqueue *vq,
+ struct virtio_device *vdev)
+{
vq->packed_ring = false;
- vq->vq.callback = callback;
- vq->vq.vdev = vdev;
- vq->vq.name = name;
- vq->vq.num_free = vring.num;
- vq->vq.index = index;
vq->we_own_ring = false;
- vq->notify = notify;
- vq->weak_barriers = weak_barriers;
vq->broken = false;
vq->last_used_idx = 0;
vq->event_triggered = false;
@@ -2234,50 +2240,67 @@ struct virtqueue *__vring_new_virtqueue(unsigned int
index,
vq->last_add_time_valid = false;
#endif
- vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)
&&
- !context;
vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
vq->weak_barriers = false;
- vq->split.queue_dma_addr = 0;
- vq->split.queue_size_in_bytes = 0;
-
- vq->split.vring = vring;
vq->split.avail_flags_shadow = 0;
vq->split.avail_idx_shadow = 0;
/* No callback? Tell other side not to bother us. */
- if (!callback) {
+ if (!vq->vq.callback) {
vq->split.avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
if (!vq->event)
vq->split.vring.avail->flags = cpu_to_virtio16(vdev,
vq->split.avail_flags_shadow);
}
- vq->split.desc_state = kmalloc_array(vring.num,
- sizeof(struct vring_desc_state_split), GFP_KERNEL);
- if (!vq->split.desc_state)
- goto err_state;
-
- vq->split.desc_extra = vring_alloc_desc_extra(vq, vring.num);
- if (!vq->split.desc_extra)
- goto err_extra;
-
/* Put everything in free lists. */
vq->free_head = 0;
- memset(vq->split.desc_state, 0, vring.num *
- sizeof(struct vring_desc_state_split));
+}
+
+/* Only available for split ring */
+struct virtqueue *__vring_new_virtqueue(unsigned int index,
+ struct vring vring,
+ struct virtio_device *vdev,
+ bool weak_barriers,
+ bool context,
+ bool (*notify)(struct virtqueue *),
+ void (*callback)(struct virtqueue *),
+ const char *name)
+{
+ struct vring_virtqueue *vq;
+ int err;
+
+ if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED))
+ return NULL;
+
+ vq = kmalloc(sizeof(*vq), GFP_KERNEL);
+ if (!vq)
+ return NULL;
+
+ vq->vq.callback = callback;
+ vq->vq.vdev = vdev;
+ vq->vq.name = name;
+ vq->vq.index = index;
+ vq->notify = notify;
+ vq->weak_barriers = weak_barriers;
+ vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)
&&
+ !context;
+
+ err = __vring_virtqueue_attach_split(vq, vdev, vring);
+ if (err)
+ goto err;
+
+ __vring_virtqueue_init_split(vq, vdev);
spin_lock(&vdev->vqs_list_lock);
list_add_tail(&vq->vq.list, &vdev->vqs);
spin_unlock(&vdev->vqs_list_lock);
- return &vq->vq;
-err_extra:
- kfree(vq->split.desc_state);
-err_state:
+ return &vq->vq;
+err:
kfree(vq);
return NULL;
}
--
2.31.0
Jason Wang
2022-Mar-09 07:36 UTC
[PATCH v7 05/26] virtio_ring: split: extract the logic of init vq and attach vring
? 2022/3/8 ??8:34, Xuan Zhuo ??:> Split the logic of split assignment vq into three parts. > > 1. The assignment passed from the function parameter > 2. The part that attaches vring to vq. -- __vring_virtqueue_attach_split() > 3. The part that initializes vq to a fixed value -- > __vring_virtqueue_init_split() > > This feature is required for subsequent virtuqueue reset vring > > Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com> > --- > drivers/virtio/virtio_ring.c | 111 +++++++++++++++++++++-------------- > 1 file changed, 67 insertions(+), 44 deletions(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index d32793615451..dc6313b79305 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -2196,34 +2196,40 @@ irqreturn_t vring_interrupt(int irq, void *_vq) > } > EXPORT_SYMBOL_GPL(vring_interrupt); > > -/* Only available for split ring */ > -struct virtqueue *__vring_new_virtqueue(unsigned int index, > - struct vring vring, > - struct virtio_device *vdev, > - bool weak_barriers, > - bool context, > - bool (*notify)(struct virtqueue *), > - void (*callback)(struct virtqueue *), > - const char *name) > +static int __vring_virtqueue_attach_split(struct vring_virtqueue *vq, > + struct virtio_device *vdev, > + struct vring vring) > { > - struct vring_virtqueue *vq; > + vq->vq.num_free = vring.num; > > - if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)) > - return NULL; > + vq->split.vring = vring; > + vq->split.queue_dma_addr = 0; > + vq->split.queue_size_in_bytes = 0; > > - vq = kmalloc(sizeof(*vq), GFP_KERNEL); > - if (!vq) > - return NULL; > + vq->split.desc_state = kmalloc_array(vring.num, > + sizeof(struct vring_desc_state_split), GFP_KERNEL); > + if (!vq->split.desc_state) > + goto err_state; > > + vq->split.desc_extra = vring_alloc_desc_extra(vq, vring.num); > + if (!vq->split.desc_extra) > + goto err_extra;So this contains stuffs more than just attach. I wonder if it's better to split the allocation out to an dedicated helper (we have dedicated helper to allocate vring). Thanks> + > + memset(vq->split.desc_state, 0, vring.num * > + sizeof(struct vring_desc_state_split)); > + return 0; > + > +err_extra: > + kfree(vq->split.desc_state); > +err_state: > + return -ENOMEM; > +} > + > +static void __vring_virtqueue_init_split(struct vring_virtqueue *vq, > + struct virtio_device *vdev) > +{ > vq->packed_ring = false; > - vq->vq.callback = callback; > - vq->vq.vdev = vdev; > - vq->vq.name = name; > - vq->vq.num_free = vring.num; > - vq->vq.index = index; > vq->we_own_ring = false; > - vq->notify = notify; > - vq->weak_barriers = weak_barriers; > vq->broken = false; > vq->last_used_idx = 0; > vq->event_triggered = false; > @@ -2234,50 +2240,67 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index, > vq->last_add_time_valid = false; > #endif > > - vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC) && > - !context; > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX); > > if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM)) > vq->weak_barriers = false; > > - vq->split.queue_dma_addr = 0; > - vq->split.queue_size_in_bytes = 0; > - > - vq->split.vring = vring; > vq->split.avail_flags_shadow = 0; > vq->split.avail_idx_shadow = 0; > > /* No callback? Tell other side not to bother us. */ > - if (!callback) { > + if (!vq->vq.callback) { > vq->split.avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT; > if (!vq->event) > vq->split.vring.avail->flags = cpu_to_virtio16(vdev, > vq->split.avail_flags_shadow); > } > > - vq->split.desc_state = kmalloc_array(vring.num, > - sizeof(struct vring_desc_state_split), GFP_KERNEL); > - if (!vq->split.desc_state) > - goto err_state; > - > - vq->split.desc_extra = vring_alloc_desc_extra(vq, vring.num); > - if (!vq->split.desc_extra) > - goto err_extra; > - > /* Put everything in free lists. */ > vq->free_head = 0; > - memset(vq->split.desc_state, 0, vring.num * > - sizeof(struct vring_desc_state_split)); > +} > + > +/* Only available for split ring */ > +struct virtqueue *__vring_new_virtqueue(unsigned int index, > + struct vring vring, > + struct virtio_device *vdev, > + bool weak_barriers, > + bool context, > + bool (*notify)(struct virtqueue *), > + void (*callback)(struct virtqueue *), > + const char *name) > +{ > + struct vring_virtqueue *vq; > + int err; > + > + if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)) > + return NULL; > + > + vq = kmalloc(sizeof(*vq), GFP_KERNEL); > + if (!vq) > + return NULL; > + > + vq->vq.callback = callback; > + vq->vq.vdev = vdev; > + vq->vq.name = name; > + vq->vq.index = index; > + vq->notify = notify; > + vq->weak_barriers = weak_barriers; > + vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC) && > + !context; > + > + err = __vring_virtqueue_attach_split(vq, vdev, vring); > + if (err) > + goto err; > + > + __vring_virtqueue_init_split(vq, vdev); > > spin_lock(&vdev->vqs_list_lock); > list_add_tail(&vq->vq.list, &vdev->vqs); > spin_unlock(&vdev->vqs_list_lock); > - return &vq->vq; > > -err_extra: > - kfree(vq->split.desc_state); > -err_state: > + return &vq->vq; > +err: > kfree(vq); > return NULL; > }