diff mbox series

[vhost,v6,2/6] virtio: remove support for names array entries being null.

Message ID 20240327095741.88135-3-xuanzhuo@linux.alibaba.com
State Not Applicable
Headers show
Series refactor the params of find_vqs() | expand

Commit Message

Xuan Zhuo March 27, 2024, 9:57 a.m. UTC
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>
---
 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           |  8 ++++----
 drivers/virtio/virtio_pci_common.c     | 18 +++++++++---------
 drivers/virtio/virtio_vdpa.c           | 11 ++++-------
 include/linux/virtio_config.h          |  2 +-
 7 files changed, 30 insertions(+), 36 deletions(-)

Comments

Jason Wang March 28, 2024, 4:19 a.m. UTC | #1
On Wed, Mar 27, 2024 at 5:58 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> 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>
> ---
>  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           |  8 ++++----
>  drivers/virtio/virtio_pci_common.c     | 18 +++++++++---------
>  drivers/virtio/virtio_vdpa.c           | 11 ++++-------
>  include/linux/virtio_config.h          |  2 +-
>  7 files changed, 30 insertions(+), 36 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 83d76915a6ad..8fb5118b6953 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 ac67576301bf..508154705554 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -659,7 +659,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  {
>         struct virtio_ccw_device *vcdev = to_vc_device(vdev);
>         unsigned long *indicatorp = NULL;
> -       int ret, i, queue_idx = 0;
> +       int ret, i;
>         struct ccw1 *ccw;
>
>         ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));
> @@ -668,11 +668,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],

Nit:

This seems an unnecessary change or we need to remove the queue_idx variable.

>                                              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 59892a31cf76..82ee4a288728 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -496,7 +496,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 +511,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],

Similar issue as above.

>                                      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,

And here.

With those fixed.

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

>                                               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)
> --
> 2.32.0.3.g01195cf9f
>
>
Xuan Zhuo March 28, 2024, 6:36 a.m. UTC | #2
On Thu, 28 Mar 2024 12:19:04 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Wed, Mar 27, 2024 at 5:58 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> 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>
> > ---
> >  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           |  8 ++++----
> >  drivers/virtio/virtio_pci_common.c     | 18 +++++++++---------
> >  drivers/virtio/virtio_vdpa.c           | 11 ++++-------
> >  include/linux/virtio_config.h          |  2 +-
> >  7 files changed, 30 insertions(+), 36 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 83d76915a6ad..8fb5118b6953 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 ac67576301bf..508154705554 100644
> > --- a/drivers/s390/virtio/virtio_ccw.c
> > +++ b/drivers/s390/virtio/virtio_ccw.c
> > @@ -659,7 +659,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> >  {
> >         struct virtio_ccw_device *vcdev = to_vc_device(vdev);
> >         unsigned long *indicatorp = NULL;
> > -       int ret, i, queue_idx = 0;
> > +       int ret, i;
> >         struct ccw1 *ccw;
> >
> >         ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));
> > @@ -668,11 +668,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],
>
> Nit:
>
> This seems an unnecessary change or we need to remove the queue_idx variable.


YES. queue_idx is removed.

Thanks.


>
> >                                              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 59892a31cf76..82ee4a288728 100644
> > --- a/drivers/virtio/virtio_mmio.c
> > +++ b/drivers/virtio/virtio_mmio.c
> > @@ -496,7 +496,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 +511,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],
>
> Similar issue as above.
>
> >                                      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,
>
> And here.
>
> With those fixed.
>
> Acked-by: Jason Wang <jasowang@redhat.com>
>
> Thanks
>
> >                                               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)
> > --
> > 2.32.0.3.g01195cf9f
> >
> >
>
diff mbox series

Patch

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 83d76915a6ad..8fb5118b6953 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 ac67576301bf..508154705554 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -659,7 +659,7 @@  static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 {
 	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
 	unsigned long *indicatorp = NULL;
-	int ret, i, queue_idx = 0;
+	int ret, i;
 	struct ccw1 *ccw;
 
 	ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));
@@ -668,11 +668,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 59892a31cf76..82ee4a288728 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -496,7 +496,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 +511,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)