* [PATCH] virtio_ring: return an error pointer from vring_create_virtqueue()
@ 2026-11-05 19:34 Stjepan Poljak
2026-11-05 19:43 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Stjepan Poljak @ 2026-11-05 19:34 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang
Cc: Xuan Zhuo, Eugenio Pérez, Richard Weinberger, Anton Ivanov,
Johannes Berg, Cornelia Huck, Halil Pasic, Eric Farman,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Alexander Graf,
virtualization, linux-um, linux-s390, linux-kernel
vring_create_virtqueue() and vring_create_virtqueue_map() return NULL
on any failure, so the error returned from vring_alloc_queue_split()
is lost and turned into -ENOMEM. This is especially misleading if
vring_alloc_queue_split() rejects a queue size with -EINVAL (i.e. if
queue size is not a power of two):
virtio_blk virtio0: Bad virtqueue length 100
virtio_blk virtio0: probe with driver virtio_blk failed with error -12
So the error points whoever debugs it at an allocation failure that
never happened.
Return an ERR_PTR() instead: the error from vring_alloc_queue_split()
or vring_alloc_queue_packed(), or -ENOMEM when allocating the
virtqueue itself fails. Make the callers, virtio_pci (modern and
legacy), virtio_mmio, virtio_vdpa, virtio_ccw and virtio_uml, check
with IS_ERR() and pass the error on. The error path in virtio_ccw
deletes a non-NULL vq, so clear vq there before taking it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Stjepan Poljak <stjepan.poljak@gmail.com>
---
Tested on x86_64 with a virtio-mmio virtio-blk device offering a
100-entry queue: the probe now fails with -22 instead of -12, and a
128-entry queue probes as before. The other transports are only
compile-tested (x86_64, ARCH=um, s390 with clang, W=1).
No Fixes: tag, since the error has been -ENOMEM since 2010 and nothing
regressed.
This conflicts trivially with patch 07/12 of Alexander Graf's "virtio:
support devices that own their virtqueue memory" v2:
https://lore.kernel.org/virtualization/20260818211425.91009-1-graf@amazon.com/
---
arch/um/drivers/virtio_uml.c | 4 ++--
drivers/s390/virtio/virtio_ccw.c | 5 +++--
drivers/virtio/virtio_mmio.c | 4 ++--
drivers/virtio/virtio_pci_legacy.c | 4 ++--
drivers/virtio/virtio_pci_modern.c | 4 ++--
drivers/virtio/virtio_ring.c | 12 +++++++-----
drivers/virtio/virtio_vdpa.c | 4 ++--
include/linux/virtio_ring.h | 6 +++++-
8 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
index 881c34098a74..8b645523277f 100644
--- a/arch/um/drivers/virtio_uml.c
+++ b/arch/um/drivers/virtio_uml.c
@@ -984,8 +984,8 @@ static struct virtqueue *vu_setup_vq(struct virtio_device *vdev,
vq = vring_create_virtqueue(index, num, PAGE_SIZE, vdev, true, true,
ctx, vu_notify, callback, info->name);
- if (!vq) {
- rc = -ENOMEM;
+ if (IS_ERR(vq)) {
+ rc = PTR_ERR(vq);
goto error_create;
}
vq->priv = info;
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 552d77998012..c5a30f0a2bc2 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -590,10 +590,11 @@ static struct virtqueue *virtio_ccw_setup_vq(struct virtio_device *vdev,
vdev, true, may_reduce, ctx,
notify, callback, name);
- if (!vq) {
+ if (IS_ERR(vq)) {
/* For now, we fail if we can't get the requested size. */
dev_warn(&vcdev->cdev->dev, "no vq\n");
- err = -ENOMEM;
+ err = PTR_ERR(vq);
+ vq = NULL;
goto out_err;
}
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index e2ec0ec01d67..c403a025006e 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -396,8 +396,8 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
/* Create the vring */
vq = vring_create_virtqueue(index, num, VIRTIO_MMIO_VRING_ALIGN, vdev,
true, true, ctx, notify, callback, name);
- if (!vq) {
- err = -ENOMEM;
+ if (IS_ERR(vq)) {
+ err = PTR_ERR(vq);
goto error_new_virtqueue;
}
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 8115aa39e01e..9d94fffff344 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -130,8 +130,8 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
VIRTIO_PCI_VRING_ALIGN, &vp_dev->vdev,
true, false, ctx,
vp_notify, callback, name);
- if (!vq)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(vq))
+ return vq;
vq->num_max = num;
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 922e2df027de..28ccb2130e27 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -720,8 +720,8 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
SMP_CACHE_BYTES, &vp_dev->vdev,
true, true, ctx,
notify, callback, name);
- if (!vq)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(vq))
+ return vq;
vq->num_max = num;
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index f683a2d6a200..6dd4a7651bde 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1421,13 +1421,13 @@ static struct virtqueue *vring_create_virtqueue_split(
err = vring_alloc_queue_split(&vring_split, vdev, num, vring_align,
may_reduce_num, map);
if (err)
- return NULL;
+ return ERR_PTR(err);
vq = __vring_new_virtqueue_split(index, &vring_split, vdev, weak_barriers,
context, notify, callback, name, map);
if (!vq) {
vring_free_split(&vring_split, vdev, map);
- return NULL;
+ return ERR_PTR(-ENOMEM);
}
to_vvq(vq)->we_own_ring = true;
@@ -2671,15 +2671,17 @@ static struct virtqueue *vring_create_virtqueue_packed(
{
struct vring_virtqueue_packed vring_packed = {};
struct virtqueue *vq;
+ int err;
- if (vring_alloc_queue_packed(&vring_packed, vdev, num, map))
- return NULL;
+ err = vring_alloc_queue_packed(&vring_packed, vdev, num, map);
+ if (err)
+ return ERR_PTR(err);
vq = __vring_new_virtqueue_packed(index, &vring_packed, vdev, weak_barriers,
context, notify, callback, name, map);
if (!vq) {
vring_free_packed(&vring_packed, vdev, map);
- return NULL;
+ return ERR_PTR(-ENOMEM);
}
to_vvq(vq)->we_own_ring = true;
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index f9559b6b2e0b..c1b6134b2267 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -208,8 +208,8 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index,
vq = vring_create_virtqueue_map(index, max_num, align, vdev,
true, may_reduce_num, ctx,
notify, callback, name, map);
- if (!vq) {
- err = -ENOMEM;
+ if (IS_ERR(vq)) {
+ err = PTR_ERR(vq);
goto error_new_virtqueue;
}
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index c97a12c1cda3..c808ca7e8d58 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -66,6 +66,9 @@ struct device;
* may_reduce_num is set, then this may allocate a smaller ring than
* expected. The caller should query virtqueue_get_vring_size to learn
* the actual size of the ring.
+ *
+ * Returns the virtqueue or an ERR_PTR() on failure: -EINVAL if num is
+ * not a valid ring size, -ENOMEM if the ring could not be allocated.
*/
struct virtqueue *vring_create_virtqueue(unsigned int index,
unsigned int num,
@@ -80,7 +83,8 @@ struct virtqueue *vring_create_virtqueue(unsigned int index,
/*
* Creates a virtqueue and allocates the descriptor ring with per
- * virtqueue mapping operations.
+ * virtqueue mapping operations. Returns the virtqueue or an ERR_PTR()
+ * on failure, like vring_create_virtqueue().
*/
struct virtqueue *vring_create_virtqueue_map(unsigned int index,
unsigned int num,
base-commit: 1a577f9402c1780cc1e663f48d5dd54c02051b09
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] virtio_ring: return an error pointer from vring_create_virtqueue()
2026-11-05 19:34 [PATCH] virtio_ring: return an error pointer from vring_create_virtqueue() Stjepan Poljak
@ 2026-11-05 19:43 ` Michael S. Tsirkin
2026-11-06 6:02 ` Stjepan Poljak
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2026-11-05 19:43 UTC (permalink / raw)
To: Stjepan Poljak
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Richard Weinberger,
Anton Ivanov, Johannes Berg, Cornelia Huck, Halil Pasic,
Eric Farman, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Alexander Graf,
virtualization, linux-um, linux-s390, linux-kernel
On Thu, Nov 05, 2026 at 08:34:25PM +0100, Stjepan Poljak wrote:
> vring_create_virtqueue() and vring_create_virtqueue_map() return NULL
> on any failure, so the error returned from vring_alloc_queue_split()
> is lost and turned into -ENOMEM. This is especially misleading if
> vring_alloc_queue_split() rejects a queue size with -EINVAL (i.e. if
> queue size is not a power of two):
>
> virtio_blk virtio0: Bad virtqueue length 100
> virtio_blk virtio0: probe with driver virtio_blk failed with error -12
>
> So the error points whoever debugs it at an allocation failure that
> never happened.
>
> Return an ERR_PTR() instead: the error from vring_alloc_queue_split()
> or vring_alloc_queue_packed(), or -ENOMEM when allocating the
> virtqueue itself fails. Make the callers, virtio_pci (modern and
> legacy), virtio_mmio, virtio_vdpa, virtio_ccw and virtio_uml, check
> with IS_ERR() and pass the error on. The error path in virtio_ccw
> deletes a non-NULL vq, so clear vq there before taking it.
>
> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
not how you ae supposed to attribute LLM use.
> Signed-off-by: Stjepan Poljak <stjepan.poljak@gmail.com>
> ---
> Tested on x86_64 with a virtio-mmio virtio-blk device offering a
> 100-entry queue: the probe now fails with -22 instead of -12, and a
> 128-entry queue probes as before. The other transports are only
> compile-tested (x86_64, ARCH=um, s390 with clang, W=1).
>
> No Fixes: tag, since the error has been -ENOMEM since 2010 and nothing
> regressed.
>
> This conflicts trivially with patch 07/12 of Alexander Graf's "virtio:
> support devices that own their virtqueue memory" v2:
> https://lore.kernel.org/virtualization/20260818211425.91009-1-graf@amazon.com/
> ---
> arch/um/drivers/virtio_uml.c | 4 ++--
> drivers/s390/virtio/virtio_ccw.c | 5 +++--
> drivers/virtio/virtio_mmio.c | 4 ++--
> drivers/virtio/virtio_pci_legacy.c | 4 ++--
> drivers/virtio/virtio_pci_modern.c | 4 ++--
> drivers/virtio/virtio_ring.c | 12 +++++++-----
> drivers/virtio/virtio_vdpa.c | 4 ++--
> include/linux/virtio_ring.h | 6 +++++-
> 8 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
> index 881c34098a74..8b645523277f 100644
> --- a/arch/um/drivers/virtio_uml.c
> +++ b/arch/um/drivers/virtio_uml.c
> @@ -984,8 +984,8 @@ static struct virtqueue *vu_setup_vq(struct virtio_device *vdev,
>
> vq = vring_create_virtqueue(index, num, PAGE_SIZE, vdev, true, true,
> ctx, vu_notify, callback, info->name);
> - if (!vq) {
> - rc = -ENOMEM;
> + if (IS_ERR(vq)) {
> + rc = PTR_ERR(vq);
> goto error_create;
> }
> vq->priv = info;
> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> index 552d77998012..c5a30f0a2bc2 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -590,10 +590,11 @@ static struct virtqueue *virtio_ccw_setup_vq(struct virtio_device *vdev,
> vdev, true, may_reduce, ctx,
> notify, callback, name);
>
> - if (!vq) {
> + if (IS_ERR(vq)) {
> /* For now, we fail if we can't get the requested size. */
> dev_warn(&vcdev->cdev->dev, "no vq\n");
> - err = -ENOMEM;
> + err = PTR_ERR(vq);
> + vq = NULL;
> goto out_err;
> }
>
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index e2ec0ec01d67..c403a025006e 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -396,8 +396,8 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
> /* Create the vring */
> vq = vring_create_virtqueue(index, num, VIRTIO_MMIO_VRING_ALIGN, vdev,
> true, true, ctx, notify, callback, name);
> - if (!vq) {
> - err = -ENOMEM;
> + if (IS_ERR(vq)) {
> + err = PTR_ERR(vq);
> goto error_new_virtqueue;
> }
>
> diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> index 8115aa39e01e..9d94fffff344 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -130,8 +130,8 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
> VIRTIO_PCI_VRING_ALIGN, &vp_dev->vdev,
> true, false, ctx,
> vp_notify, callback, name);
> - if (!vq)
> - return ERR_PTR(-ENOMEM);
> + if (IS_ERR(vq))
> + return vq;
>
> vq->num_max = num;
>
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 922e2df027de..28ccb2130e27 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -720,8 +720,8 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
> SMP_CACHE_BYTES, &vp_dev->vdev,
> true, true, ctx,
> notify, callback, name);
> - if (!vq)
> - return ERR_PTR(-ENOMEM);
> + if (IS_ERR(vq))
> + return vq;
>
> vq->num_max = num;
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index f683a2d6a200..6dd4a7651bde 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1421,13 +1421,13 @@ static struct virtqueue *vring_create_virtqueue_split(
> err = vring_alloc_queue_split(&vring_split, vdev, num, vring_align,
> may_reduce_num, map);
> if (err)
> - return NULL;
> + return ERR_PTR(err);
>
> vq = __vring_new_virtqueue_split(index, &vring_split, vdev, weak_barriers,
> context, notify, callback, name, map);
> if (!vq) {
> vring_free_split(&vring_split, vdev, map);
> - return NULL;
> + return ERR_PTR(-ENOMEM);
> }
>
> to_vvq(vq)->we_own_ring = true;
> @@ -2671,15 +2671,17 @@ static struct virtqueue *vring_create_virtqueue_packed(
> {
> struct vring_virtqueue_packed vring_packed = {};
> struct virtqueue *vq;
> + int err;
>
> - if (vring_alloc_queue_packed(&vring_packed, vdev, num, map))
> - return NULL;
> + err = vring_alloc_queue_packed(&vring_packed, vdev, num, map);
> + if (err)
> + return ERR_PTR(err);
>
> vq = __vring_new_virtqueue_packed(index, &vring_packed, vdev, weak_barriers,
> context, notify, callback, name, map);
> if (!vq) {
> vring_free_packed(&vring_packed, vdev, map);
> - return NULL;
> + return ERR_PTR(-ENOMEM);
> }
>
> to_vvq(vq)->we_own_ring = true;
> diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> index f9559b6b2e0b..c1b6134b2267 100644
> --- a/drivers/virtio/virtio_vdpa.c
> +++ b/drivers/virtio/virtio_vdpa.c
> @@ -208,8 +208,8 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index,
> vq = vring_create_virtqueue_map(index, max_num, align, vdev,
> true, may_reduce_num, ctx,
> notify, callback, name, map);
> - if (!vq) {
> - err = -ENOMEM;
> + if (IS_ERR(vq)) {
> + err = PTR_ERR(vq);
> goto error_new_virtqueue;
> }
>
> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> index c97a12c1cda3..c808ca7e8d58 100644
> --- a/include/linux/virtio_ring.h
> +++ b/include/linux/virtio_ring.h
> @@ -66,6 +66,9 @@ struct device;
> * may_reduce_num is set, then this may allocate a smaller ring than
> * expected. The caller should query virtqueue_get_vring_size to learn
> * the actual size of the ring.
> + *
> + * Returns the virtqueue or an ERR_PTR() on failure: -EINVAL if num is
> + * not a valid ring size, -ENOMEM if the ring could not be allocated.
> */
> struct virtqueue *vring_create_virtqueue(unsigned int index,
> unsigned int num,
> @@ -80,7 +83,8 @@ struct virtqueue *vring_create_virtqueue(unsigned int index,
>
> /*
> * Creates a virtqueue and allocates the descriptor ring with per
> - * virtqueue mapping operations.
> + * virtqueue mapping operations. Returns the virtqueue or an ERR_PTR()
> + * on failure, like vring_create_virtqueue().
> */
> struct virtqueue *vring_create_virtqueue_map(unsigned int index,
> unsigned int num,
>
> base-commit: 1a577f9402c1780cc1e663f48d5dd54c02051b09
> --
> 2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] virtio_ring: return an error pointer from vring_create_virtqueue()
2026-11-05 19:43 ` Michael S. Tsirkin
@ 2026-11-06 6:02 ` Stjepan Poljak
0 siblings, 0 replies; 3+ messages in thread
From: Stjepan Poljak @ 2026-11-06 6:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Richard Weinberger,
Anton Ivanov, Johannes Berg, Cornelia Huck, Halil Pasic,
Eric Farman, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Alexander Graf,
virtualization, linux-um, linux-s390, linux-kernel
On Thu, Nov 5, 2026 at 8:43 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> not how you ae supposed to attribute LLM use.
>
Sorry about that. I've sent the v2 with corrected LLM disclosure as
"Assisted-by": https://lore.kernel.org/virtualization/20261106055635.3019889-1-stjepan.poljak@gmail.com/T/#u
Thank you,
Stjepan Poljak
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-06 6:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-05 19:34 [PATCH] virtio_ring: return an error pointer from vring_create_virtqueue() Stjepan Poljak
2026-11-05 19:43 ` Michael S. Tsirkin
2026-11-06 6:02 ` Stjepan Poljak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®