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

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Aug 31, 2026 at 12:04:57PM +0200, Eugenio Perez Martin wrote:
> On Mon, Aug 31, 2026 at 10:22 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Aug 31, 2026 at 10:17:56AM +0200, Eugenio Perez Martin wrote:
> > > On Mon, Aug 31, 2026 at 8:21 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Mon, Aug 31, 2026 at 08:00:00AM +0200, Eugenio Perez Martin wrote:
> > > > > On Wed, Aug 26, 2026 at 2:47 PM Eugenio Perez Martin
> > > > > <eperezma@redhat.com> wrote:
> > > > > >
> > > > > > On Tue, Aug 18, 2026 at 11:15 PM Alexander Graf <graf@amazon.com> wrote:
> > > > > > >
> > > > > > > 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.

For the better.

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