Thread (24 messages) 24 messages, 3 authors, 2026-08-31

Re: [PATCH v2 04/12] virtio_ring: return -ENOMEM when a packed ring mapping fails

flat view

From: Eugenio Perez Martin <eperezma@redhat.com>
Date: 2026-08-31 10:05:39
Also in: lkml

On Mon, Aug 31, 2026 at 10:22 AM Michael S. Tsirkin [off-list ref] wrote:
On Mon, Aug 31, 2026 at 10:17:56AM +0200, Eugenio Perez Martin wrote:
quoted
On Mon, Aug 31, 2026 at 8:21 AM Michael S. Tsirkin [off-list ref] wrote:
quoted
On Mon, Aug 31, 2026 at 08:00:00AM +0200, Eugenio Perez Martin wrote:
quoted
On Wed, Aug 26, 2026 at 2:47 PM Eugenio Perez Martin
[off-list ref] wrote:
quoted
On Tue, Aug 18, 2026 at 11:15 PM Alexander Graf [off-list ref] wrote:
quoted
Commit f7728002c1c7 ("virtio_ring: fix return code on DMA mapping
fails") moved virtqueue_add_split() and virtqueue_add_indirect_packed()
to -ENOMEM, because virtio_queue_rq() maps -EIO to BLK_STS_IOERR and
the request fails. We still return -EIO from virtqueue_add_packed(),
and virtqueue_add_packed_in_order() copied that when it was added later.

Guests that bounce their I/O through swiotlb (SEV-SNP, TDX, s390 secure
execution) run the pool out with enough I/O in flight. On a split ring
virtio_queue_rq() reports BLK_STS_RESOURCE and the block layer requeues
the request. On a packed ring virtio_queue_rq() reports BLK_STS_IOERR
instead and the error reaches the filesystem.

Return -ENOMEM from the packed unmap_release paths too. Both are reached
from a single goto on a failed mapping, which is where
vring_map_one_sg() already produces -ENOMEM.

That way every ring layout reports the same errno, and the block layer
requeues the request instead of failing it.

Fixes: f7728002c1c7 ("virtio_ring: fix return code on DMA mapping fails")
Fixes: f6a15d854986 ("virtio_ring: add in order support")
Acked-by: Eugenio Pérez <eperezma@redhat.com>
Even if I'd like to see this merged, I'm having second thoughts
because it introduces userland visible changes in some drivers. Are
them acceptable?
I mean, fixing the kernel for the userspace is kinda what we do, right?
Yes, but changing error codes returned from the kernel to userland
always reminds me of this old thread so I wanted to give a heads up:

https://lkml.org/lkml/2012/12/23/75

Now I don't think we're in the same situation, though; probably no
userland app checks the actual errno in these operations. However, the
userland apps are not limited to VMMs; they include actual subsystem
users that do not know the backend is a virtio device, so the base is
large. If you still think that will not be a problem, I'm totally in
:).

Thanks!
Well actual subsystem users for sure expect EIO on actual io errors no?
Yes, but code like drivers/scsi/virtio_scsi.c:virtscsi_queuecommand in
the kernel explicitely checks for -EIO, and had a different code path
to handle it than -ENOMEM. Even if both are valid, the visible
behavior of the code changes. My fear here is that we have similar
code in userland that we don't handle. I admit it is not very likely,
and I actually prefer making both split and packed to return the same
ERRNO, so if you're ok with the change I'm ok too.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help