On Wed, 20 Oct 2021 04:24:33 -0400, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
> On Wed, Oct 20, 2021 at 10:19:46AM +0800, Xuan Zhuo wrote:
> > On Tue, 19 Oct 2021 09:21:51 -0400, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
> > > On Tue, Oct 19, 2021 at 07:52:35PM +0800, Xuan Zhuo wrote:
> > > > When using indirect with packed, we don't check for allocation failures.
> > > > This patch checks that and fall back on direct.
> > > >
> > > > Fixes: 1ce9e6055fa ("virtio_ring: introduce packed ring support")
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > > > ---
> > > > drivers/virtio/virtio_ring.c | 13 ++++++++++---
> > > > 1 file changed, 10 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index 91a46c4da87d..44a03b6e4dc4 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -1065,6 +1065,9 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
> > > >
> > > > head = vq->packed.next_avail_idx;
> > > > desc = alloc_indirect_packed(total_sg, gfp);
> > > > + if (!desc)
> > > > + /* fall back on direct */
> > >
> > > this comment belongs in virtqueue_add_packed right after
> > > return.
> > >
> > > > + return -EAGAIN;
> > > >
> > >
> > > -ENOMEM surely?
> >
> > virtqueue_add_indirect_packed() will return -ENOMEM when dma mmap fails, so we
> > have to choose a different error code.
> >
> > Thanks.
>
> That's exactly the point. Why would we want to recover from allocation
> failure but not DMA map failure?
>From indirect to direct mode, there is no need to allocate memory, so if we
encounter memory allocation failure, we can fall back from indirect to direct
mode. Memory allocation failure is a temporary problem.
And if you encounter a dma mmap error, this situation should be notified to the
upper layer.
Thanks.
>
> > >
> > > > if (unlikely(vq->vq.num_free < 1)) {
> > > > pr_debug("Can't add buf len 1 - avail = 0\n");
> > > > @@ -1176,6 +1179,7 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
> > > > unsigned int i, n, c, descs_used, err_idx;
> > > > __le16 head_flags, flags;
> > > > u16 head, id, prev, curr, avail_used_flags;
> > > > + int err;
> > > >
> > > > START_USE(vq);
> > > >
> > > > @@ -1191,9 +1195,12 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
> > > >
> > > > BUG_ON(total_sg == 0);
> > > >
> > > > - if (virtqueue_use_indirect(_vq, total_sg))
> > > > - return virtqueue_add_indirect_packed(vq, sgs, total_sg,
> > > > - out_sgs, in_sgs, data, gfp);
> > > > + if (virtqueue_use_indirect(_vq, total_sg)) {
> > > > + err = virtqueue_add_indirect_packed(vq, sgs, total_sg, out_sgs,
> > > > + in_sgs, data, gfp);
> > > > + if (err != -EAGAIN)
> > > > + return err;
> > > > + }
> > > >
> > > > head = vq->packed.next_avail_idx;
> > > > avail_used_flags = vq->packed.avail_used_flags;
> > > > --
> > > > 2.31.0
> > >
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
