| Message ID | 20240424091533.86949-3-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | refactor the params of find_vqs() | expand |
On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > doesn't apply. And not one uses this. > > On the other side, that makes some trouble for us to refactor the > find_vqs() params. > > So I remove this support. > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > Acked-by: Jason Wang <jasowang@redhat.com> > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > Acked-by: Halil Pasic <pasic@linux.ibm.com> I don't mind, but this patchset is too big already. Why do we need to make this part of this patchset? > --- > arch/um/drivers/virtio_uml.c | 8 ++++---- > drivers/remoteproc/remoteproc_virtio.c | 11 ++++------- > drivers/s390/virtio/virtio_ccw.c | 8 ++++---- > drivers/virtio/virtio_mmio.c | 11 ++++------- > drivers/virtio/virtio_pci_common.c | 18 +++++++++--------- > drivers/virtio/virtio_vdpa.c | 11 ++++------- > include/linux/virtio_config.h | 2 +- > 7 files changed, 30 insertions(+), 39 deletions(-) > > diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c > index 8adca2000e51..773f9fc4d582 100644 > --- a/arch/um/drivers/virtio_uml.c > +++ b/arch/um/drivers/virtio_uml.c > @@ -1019,8 +1019,8 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs, > struct irq_affinity *desc) > { > struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); > - int i, queue_idx = 0, rc; > struct virtqueue *vq; > + int i, rc; > > /* not supported for now */ > if (WARN_ON(nvqs > 64)) > @@ -1032,11 +1032,11 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > for (i = 0; i < nvqs; ++i) { > if (!names[i]) { > - vqs[i] = NULL; > - continue; > + rc = -EINVAL; > + goto error_setup; > } > > - vqs[i] = vu_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > + vqs[i] = vu_setup_vq(vdev, i, callbacks[i], names[i], > ctx ? ctx[i] : false); > if (IS_ERR(vqs[i])) { > rc = PTR_ERR(vqs[i]); > diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c > index 25b66b113b69..7f58634fcc41 100644 > --- a/drivers/remoteproc/remoteproc_virtio.c > +++ b/drivers/remoteproc/remoteproc_virtio.c > @@ -119,9 +119,6 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, > if (id >= ARRAY_SIZE(rvdev->vring)) > return ERR_PTR(-EINVAL); > > - if (!name) > - return NULL; > - > /* Search allocated memory region by name */ > mem = rproc_find_carveout_by_name(rproc, "vdev%dvring%d", rvdev->index, > id); > @@ -187,15 +184,15 @@ static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > const bool * ctx, > struct irq_affinity *desc) > { > - int i, ret, queue_idx = 0; > + int i, ret; > > for (i = 0; i < nvqs; ++i) { > if (!names[i]) { > - vqs[i] = NULL; > - continue; > + ret = -EINVAL; > + goto error; > } > > - vqs[i] = rp_find_vq(vdev, queue_idx++, callbacks[i], names[i], > + vqs[i] = rp_find_vq(vdev, i, callbacks[i], names[i], > ctx ? ctx[i] : false); > if (IS_ERR(vqs[i])) { > ret = PTR_ERR(vqs[i]); > diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c > index d7569f395559..6cdd29952bc0 100644 > --- a/drivers/s390/virtio/virtio_ccw.c > +++ b/drivers/s390/virtio/virtio_ccw.c > @@ -696,7 +696,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, > { > struct virtio_ccw_device *vcdev = to_vc_device(vdev); > dma64_t *indicatorp = NULL; > - int ret, i, queue_idx = 0; > + int ret, i; > struct ccw1 *ccw; > > ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw), NULL); > @@ -705,11 +705,11 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > for (i = 0; i < nvqs; ++i) { > if (!names[i]) { > - vqs[i] = NULL; > - continue; > + ret = -EINVAL; > + goto out; > } > > - vqs[i] = virtio_ccw_setup_vq(vdev, queue_idx++, callbacks[i], > + vqs[i] = virtio_ccw_setup_vq(vdev, i, callbacks[i], > names[i], ctx ? ctx[i] : false, > ccw); > if (IS_ERR(vqs[i])) { > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > index 173596589c71..c3c8dd282952 100644 > --- a/drivers/virtio/virtio_mmio.c > +++ b/drivers/virtio/virtio_mmio.c > @@ -386,9 +386,6 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in > else > notify = vm_notify; > > - if (!name) > - return NULL; > - > /* Select the queue we're interested in */ > writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); > > @@ -496,7 +493,7 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > { > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > int irq = platform_get_irq(vm_dev->pdev, 0); > - int i, err, queue_idx = 0; > + int i, err; > > if (irq < 0) > return irq; > @@ -511,11 +508,11 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > for (i = 0; i < nvqs; ++i) { > if (!names[i]) { > - vqs[i] = NULL; > - continue; > + vm_del_vqs(vdev); > + return -EINVAL; > } > > - vqs[i] = vm_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > + vqs[i] = vm_setup_vq(vdev, i, callbacks[i], names[i], > ctx ? ctx[i] : false); > if (IS_ERR(vqs[i])) { > vm_del_vqs(vdev); > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > index b655fccaf773..eda71c6e87ee 100644 > --- a/drivers/virtio/virtio_pci_common.c > +++ b/drivers/virtio/virtio_pci_common.c > @@ -292,7 +292,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > u16 msix_vec; > - int i, err, nvectors, allocated_vectors, queue_idx = 0; > + int i, err, nvectors, allocated_vectors; > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > if (!vp_dev->vqs) > @@ -302,7 +302,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > /* Best option: one for change interrupt, one per vq. */ > nvectors = 1; > for (i = 0; i < nvqs; ++i) > - if (names[i] && callbacks[i]) > + if (callbacks[i]) > ++nvectors; > } else { > /* Second best: one for change, shared for all vqs. */ > @@ -318,8 +318,8 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > allocated_vectors = vp_dev->msix_used_vectors; > for (i = 0; i < nvqs; ++i) { > if (!names[i]) { > - vqs[i] = NULL; > - continue; > + err = -EINVAL; > + goto error_find; > } > > if (!callbacks[i]) > @@ -328,7 +328,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > msix_vec = allocated_vectors++; > else > msix_vec = VP_MSIX_VQ_VECTOR; > - vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > + vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], > ctx ? ctx[i] : false, > msix_vec); > if (IS_ERR(vqs[i])) { > @@ -363,7 +363,7 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > const char * const names[], const bool *ctx) > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > - int i, err, queue_idx = 0; > + int i, err; > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > if (!vp_dev->vqs) > @@ -378,10 +378,10 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > vp_dev->per_vq_vectors = false; > for (i = 0; i < nvqs; ++i) { > if (!names[i]) { > - vqs[i] = NULL; > - continue; > + err = -EINVAL; > + goto out_del_vqs; > } > - vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > + vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], > ctx ? ctx[i] : false, > VIRTIO_MSI_NO_VECTOR); > if (IS_ERR(vqs[i])) { > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c > index e803db0da307..e82cca24d6e6 100644 > --- a/drivers/virtio/virtio_vdpa.c > +++ b/drivers/virtio/virtio_vdpa.c > @@ -161,9 +161,6 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > bool may_reduce_num = true; > int err; > > - if (!name) > - return NULL; > - > if (index >= vdpa->nvqs) > return ERR_PTR(-ENOENT); > > @@ -370,7 +367,7 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > struct cpumask *masks; > struct vdpa_callback cb; > bool has_affinity = desc && ops->set_vq_affinity; > - int i, err, queue_idx = 0; > + int i, err; > > if (has_affinity) { > masks = create_affinity_masks(nvqs, desc ? desc : &default_affd); > @@ -380,11 +377,11 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > for (i = 0; i < nvqs; ++i) { > if (!names[i]) { > - vqs[i] = NULL; > - continue; > + err = -EINVAL; > + goto err_setup_vq; > } > > - vqs[i] = virtio_vdpa_setup_vq(vdev, queue_idx++, > + vqs[i] = virtio_vdpa_setup_vq(vdev, i, > callbacks[i], names[i], ctx ? > ctx[i] : false); > if (IS_ERR(vqs[i])) { > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h > index da9b271b54db..1c79cec258f4 100644 > --- a/include/linux/virtio_config.h > +++ b/include/linux/virtio_config.h > @@ -56,7 +56,7 @@ typedef void vq_callback_t(struct virtqueue *); > * callbacks: array of callbacks, for each virtqueue > * include a NULL entry for vqs that do not need a callback > * names: array of virtqueue names (mainly for debugging) > - * include a NULL entry for vqs unused by driver > + * MUST NOT be NULL Do not shout - just drop "include a NULL entry" text - not being NULL is default assumption for pointers. > * Returns 0 on success or error status > * @del_vqs: free virtqueues found by find_vqs(). > * @synchronize_cbs: synchronize with the virtqueue callbacks (optional) > -- > 2.32.0.3.g01195cf9f
On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > > doesn't apply. And not one uses this. > > > > On the other side, that makes some trouble for us to refactor the > > find_vqs() params. > > > > So I remove this support. > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > Acked-by: Jason Wang <jasowang@redhat.com> > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > I don't mind, but this patchset is too big already. > Why do we need to make this part of this patchset? If some the pointers of the names is NULL, then in the virtio ring, we will have a trouble to index from the arrays(names, callbacks...). Becasue that the idx of the vq is not the index of these arrays. If the names is [NULL, "rx", "tx"], the first vq is the "rx", but index of the vq is zero, but the index of the info of this vq inside the arrays is 1. So I do this commit. I will update the commit log in next version. Thanks. > > > > --- > > arch/um/drivers/virtio_uml.c | 8 ++++---- > > drivers/remoteproc/remoteproc_virtio.c | 11 ++++------- > > drivers/s390/virtio/virtio_ccw.c | 8 ++++---- > > drivers/virtio/virtio_mmio.c | 11 ++++------- > > drivers/virtio/virtio_pci_common.c | 18 +++++++++--------- > > drivers/virtio/virtio_vdpa.c | 11 ++++------- > > include/linux/virtio_config.h | 2 +- > > 7 files changed, 30 insertions(+), 39 deletions(-) > > > > diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c > > index 8adca2000e51..773f9fc4d582 100644 > > --- a/arch/um/drivers/virtio_uml.c > > +++ b/arch/um/drivers/virtio_uml.c > > @@ -1019,8 +1019,8 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > struct irq_affinity *desc) > > { > > struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); > > - int i, queue_idx = 0, rc; > > struct virtqueue *vq; > > + int i, rc; > > > > /* not supported for now */ > > if (WARN_ON(nvqs > 64)) > > @@ -1032,11 +1032,11 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > - vqs[i] = NULL; > > - continue; > > + rc = -EINVAL; > > + goto error_setup; > > } > > > > - vqs[i] = vu_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > > + vqs[i] = vu_setup_vq(vdev, i, callbacks[i], names[i], > > ctx ? ctx[i] : false); > > if (IS_ERR(vqs[i])) { > > rc = PTR_ERR(vqs[i]); > > diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c > > index 25b66b113b69..7f58634fcc41 100644 > > --- a/drivers/remoteproc/remoteproc_virtio.c > > +++ b/drivers/remoteproc/remoteproc_virtio.c > > @@ -119,9 +119,6 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, > > if (id >= ARRAY_SIZE(rvdev->vring)) > > return ERR_PTR(-EINVAL); > > > > - if (!name) > > - return NULL; > > - > > /* Search allocated memory region by name */ > > mem = rproc_find_carveout_by_name(rproc, "vdev%dvring%d", rvdev->index, > > id); > > @@ -187,15 +184,15 @@ static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > const bool * ctx, > > struct irq_affinity *desc) > > { > > - int i, ret, queue_idx = 0; > > + int i, ret; > > > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > - vqs[i] = NULL; > > - continue; > > + ret = -EINVAL; > > + goto error; > > } > > > > - vqs[i] = rp_find_vq(vdev, queue_idx++, callbacks[i], names[i], > > + vqs[i] = rp_find_vq(vdev, i, callbacks[i], names[i], > > ctx ? ctx[i] : false); > > if (IS_ERR(vqs[i])) { > > ret = PTR_ERR(vqs[i]); > > diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c > > index d7569f395559..6cdd29952bc0 100644 > > --- a/drivers/s390/virtio/virtio_ccw.c > > +++ b/drivers/s390/virtio/virtio_ccw.c > > @@ -696,7 +696,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > { > > struct virtio_ccw_device *vcdev = to_vc_device(vdev); > > dma64_t *indicatorp = NULL; > > - int ret, i, queue_idx = 0; > > + int ret, i; > > struct ccw1 *ccw; > > > > ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw), NULL); > > @@ -705,11 +705,11 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > - vqs[i] = NULL; > > - continue; > > + ret = -EINVAL; > > + goto out; > > } > > > > - vqs[i] = virtio_ccw_setup_vq(vdev, queue_idx++, callbacks[i], > > + vqs[i] = virtio_ccw_setup_vq(vdev, i, callbacks[i], > > names[i], ctx ? ctx[i] : false, > > ccw); > > if (IS_ERR(vqs[i])) { > > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > > index 173596589c71..c3c8dd282952 100644 > > --- a/drivers/virtio/virtio_mmio.c > > +++ b/drivers/virtio/virtio_mmio.c > > @@ -386,9 +386,6 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in > > else > > notify = vm_notify; > > > > - if (!name) > > - return NULL; > > - > > /* Select the queue we're interested in */ > > writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); > > > > @@ -496,7 +493,7 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > { > > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > > int irq = platform_get_irq(vm_dev->pdev, 0); > > - int i, err, queue_idx = 0; > > + int i, err; > > > > if (irq < 0) > > return irq; > > @@ -511,11 +508,11 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > - vqs[i] = NULL; > > - continue; > > + vm_del_vqs(vdev); > > + return -EINVAL; > > } > > > > - vqs[i] = vm_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > > + vqs[i] = vm_setup_vq(vdev, i, callbacks[i], names[i], > > ctx ? ctx[i] : false); > > if (IS_ERR(vqs[i])) { > > vm_del_vqs(vdev); > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > > index b655fccaf773..eda71c6e87ee 100644 > > --- a/drivers/virtio/virtio_pci_common.c > > +++ b/drivers/virtio/virtio_pci_common.c > > @@ -292,7 +292,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > u16 msix_vec; > > - int i, err, nvectors, allocated_vectors, queue_idx = 0; > > + int i, err, nvectors, allocated_vectors; > > > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > > if (!vp_dev->vqs) > > @@ -302,7 +302,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > /* Best option: one for change interrupt, one per vq. */ > > nvectors = 1; > > for (i = 0; i < nvqs; ++i) > > - if (names[i] && callbacks[i]) > > + if (callbacks[i]) > > ++nvectors; > > } else { > > /* Second best: one for change, shared for all vqs. */ > > @@ -318,8 +318,8 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > allocated_vectors = vp_dev->msix_used_vectors; > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > - vqs[i] = NULL; > > - continue; > > + err = -EINVAL; > > + goto error_find; > > } > > > > if (!callbacks[i]) > > @@ -328,7 +328,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > msix_vec = allocated_vectors++; > > else > > msix_vec = VP_MSIX_VQ_VECTOR; > > - vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > > + vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], > > ctx ? ctx[i] : false, > > msix_vec); > > if (IS_ERR(vqs[i])) { > > @@ -363,7 +363,7 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > > const char * const names[], const bool *ctx) > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > - int i, err, queue_idx = 0; > > + int i, err; > > > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > > if (!vp_dev->vqs) > > @@ -378,10 +378,10 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > > vp_dev->per_vq_vectors = false; > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > - vqs[i] = NULL; > > - continue; > > + err = -EINVAL; > > + goto out_del_vqs; > > } > > - vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i], > > + vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], > > ctx ? ctx[i] : false, > > VIRTIO_MSI_NO_VECTOR); > > if (IS_ERR(vqs[i])) { > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c > > index e803db0da307..e82cca24d6e6 100644 > > --- a/drivers/virtio/virtio_vdpa.c > > +++ b/drivers/virtio/virtio_vdpa.c > > @@ -161,9 +161,6 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > > bool may_reduce_num = true; > > int err; > > > > - if (!name) > > - return NULL; > > - > > if (index >= vdpa->nvqs) > > return ERR_PTR(-ENOENT); > > > > @@ -370,7 +367,7 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > struct cpumask *masks; > > struct vdpa_callback cb; > > bool has_affinity = desc && ops->set_vq_affinity; > > - int i, err, queue_idx = 0; > > + int i, err; > > > > if (has_affinity) { > > masks = create_affinity_masks(nvqs, desc ? desc : &default_affd); > > @@ -380,11 +377,11 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > - vqs[i] = NULL; > > - continue; > > + err = -EINVAL; > > + goto err_setup_vq; > > } > > > > - vqs[i] = virtio_vdpa_setup_vq(vdev, queue_idx++, > > + vqs[i] = virtio_vdpa_setup_vq(vdev, i, > > callbacks[i], names[i], ctx ? > > ctx[i] : false); > > if (IS_ERR(vqs[i])) { > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h > > index da9b271b54db..1c79cec258f4 100644 > > --- a/include/linux/virtio_config.h > > +++ b/include/linux/virtio_config.h > > @@ -56,7 +56,7 @@ typedef void vq_callback_t(struct virtqueue *); > > * callbacks: array of callbacks, for each virtqueue > > * include a NULL entry for vqs that do not need a callback > > * names: array of virtqueue names (mainly for debugging) > > - * include a NULL entry for vqs unused by driver > > + * MUST NOT be NULL > > Do not shout - just drop "include a NULL entry" text - not being NULL > is default assumption for pointers. > > > * Returns 0 on success or error status > > * @del_vqs: free virtqueues found by find_vqs(). > > * @synchronize_cbs: synchronize with the virtqueue callbacks (optional) > > -- > > 2.32.0.3.g01195cf9f >
On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > > > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > > > doesn't apply. And not one uses this. > > > > > > On the other side, that makes some trouble for us to refactor the > > > find_vqs() params. > > > > > > So I remove this support. > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > I don't mind, but this patchset is too big already. > > Why do we need to make this part of this patchset? > > > If some the pointers of the names is NULL, then in the virtio ring, > we will have a trouble to index from the arrays(names, callbacks...). > Becasue that the idx of the vq is not the index of these arrays. > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but index of the > vq is zero, but the index of the info of this vq inside the arrays is 1. Ah. So actually, it used to work. What this should refer to is commit ddbeac07a39a81d82331a312d0578fab94fccbf1 Author: Wei Wang <wei.w.wang@intel.com> Date: Fri Dec 28 10:26:25 2018 +0800 virtio_pci: use queue idx instead of array idx to set up the vq When find_vqs, there will be no vq[i] allocation if its corresponding names[i] is NULL. For example, the caller may pass in names[i] (i=4) with names[2] being NULL because the related feature bit is turned off, so technically there are 3 queues on the device, and name[4] should correspond to the 3rd queue on the device. So we use queue_idx as the queue index, which is increased only when the queue exists. Signed-off-by: Wei Wang <wei.w.wang@intel.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Which made it so setting names NULL actually does not reserve a vq. But I worry about non pci transports - there's a chance they used a different index with the balloon. Did you test some of these?
On Thu, 20 Jun 2024 05:01:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > > > > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > > > > doesn't apply. And not one uses this. > > > > > > > > On the other side, that makes some trouble for us to refactor the > > > > find_vqs() params. > > > > > > > > So I remove this support. > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > > > > I don't mind, but this patchset is too big already. > > > Why do we need to make this part of this patchset? > > > > > > If some the pointers of the names is NULL, then in the virtio ring, > > we will have a trouble to index from the arrays(names, callbacks...). > > Becasue that the idx of the vq is not the index of these arrays. > > > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but index of the > > vq is zero, but the index of the info of this vq inside the arrays is 1. > > > Ah. So actually, it used to work. > > What this should refer to is > > commit ddbeac07a39a81d82331a312d0578fab94fccbf1 > Author: Wei Wang <wei.w.wang@intel.com> > Date: Fri Dec 28 10:26:25 2018 +0800 > > virtio_pci: use queue idx instead of array idx to set up the vq > > When find_vqs, there will be no vq[i] allocation if its corresponding > names[i] is NULL. For example, the caller may pass in names[i] (i=4) > with names[2] being NULL because the related feature bit is turned off, > so technically there are 3 queues on the device, and name[4] should > correspond to the 3rd queue on the device. > > So we use queue_idx as the queue index, which is increased only when the > queue exists. > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > That just work for PCI. The trouble I described is that we can not index in the virtio ring. In virtio ring, we may like to use the vq.index that do not increase for the NULL. > > Which made it so setting names NULL actually does not reserve a vq. > > But I worry about non pci transports - there's a chance they used > a different index with the balloon. Did you test some of these? > Balloon is out of spec. The vq.index does not increase for the name NULL. So the Balloon use the continuous id. That is out of spec. That does not matter for this patchset. The name NULL is always skipped. Thanks. > -- > MST >
On Thu, Jun 20, 2024 at 05:04:53PM +0800, Xuan Zhuo wrote: > On Thu, 20 Jun 2024 05:01:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > > > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > > > > > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > > > > > doesn't apply. And not one uses this. > > > > > > > > > > On the other side, that makes some trouble for us to refactor the > > > > > find_vqs() params. > > > > > > > > > > So I remove this support. > > > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > > > > > > > I don't mind, but this patchset is too big already. > > > > Why do we need to make this part of this patchset? > > > > > > > > > If some the pointers of the names is NULL, then in the virtio ring, > > > we will have a trouble to index from the arrays(names, callbacks...). > > > Becasue that the idx of the vq is not the index of these arrays. > > > > > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but index of the > > > vq is zero, but the index of the info of this vq inside the arrays is 1. > > > > > > Ah. So actually, it used to work. > > > > What this should refer to is > > > > commit ddbeac07a39a81d82331a312d0578fab94fccbf1 > > Author: Wei Wang <wei.w.wang@intel.com> > > Date: Fri Dec 28 10:26:25 2018 +0800 > > > > virtio_pci: use queue idx instead of array idx to set up the vq > > > > When find_vqs, there will be no vq[i] allocation if its corresponding > > names[i] is NULL. For example, the caller may pass in names[i] (i=4) > > with names[2] being NULL because the related feature bit is turned off, > > so technically there are 3 queues on the device, and name[4] should > > correspond to the 3rd queue on the device. > > > > So we use queue_idx as the queue index, which is increased only when the > > queue exists. > > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > That just work for PCI. > > The trouble I described is that we can not index in the virtio ring. > > In virtio ring, we may like to use the vq.index that do not increase > for the NULL. > > > > > > Which made it so setting names NULL actually does not reserve a vq. > > > > But I worry about non pci transports - there's a chance they used > > a different index with the balloon. Did you test some of these? > > > > Balloon is out of spec. > > The vq.index does not increase for the name NULL. So the Balloon use the > continuous id. That is out of spec. I see. And apparently the QEMU implementation is out of spec, too, so they work fine. And STATS is always on in QEMU. That change by Wei broke the theoretical config which has !STATS but does have FREE_PAGE. We never noticed - not many people ever bothered with FREE_PAGE. However QEMU really is broken in a weird way. In particular if it exposes STATS but driver does not configure STATS then QEMU still has the stats vq. Things will break then. In short, it's a mess, and it needs thought. At this point I suggest we keep the ability to set names to NULL in case we want to just revert Wei's patch. > That does not matter for this patchset. > The name NULL is always skipped. > > Thanks. Let's keep this patchset as small as possible. Keep the existing functionality, we'll do cleanups later. > > -- > > MST > >
On Thu, 20 Jun 2024 06:01:54 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jun 20, 2024 at 05:04:53PM +0800, Xuan Zhuo wrote: > > On Thu, 20 Jun 2024 05:01:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > > > > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > > > > > > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > > > > > > doesn't apply. And not one uses this. > > > > > > > > > > > > On the other side, that makes some trouble for us to refactor the > > > > > > find_vqs() params. > > > > > > > > > > > > So I remove this support. > > > > > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > > > > > > > > > > I don't mind, but this patchset is too big already. > > > > > Why do we need to make this part of this patchset? > > > > > > > > > > > > If some the pointers of the names is NULL, then in the virtio ring, > > > > we will have a trouble to index from the arrays(names, callbacks...). > > > > Becasue that the idx of the vq is not the index of these arrays. > > > > > > > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but index of the > > > > vq is zero, but the index of the info of this vq inside the arrays is 1. > > > > > > > > > Ah. So actually, it used to work. > > > > > > What this should refer to is > > > > > > commit ddbeac07a39a81d82331a312d0578fab94fccbf1 > > > Author: Wei Wang <wei.w.wang@intel.com> > > > Date: Fri Dec 28 10:26:25 2018 +0800 > > > > > > virtio_pci: use queue idx instead of array idx to set up the vq > > > > > > When find_vqs, there will be no vq[i] allocation if its corresponding > > > names[i] is NULL. For example, the caller may pass in names[i] (i=4) > > > with names[2] being NULL because the related feature bit is turned off, > > > so technically there are 3 queues on the device, and name[4] should > > > correspond to the 3rd queue on the device. > > > > > > So we use queue_idx as the queue index, which is increased only when the > > > queue exists. > > > > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > > That just work for PCI. > > > > The trouble I described is that we can not index in the virtio ring. > > > > In virtio ring, we may like to use the vq.index that do not increase > > for the NULL. > > > > > > > > > > Which made it so setting names NULL actually does not reserve a vq. > > > > > > But I worry about non pci transports - there's a chance they used > > > a different index with the balloon. Did you test some of these? > > > > > > > Balloon is out of spec. > > > > The vq.index does not increase for the name NULL. So the Balloon use the > > continuous id. That is out of spec. > > > I see. And apparently the QEMU implementation is out of spec, too, > so they work fine. And STATS is always on in QEMU. > > That change by Wei broke the theoretical config which has > !STATS but does have FREE_PAGE. We never noticed - not many people > ever bothered with FREE_PAGE. > > However QEMU really is broken in a weird way. > In particular if it exposes STATS but driver does not > configure STATS then QEMU still has the stats vq. > Things will break then. > > > In short, it's a mess, and it needs thought. > At this point I suggest we keep the ability to set > names to NULL in case we want to just revert Wei's patch. > > > > > That does not matter for this patchset. > > The name NULL is always skipped. > > > > Thanks. > > > Let's keep this patchset as small as possible. > Keep the existing functionality, we'll do cleanups > later. I am ok. But we need a idx to index the info of the vq. How about a new element "cfg_idx" to virtio_vq_config. struct virtio_vq_config { unsigned int nvqs; -> unsigned int cfg_idx; struct virtqueue **vqs; vq_callback_t **callbacks; const char **names; const bool *ctx; struct irq_affinity *desc; }; That is setted by transport. The virtio ring can use this to index the info of the vq. Then the #1 #2 commits can be dropped. Thanks. > > > > > -- > > > MST > > > >
On Thu, Jun 20, 2024 at 06:49:08PM +0800, Xuan Zhuo wrote: > On Thu, 20 Jun 2024 06:01:54 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Thu, Jun 20, 2024 at 05:04:53PM +0800, Xuan Zhuo wrote: > > > On Thu, 20 Jun 2024 05:01:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > > > > > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > > > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > > > > > > > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > > > > > > > doesn't apply. And not one uses this. > > > > > > > > > > > > > > On the other side, that makes some trouble for us to refactor the > > > > > > > find_vqs() params. > > > > > > > > > > > > > > So I remove this support. > > > > > > > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > > > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > > > > > > > > > > > > > I don't mind, but this patchset is too big already. > > > > > > Why do we need to make this part of this patchset? > > > > > > > > > > > > > > > If some the pointers of the names is NULL, then in the virtio ring, > > > > > we will have a trouble to index from the arrays(names, callbacks...). > > > > > Becasue that the idx of the vq is not the index of these arrays. > > > > > > > > > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but index of the > > > > > vq is zero, but the index of the info of this vq inside the arrays is 1. > > > > > > > > > > > > Ah. So actually, it used to work. > > > > > > > > What this should refer to is > > > > > > > > commit ddbeac07a39a81d82331a312d0578fab94fccbf1 > > > > Author: Wei Wang <wei.w.wang@intel.com> > > > > Date: Fri Dec 28 10:26:25 2018 +0800 > > > > > > > > virtio_pci: use queue idx instead of array idx to set up the vq > > > > > > > > When find_vqs, there will be no vq[i] allocation if its corresponding > > > > names[i] is NULL. For example, the caller may pass in names[i] (i=4) > > > > with names[2] being NULL because the related feature bit is turned off, > > > > so technically there are 3 queues on the device, and name[4] should > > > > correspond to the 3rd queue on the device. > > > > > > > > So we use queue_idx as the queue index, which is increased only when the > > > > queue exists. > > > > > > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > > > > > That just work for PCI. > > > > > > The trouble I described is that we can not index in the virtio ring. > > > > > > In virtio ring, we may like to use the vq.index that do not increase > > > for the NULL. > > > > > > > > > > > > > > Which made it so setting names NULL actually does not reserve a vq. > > > > > > > > But I worry about non pci transports - there's a chance they used > > > > a different index with the balloon. Did you test some of these? > > > > > > > > > > Balloon is out of spec. > > > > > > The vq.index does not increase for the name NULL. So the Balloon use the > > > continuous id. That is out of spec. > > > > > > I see. And apparently the QEMU implementation is out of spec, too, > > so they work fine. And STATS is always on in QEMU. > > > > That change by Wei broke the theoretical config which has > > !STATS but does have FREE_PAGE. We never noticed - not many people > > ever bothered with FREE_PAGE. > > > > However QEMU really is broken in a weird way. > > In particular if it exposes STATS but driver does not > > configure STATS then QEMU still has the stats vq. > > Things will break then. > > > > > > In short, it's a mess, and it needs thought. > > At this point I suggest we keep the ability to set > > names to NULL in case we want to just revert Wei's patch. > > > > > > > > > That does not matter for this patchset. > > > The name NULL is always skipped. > > > > > > Thanks. > > > > > > Let's keep this patchset as small as possible. > > Keep the existing functionality, we'll do cleanups > > later. > > > I am ok. But we need a idx to index the info of the vq. > > How about a new element "cfg_idx" to virtio_vq_config. > > struct virtio_vq_config { > unsigned int nvqs; > -> unsigned int cfg_idx; > > struct virtqueue **vqs; > vq_callback_t **callbacks; > const char **names; > const bool *ctx; > struct irq_affinity *desc; > }; > > > That is setted by transport. The virtio ring can use this to index the info > of the vq. Then the #1 #2 commits can be dropped. > > > Thanks. > I'm not sure why you need this in the API. Actually now I think about it, the whole struct is weird. I think nvqs etc should be outside the struct. All arrays are the same size, why not: struct virtio_vq_config { vq_callback_t callback; const char *name; const bool ctx; }; And find_vqs should get an array of these. Leave the rest of params alone. > > > > > > > > > -- > > > > MST > > > > > >
On Thu, 20 Jun 2024 07:02:42 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jun 20, 2024 at 06:49:08PM +0800, Xuan Zhuo wrote: > > On Thu, 20 Jun 2024 06:01:54 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Thu, Jun 20, 2024 at 05:04:53PM +0800, Xuan Zhuo wrote: > > > > On Thu, 20 Jun 2024 05:01:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > > > > > > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > > > > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > > > > > > > > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > > > > > > > > doesn't apply. And not one uses this. > > > > > > > > > > > > > > > > On the other side, that makes some trouble for us to refactor the > > > > > > > > find_vqs() params. > > > > > > > > > > > > > > > > So I remove this support. > > > > > > > > > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > > > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > > > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > > > > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > > > > > > > > > > > > > > > > I don't mind, but this patchset is too big already. > > > > > > > Why do we need to make this part of this patchset? > > > > > > > > > > > > > > > > > > If some the pointers of the names is NULL, then in the virtio ring, > > > > > > we will have a trouble to index from the arrays(names, callbacks...). > > > > > > Becasue that the idx of the vq is not the index of these arrays. > > > > > > > > > > > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but index of the > > > > > > vq is zero, but the index of the info of this vq inside the arrays is 1. > > > > > > > > > > > > > > > Ah. So actually, it used to work. > > > > > > > > > > What this should refer to is > > > > > > > > > > commit ddbeac07a39a81d82331a312d0578fab94fccbf1 > > > > > Author: Wei Wang <wei.w.wang@intel.com> > > > > > Date: Fri Dec 28 10:26:25 2018 +0800 > > > > > > > > > > virtio_pci: use queue idx instead of array idx to set up the vq > > > > > > > > > > When find_vqs, there will be no vq[i] allocation if its corresponding > > > > > names[i] is NULL. For example, the caller may pass in names[i] (i=4) > > > > > with names[2] being NULL because the related feature bit is turned off, > > > > > so technically there are 3 queues on the device, and name[4] should > > > > > correspond to the 3rd queue on the device. > > > > > > > > > > So we use queue_idx as the queue index, which is increased only when the > > > > > queue exists. > > > > > > > > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > > > > > > > > That just work for PCI. > > > > > > > > The trouble I described is that we can not index in the virtio ring. > > > > > > > > In virtio ring, we may like to use the vq.index that do not increase > > > > for the NULL. > > > > > > > > > > > > > > > > > > Which made it so setting names NULL actually does not reserve a vq. > > > > > > > > > > But I worry about non pci transports - there's a chance they used > > > > > a different index with the balloon. Did you test some of these? > > > > > > > > > > > > > Balloon is out of spec. > > > > > > > > The vq.index does not increase for the name NULL. So the Balloon use the > > > > continuous id. That is out of spec. > > > > > > > > > I see. And apparently the QEMU implementation is out of spec, too, > > > so they work fine. And STATS is always on in QEMU. > > > > > > That change by Wei broke the theoretical config which has > > > !STATS but does have FREE_PAGE. We never noticed - not many people > > > ever bothered with FREE_PAGE. > > > > > > However QEMU really is broken in a weird way. > > > In particular if it exposes STATS but driver does not > > > configure STATS then QEMU still has the stats vq. > > > Things will break then. > > > > > > > > > In short, it's a mess, and it needs thought. > > > At this point I suggest we keep the ability to set > > > names to NULL in case we want to just revert Wei's patch. > > > > > > > > > > > > > That does not matter for this patchset. > > > > The name NULL is always skipped. > > > > > > > > Thanks. > > > > > > > > > Let's keep this patchset as small as possible. > > > Keep the existing functionality, we'll do cleanups > > > later. > > > > > > I am ok. But we need a idx to index the info of the vq. > > > > How about a new element "cfg_idx" to virtio_vq_config. > > > > struct virtio_vq_config { > > unsigned int nvqs; > > -> unsigned int cfg_idx; > > > > struct virtqueue **vqs; > > vq_callback_t **callbacks; > > const char **names; > > const bool *ctx; > > struct irq_affinity *desc; > > }; > > > > > > That is setted by transport. The virtio ring can use this to index the info > > of the vq. Then the #1 #2 commits can be dropped. > > > > > > Thanks. > > > > I'm not sure why you need this in the API. > > > Actually now I think about it, the whole struct is weird. > I think nvqs etc should be outside the struct. > All arrays are the same size, why not: > > struct virtio_vq_config { > vq_callback_t callback; > const char *name; > const bool ctx; > }; > > And find_vqs should get an array of these. > Leave the rest of params alone. YES, this is great. I thought about this. The trouble is that all the callers need to be changed. That are too many. Thanks. > > > > > > > > > > > > > > > -- > > > > > MST > > > > > > > > >
On Thu, Jun 20, 2024 at 07:04:08PM +0800, Xuan Zhuo wrote: > On Thu, 20 Jun 2024 07:02:42 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Thu, Jun 20, 2024 at 06:49:08PM +0800, Xuan Zhuo wrote: > > > On Thu, 20 Jun 2024 06:01:54 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > On Thu, Jun 20, 2024 at 05:04:53PM +0800, Xuan Zhuo wrote: > > > > > On Thu, 20 Jun 2024 05:01:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > > > > > > > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > > > > > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced this > > > > > > > > > support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic > > > > > > > > > doesn't apply. And not one uses this. > > > > > > > > > > > > > > > > > > On the other side, that makes some trouble for us to refactor the > > > > > > > > > find_vqs() params. > > > > > > > > > > > > > > > > > > So I remove this support. > > > > > > > > > > > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > > > > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > > > > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > > > > > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > > > > > > > > > > > > > > > > > > > I don't mind, but this patchset is too big already. > > > > > > > > Why do we need to make this part of this patchset? > > > > > > > > > > > > > > > > > > > > > If some the pointers of the names is NULL, then in the virtio ring, > > > > > > > we will have a trouble to index from the arrays(names, callbacks...). > > > > > > > Becasue that the idx of the vq is not the index of these arrays. > > > > > > > > > > > > > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but index of the > > > > > > > vq is zero, but the index of the info of this vq inside the arrays is 1. > > > > > > > > > > > > > > > > > > Ah. So actually, it used to work. > > > > > > > > > > > > What this should refer to is > > > > > > > > > > > > commit ddbeac07a39a81d82331a312d0578fab94fccbf1 > > > > > > Author: Wei Wang <wei.w.wang@intel.com> > > > > > > Date: Fri Dec 28 10:26:25 2018 +0800 > > > > > > > > > > > > virtio_pci: use queue idx instead of array idx to set up the vq > > > > > > > > > > > > When find_vqs, there will be no vq[i] allocation if its corresponding > > > > > > names[i] is NULL. For example, the caller may pass in names[i] (i=4) > > > > > > with names[2] being NULL because the related feature bit is turned off, > > > > > > so technically there are 3 queues on the device, and name[4] should > > > > > > correspond to the 3rd queue on the device. > > > > > > > > > > > > So we use queue_idx as the queue index, which is increased only when the > > > > > > queue exists. > > > > > > > > > > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > > > > > > > > > > > That just work for PCI. > > > > > > > > > > The trouble I described is that we can not index in the virtio ring. > > > > > > > > > > In virtio ring, we may like to use the vq.index that do not increase > > > > > for the NULL. > > > > > > > > > > > > > > > > > > > > > > Which made it so setting names NULL actually does not reserve a vq. > > > > > > > > > > > > But I worry about non pci transports - there's a chance they used > > > > > > a different index with the balloon. Did you test some of these? > > > > > > > > > > > > > > > > Balloon is out of spec. > > > > > > > > > > The vq.index does not increase for the name NULL. So the Balloon use the > > > > > continuous id. That is out of spec. > > > > > > > > > > > > I see. And apparently the QEMU implementation is out of spec, too, > > > > so they work fine. And STATS is always on in QEMU. > > > > > > > > That change by Wei broke the theoretical config which has > > > > !STATS but does have FREE_PAGE. We never noticed - not many people > > > > ever bothered with FREE_PAGE. > > > > > > > > However QEMU really is broken in a weird way. > > > > In particular if it exposes STATS but driver does not > > > > configure STATS then QEMU still has the stats vq. > > > > Things will break then. > > > > > > > > > > > > In short, it's a mess, and it needs thought. > > > > At this point I suggest we keep the ability to set > > > > names to NULL in case we want to just revert Wei's patch. > > > > > > > > > > > > > > > > > That does not matter for this patchset. > > > > > The name NULL is always skipped. > > > > > > > > > > Thanks. > > > > > > > > > > > > Let's keep this patchset as small as possible. > > > > Keep the existing functionality, we'll do cleanups > > > > later. > > > > > > > > > I am ok. But we need a idx to index the info of the vq. > > > > > > How about a new element "cfg_idx" to virtio_vq_config. > > > > > > struct virtio_vq_config { > > > unsigned int nvqs; > > > -> unsigned int cfg_idx; > > > > > > struct virtqueue **vqs; > > > vq_callback_t **callbacks; > > > const char **names; > > > const bool *ctx; > > > struct irq_affinity *desc; > > > }; > > > > > > > > > That is setted by transport. The virtio ring can use this to index the info > > > of the vq. Then the #1 #2 commits can be dropped. > > > > > > > > > Thanks. > > > > > > > I'm not sure why you need this in the API. > > > > > > Actually now I think about it, the whole struct is weird. > > I think nvqs etc should be outside the struct. > > All arrays are the same size, why not: > > > > struct virtio_vq_config { > > vq_callback_t callback; > > const char *name; > > const bool ctx; > > }; > > > > And find_vqs should get an array of these. > > Leave the rest of params alone. > > > YES, this is great. > > I thought about this. > > The trouble is that all the callers need to be changed. > That are too many. > > Thanks. > Not too many. > > > > > > > > > > > > > > > > > > > > > -- > > > > > > MST > > > > > > > > > > > >
On Thursday, June 20, 2024 5:01 PM, Michael S. Tsirkin wrote: > On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> > wrote: > > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced > > > > this support. Multiqueue virtio-net use 2N as ctrl vq finally, so > > > > the logic doesn't apply. And not one uses this. > > > > > > > > On the other side, that makes some trouble for us to refactor the > > > > find_vqs() params. > > > > > > > > So I remove this support. > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > > > > I don't mind, but this patchset is too big already. > > > Why do we need to make this part of this patchset? > > > > > > If some the pointers of the names is NULL, then in the virtio ring, we > > will have a trouble to index from the arrays(names, callbacks...). > > Becasue that the idx of the vq is not the index of these arrays. > > > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but > > index of the vq is zero, but the index of the info of this vq inside the arrays is > 1. > > > Ah. So actually, it used to work. > > What this should refer to is > > commit ddbeac07a39a81d82331a312d0578fab94fccbf1 > Author: Wei Wang <wei.w.wang@intel.com> > Date: Fri Dec 28 10:26:25 2018 +0800 > > virtio_pci: use queue idx instead of array idx to set up the vq > > When find_vqs, there will be no vq[i] allocation if its corresponding > names[i] is NULL. For example, the caller may pass in names[i] (i=4) > with names[2] being NULL because the related feature bit is turned off, > so technically there are 3 queues on the device, and name[4] should > correspond to the 3rd queue on the device. > > So we use queue_idx as the queue index, which is increased only when the > queue exists. > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > The approach was taken to prevent the creation (by the device) of unnecessary queues that would remain unused when the feature bit is turned off. Otherwise, the device is required to create all conditional queues regardless of their necessity. > > Which made it so setting names NULL actually does not reserve a vq. If there is a need for an explicit queue reservation, it might be feasible to assign a specific name to the queue(e.g. "reserved")? This will require the device to have the reserved queue added. > > But I worry about non pci transports - there's a chance they used a different > index with the balloon. Did you test some of these? > > -- > MST >
On Sat, Jun 22, 2024 at 06:07:35AM +0000, Wang, Wei W wrote: > On Thursday, June 20, 2024 5:01 PM, Michael S. Tsirkin wrote: > > On Thu, Jun 20, 2024 at 04:39:38PM +0800, Xuan Zhuo wrote: > > > On Thu, 20 Jun 2024 04:02:45 -0400, "Michael S. Tsirkin" <mst@redhat.com> > > wrote: > > > > On Wed, Apr 24, 2024 at 05:15:29PM +0800, Xuan Zhuo wrote: > > > > > commit 6457f126c888 ("virtio: support reserved vqs") introduced > > > > > this support. Multiqueue virtio-net use 2N as ctrl vq finally, so > > > > > the logic doesn't apply. And not one uses this. > > > > > > > > > > On the other side, that makes some trouble for us to refactor the > > > > > find_vqs() params. > > > > > > > > > > So I remove this support. > > > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > > > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > > > > > > > > > > > I don't mind, but this patchset is too big already. > > > > Why do we need to make this part of this patchset? > > > > > > > > > If some the pointers of the names is NULL, then in the virtio ring, we > > > will have a trouble to index from the arrays(names, callbacks...). > > > Becasue that the idx of the vq is not the index of these arrays. > > > > > > If the names is [NULL, "rx", "tx"], the first vq is the "rx", but > > > index of the vq is zero, but the index of the info of this vq inside the arrays is > > 1. > > > > > > Ah. So actually, it used to work. > > > > What this should refer to is > > > > commit ddbeac07a39a81d82331a312d0578fab94fccbf1 > > Author: Wei Wang <wei.w.wang@intel.com> > > Date: Fri Dec 28 10:26:25 2018 +0800 > > > > virtio_pci: use queue idx instead of array idx to set up the vq > > > > When find_vqs, there will be no vq[i] allocation if its corresponding > > names[i] is NULL. For example, the caller may pass in names[i] (i=4) > > with names[2] being NULL because the related feature bit is turned off, > > so technically there are 3 queues on the device, and name[4] should > > correspond to the 3rd queue on the device. > > > > So we use queue_idx as the queue index, which is increased only when the > > queue exists. > > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > The approach was taken to prevent the creation (by the device) of unnecessary > queues that would remain unused when the feature bit is turned off. Otherwise, > the device is required to create all conditional queues regardless of their necessity. > > > > > Which made it so setting names NULL actually does not reserve a vq. > > If there is a need for an explicit queue reservation, it might be feasible to assign > a specific name to the queue(e.g. "reserved")? > This will require the device to have the reserved queue added. That's quite a hack, NULL as a special value is much more idiomatic. Given driver and qemu are both non spec compliant but *in splightly different ways* I think we should just fix both the driver and qemu to be spec compliant. > > > > But I worry about non pci transports - there's a chance they used a different > > index with the balloon. Did you test some of these? > > > > -- > > MST > >
diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c index 8adca2000e51..773f9fc4d582 100644 --- a/arch/um/drivers/virtio_uml.c +++ b/arch/um/drivers/virtio_uml.c @@ -1019,8 +1019,8 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs, struct irq_affinity *desc) { struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); - int i, queue_idx = 0, rc; struct virtqueue *vq; + int i, rc; /* not supported for now */ if (WARN_ON(nvqs > 64)) @@ -1032,11 +1032,11 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs, for (i = 0; i < nvqs; ++i) { if (!names[i]) { - vqs[i] = NULL; - continue; + rc = -EINVAL; + goto error_setup; } - vqs[i] = vu_setup_vq(vdev, queue_idx++, callbacks[i], names[i], + vqs[i] = vu_setup_vq(vdev, i, callbacks[i], names[i], ctx ? ctx[i] : false); if (IS_ERR(vqs[i])) { rc = PTR_ERR(vqs[i]); diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index 25b66b113b69..7f58634fcc41 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -119,9 +119,6 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, if (id >= ARRAY_SIZE(rvdev->vring)) return ERR_PTR(-EINVAL); - if (!name) - return NULL; - /* Search allocated memory region by name */ mem = rproc_find_carveout_by_name(rproc, "vdev%dvring%d", rvdev->index, id); @@ -187,15 +184,15 @@ static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned int nvqs, const bool * ctx, struct irq_affinity *desc) { - int i, ret, queue_idx = 0; + int i, ret; for (i = 0; i < nvqs; ++i) { if (!names[i]) { - vqs[i] = NULL; - continue; + ret = -EINVAL; + goto error; } - vqs[i] = rp_find_vq(vdev, queue_idx++, callbacks[i], names[i], + vqs[i] = rp_find_vq(vdev, i, callbacks[i], names[i], ctx ? ctx[i] : false); if (IS_ERR(vqs[i])) { ret = PTR_ERR(vqs[i]); diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c index d7569f395559..6cdd29952bc0 100644 --- a/drivers/s390/virtio/virtio_ccw.c +++ b/drivers/s390/virtio/virtio_ccw.c @@ -696,7 +696,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, { struct virtio_ccw_device *vcdev = to_vc_device(vdev); dma64_t *indicatorp = NULL; - int ret, i, queue_idx = 0; + int ret, i; struct ccw1 *ccw; ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw), NULL); @@ -705,11 +705,11 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, for (i = 0; i < nvqs; ++i) { if (!names[i]) { - vqs[i] = NULL; - continue; + ret = -EINVAL; + goto out; } - vqs[i] = virtio_ccw_setup_vq(vdev, queue_idx++, callbacks[i], + vqs[i] = virtio_ccw_setup_vq(vdev, i, callbacks[i], names[i], ctx ? ctx[i] : false, ccw); if (IS_ERR(vqs[i])) { diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 173596589c71..c3c8dd282952 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -386,9 +386,6 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in else notify = vm_notify; - if (!name) - return NULL; - /* Select the queue we're interested in */ writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); @@ -496,7 +493,7 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); int irq = platform_get_irq(vm_dev->pdev, 0); - int i, err, queue_idx = 0; + int i, err; if (irq < 0) return irq; @@ -511,11 +508,11 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, for (i = 0; i < nvqs; ++i) { if (!names[i]) { - vqs[i] = NULL; - continue; + vm_del_vqs(vdev); + return -EINVAL; } - vqs[i] = vm_setup_vq(vdev, queue_idx++, callbacks[i], names[i], + vqs[i] = vm_setup_vq(vdev, i, callbacks[i], names[i], ctx ? ctx[i] : false); if (IS_ERR(vqs[i])) { vm_del_vqs(vdev); diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index b655fccaf773..eda71c6e87ee 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -292,7 +292,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, { struct virtio_pci_device *vp_dev = to_vp_device(vdev); u16 msix_vec; - int i, err, nvectors, allocated_vectors, queue_idx = 0; + int i, err, nvectors, allocated_vectors; vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); if (!vp_dev->vqs) @@ -302,7 +302,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, /* Best option: one for change interrupt, one per vq. */ nvectors = 1; for (i = 0; i < nvqs; ++i) - if (names[i] && callbacks[i]) + if (callbacks[i]) ++nvectors; } else { /* Second best: one for change, shared for all vqs. */ @@ -318,8 +318,8 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, allocated_vectors = vp_dev->msix_used_vectors; for (i = 0; i < nvqs; ++i) { if (!names[i]) { - vqs[i] = NULL; - continue; + err = -EINVAL; + goto error_find; } if (!callbacks[i]) @@ -328,7 +328,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, msix_vec = allocated_vectors++; else msix_vec = VP_MSIX_VQ_VECTOR; - vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i], + vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], ctx ? ctx[i] : false, msix_vec); if (IS_ERR(vqs[i])) { @@ -363,7 +363,7 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, const char * const names[], const bool *ctx) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); - int i, err, queue_idx = 0; + int i, err; vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); if (!vp_dev->vqs) @@ -378,10 +378,10 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, vp_dev->per_vq_vectors = false; for (i = 0; i < nvqs; ++i) { if (!names[i]) { - vqs[i] = NULL; - continue; + err = -EINVAL; + goto out_del_vqs; } - vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i], + vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], ctx ? ctx[i] : false, VIRTIO_MSI_NO_VECTOR); if (IS_ERR(vqs[i])) { diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c index e803db0da307..e82cca24d6e6 100644 --- a/drivers/virtio/virtio_vdpa.c +++ b/drivers/virtio/virtio_vdpa.c @@ -161,9 +161,6 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, bool may_reduce_num = true; int err; - if (!name) - return NULL; - if (index >= vdpa->nvqs) return ERR_PTR(-ENOENT); @@ -370,7 +367,7 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, struct cpumask *masks; struct vdpa_callback cb; bool has_affinity = desc && ops->set_vq_affinity; - int i, err, queue_idx = 0; + int i, err; if (has_affinity) { masks = create_affinity_masks(nvqs, desc ? desc : &default_affd); @@ -380,11 +377,11 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, for (i = 0; i < nvqs; ++i) { if (!names[i]) { - vqs[i] = NULL; - continue; + err = -EINVAL; + goto err_setup_vq; } - vqs[i] = virtio_vdpa_setup_vq(vdev, queue_idx++, + vqs[i] = virtio_vdpa_setup_vq(vdev, i, callbacks[i], names[i], ctx ? ctx[i] : false); if (IS_ERR(vqs[i])) { diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index da9b271b54db..1c79cec258f4 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -56,7 +56,7 @@ typedef void vq_callback_t(struct virtqueue *); * callbacks: array of callbacks, for each virtqueue * include a NULL entry for vqs that do not need a callback * names: array of virtqueue names (mainly for debugging) - * include a NULL entry for vqs unused by driver + * MUST NOT be NULL * Returns 0 on success or error status * @del_vqs: free virtqueues found by find_vqs(). * @synchronize_cbs: synchronize with the virtqueue callbacks (optional)
commit 6457f126c888 ("virtio: support reserved vqs") introduced this support. Multiqueue virtio-net use 2N as ctrl vq finally, so the logic doesn't apply. And not one uses this. On the other side, that makes some trouble for us to refactor the find_vqs() params. So I remove this support. Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> Acked-by: Jason Wang <jasowang@redhat.com> Acked-by: Eric Farman <farman@linux.ibm.com> # s390 Acked-by: Halil Pasic <pasic@linux.ibm.com> --- arch/um/drivers/virtio_uml.c | 8 ++++---- drivers/remoteproc/remoteproc_virtio.c | 11 ++++------- drivers/s390/virtio/virtio_ccw.c | 8 ++++---- drivers/virtio/virtio_mmio.c | 11 ++++------- drivers/virtio/virtio_pci_common.c | 18 +++++++++--------- drivers/virtio/virtio_vdpa.c | 11 ++++------- include/linux/virtio_config.h | 2 +- 7 files changed, 30 insertions(+), 39 deletions(-)