Xuan Zhuo
2022-Jan-19 02:35 UTC
[PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches
Add a vq_enable_vq_msix() function to enable a vq alone.
Move irq's processing logic into vp_del_vq(), so that this function can
handle a vq's del operation independently.
In the subsequent patches that supports queue reset, I have the
need to delete/enable a vq separately.
Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com>
---
drivers/virtio/virtio_pci_common.c | 86 +++++++++++++++++++-----------
1 file changed, 54 insertions(+), 32 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c
b/drivers/virtio/virtio_pci_common.c
index fdbde1db5ec5..5afe207ce28a 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -248,6 +248,17 @@ static void vp_del_vq(struct virtqueue *vq)
struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
unsigned long flags;
+ if (vp_dev->per_vq_vectors) {
+ int v = vp_dev->vqs[vq->index]->msix_vector;
+
+ if (v != VIRTIO_MSI_NO_VECTOR) {
+ int irq = pci_irq_vector(vp_dev->pci_dev, v);
+
+ irq_set_affinity_hint(irq, NULL);
+ free_irq(irq, vq);
+ }
+ }
+
spin_lock_irqsave(&vp_dev->lock, flags);
list_del(&info->node);
spin_unlock_irqrestore(&vp_dev->lock, flags);
@@ -263,19 +274,9 @@ void vp_del_vqs(struct virtio_device *vdev)
struct virtqueue *vq, *n;
int i;
- list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
- if (vp_dev->per_vq_vectors) {
- int v = vp_dev->vqs[vq->index]->msix_vector;
-
- if (v != VIRTIO_MSI_NO_VECTOR) {
- int irq = pci_irq_vector(vp_dev->pci_dev, v);
-
- irq_set_affinity_hint(irq, NULL);
- free_irq(irq, vq);
- }
- }
+ list_for_each_entry_safe(vq, n, &vdev->vqs, list)
vp_del_vq(vq);
- }
+
vp_dev->per_vq_vectors = false;
if (vp_dev->intx_enabled) {
@@ -310,6 +311,40 @@ void vp_del_vqs(struct virtio_device *vdev)
vp_dev->vqs = NULL;
}
+static struct virtqueue *vp_enable_vq_msix(struct virtio_device *vdev,
+ int queue_index,
+ vq_callback_t *callback,
+ const char * const name,
+ bool ctx,
+ u16 msix_vec)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct virtqueue *vq;
+ int err;
+
+ vq = vp_setup_vq(vdev, queue_index, callback, name, ctx, msix_vec);
+ if (IS_ERR(vq))
+ return vq;
+
+ if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
+ return vq;
+
+ /* allocate per-vq irq if available and necessary */
+ snprintf(vp_dev->msix_names[msix_vec],
+ sizeof *vp_dev->msix_names,
+ "%s-%s", dev_name(&vp_dev->vdev.dev), name);
+
+ err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
+ vring_interrupt, IRQF_NO_AUTOEN,
+ vp_dev->msix_names[msix_vec], vq);
+ if (err) {
+ vp_del_vq(vq);
+ return ERR_PTR(err);
+ }
+
+ return vq;
+}
+
static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[], vq_callback_t *callbacks[],
const char * const names[], bool per_vq_vectors,
@@ -319,6 +354,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev,
unsigned nvqs,
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
u16 msix_vec;
int i, err, nvectors, allocated_vectors, queue_idx = 0;
+ struct virtqueue *vq;
vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
if (!vp_dev->vqs)
@@ -354,28 +390,14 @@ static int vp_find_vqs_msix(struct virtio_device *vdev,
unsigned nvqs,
msix_vec = allocated_vectors++;
else
msix_vec = VP_MSIX_VQ_VECTOR;
- vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
- ctx ? ctx[i] : false,
- msix_vec);
- if (IS_ERR(vqs[i])) {
- err = PTR_ERR(vqs[i]);
- goto error_find;
- }
- if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
- continue;
-
- /* allocate per-vq irq if available and necessary */
- snprintf(vp_dev->msix_names[msix_vec],
- sizeof *vp_dev->msix_names,
- "%s-%s",
- dev_name(&vp_dev->vdev.dev), names[i]);
- err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
- vring_interrupt, IRQF_NO_AUTOEN,
- vp_dev->msix_names[msix_vec],
- vqs[i]);
- if (err)
+ vq = vp_enable_vq_msix(vdev, queue_idx++, callbacks[i],
+ names[i], ctx ? ctx[i] : false, msix_vec);
+ if (IS_ERR(vq)) {
+ err = PTR_ERR(vq);
goto error_find;
+ }
+ vqs[i] = vq;
}
return 0;
--
2.31.0
Jason Wang
2022-Jan-19 06:04 UTC
[PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches
On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo at linux.alibaba.com> wrote:> > Add a vq_enable_vq_msix() function to enable a vq alone. > > Move irq's processing logic into vp_del_vq(), so that this function can > handle a vq's del operation independently. > > In the subsequent patches that supports queue reset, I have the > need to delete/enable a vq separately. > > Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com> > --- > drivers/virtio/virtio_pci_common.c | 86 +++++++++++++++++++----------- > 1 file changed, 54 insertions(+), 32 deletions(-) > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > index fdbde1db5ec5..5afe207ce28a 100644 > --- a/drivers/virtio/virtio_pci_common.c > +++ b/drivers/virtio/virtio_pci_common.c > @@ -248,6 +248,17 @@ static void vp_del_vq(struct virtqueue *vq) > struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index]; > unsigned long flags; > > + if (vp_dev->per_vq_vectors) { > + int v = vp_dev->vqs[vq->index]->msix_vector; > + > + if (v != VIRTIO_MSI_NO_VECTOR) { > + int irq = pci_irq_vector(vp_dev->pci_dev, v); > + > + irq_set_affinity_hint(irq, NULL); > + free_irq(irq, vq); > + } > + } > + > spin_lock_irqsave(&vp_dev->lock, flags); > list_del(&info->node); > spin_unlock_irqrestore(&vp_dev->lock, flags); > @@ -263,19 +274,9 @@ void vp_del_vqs(struct virtio_device *vdev) > struct virtqueue *vq, *n; > int i; > > - list_for_each_entry_safe(vq, n, &vdev->vqs, list) { > - if (vp_dev->per_vq_vectors) { > - int v = vp_dev->vqs[vq->index]->msix_vector; > - > - if (v != VIRTIO_MSI_NO_VECTOR) { > - int irq = pci_irq_vector(vp_dev->pci_dev, v); > - > - irq_set_affinity_hint(irq, NULL); > - free_irq(irq, vq); > - } > - } > + list_for_each_entry_safe(vq, n, &vdev->vqs, list) > vp_del_vq(vq); > - }I think we need a better subject of the patch and it's better to even split the patch into two. This part could be "move the per queue irq logic from vp_del_vqs to vp_del_vq".> + > vp_dev->per_vq_vectors = false; > > if (vp_dev->intx_enabled) { > @@ -310,6 +311,40 @@ void vp_del_vqs(struct virtio_device *vdev) > vp_dev->vqs = NULL; > } > > +static struct virtqueue *vp_enable_vq_msix(struct virtio_device *vdev, > + int queue_index, > + vq_callback_t *callback, > + const char * const name, > + bool ctx, > + u16 msix_vec) > +{ > + struct virtio_pci_device *vp_dev = to_vp_device(vdev); > + struct virtqueue *vq; > + int err; > + > + vq = vp_setup_vq(vdev, queue_index, callback, name, ctx, msix_vec); > + if (IS_ERR(vq)) > + return vq; > + > + if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR) > + return vq; > + > + /* allocate per-vq irq if available and necessary */ > + snprintf(vp_dev->msix_names[msix_vec], > + sizeof *vp_dev->msix_names, > + "%s-%s", dev_name(&vp_dev->vdev.dev), name); > + > + err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), > + vring_interrupt, IRQF_NO_AUTOEN, > + vp_dev->msix_names[msix_vec], vq); > + if (err) { > + vp_del_vq(vq); > + return ERR_PTR(err); > + } > + > + return vq; > +}And this deserves a separate patch as well. Thanks> + > static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > struct virtqueue *vqs[], vq_callback_t *callbacks[], > const char * const names[], bool per_vq_vectors, > @@ -319,6 +354,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > u16 msix_vec; > int i, err, nvectors, allocated_vectors, queue_idx = 0; > + struct virtqueue *vq; > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > if (!vp_dev->vqs) > @@ -354,28 +390,14 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > msix_vec = allocated_vectors++; > else > msix_vec = VP_MSIX_VQ_VECTOR; > - vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > - ctx ? ctx[i] : false, > - msix_vec); > - if (IS_ERR(vqs[i])) { > - err = PTR_ERR(vqs[i]); > - goto error_find; > - } > > - if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR) > - continue; > - > - /* allocate per-vq irq if available and necessary */ > - snprintf(vp_dev->msix_names[msix_vec], > - sizeof *vp_dev->msix_names, > - "%s-%s", > - dev_name(&vp_dev->vdev.dev), names[i]); > - err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), > - vring_interrupt, IRQF_NO_AUTOEN, > - vp_dev->msix_names[msix_vec], > - vqs[i]); > - if (err) > + vq = vp_enable_vq_msix(vdev, queue_idx++, callbacks[i], > + names[i], ctx ? ctx[i] : false, msix_vec); > + if (IS_ERR(vq)) { > + err = PTR_ERR(vq); > goto error_find; > + } > + vqs[i] = vq; > } > return 0; > > -- > 2.31.0 >