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.