[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG |
Date: |
Mon, 2 Oct 2017 16:44:52 +0300 |
On Mon, Oct 02, 2017 at 12:39:30PM +0000, Wang, Wei W wrote:
> On Monday, October 2, 2017 12:30 PM, Michael S. Tsirkin wrote:
> > On Sat, Sep 30, 2017 at 12:05:52PM +0800, Wei Wang wrote:
> > > +static int send_balloon_page_sg(struct virtio_balloon *vb,
> > > + struct virtqueue *vq,
> > > + void *addr,
> > > + uint32_t size,
> > > + bool batch)
> > > +{
> > > + int err;
> > > +
> > > + err = add_one_sg(vq, addr, size);
> > > +
> > > + /* If batchng is requested, we batch till the vq is full */
> >
> > typo
> >
> > > + if (!batch || !vq->num_free)
> > > + kick_and_wait(vq, vb->acked);
> > > +
> > > + return err;
> > > +}
> >
> > If add_one_sg fails, kick_and_wait will hang forever.
> >
> > The reason this might work in because
> > 1. with 1 sg there are no memory allocations 2. if adding fails on vq full,
> > then
> > something
> > is in queue and will wake up kick_and_wait.
> >
> > So in short this is expected to never fail.
> > How about a BUG_ON here then?
> > And make it void, and add a comment with above explanation.
> >
>
>
> Yes, agree that this wouldn't fail - the worker thread performing the
> ballooning operations has been put into sleep when the vq is full, so I think
> there shouldn't be anyone else to put more sgs onto the vq then.
> Btw, not sure if we need to mention memory allocation in the comment, I found
> virtqueue_add() doesn't return any error when allocation (for indirect
> desc-s) fails - it simply avoids the use of indirect desc.
>
> What do you think of the following?
>
> err = add_one_sg(vq, addr, size);
> /*
> * This is expected to never fail: there is always at least 1 entry
> available on the vq,
> * because when the vq is full the worker thread that adds the sg will be
> put into
> * sleep until at least 1 entry is available to use.
> */
> BUG_ON(err);
>
> Best,
> Wei
>
>
>
>
Sounds good.
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Michael S. Tsirkin, 2017/10/02
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Michael S. Tsirkin, 2017/10/09
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Wei Wang, 2017/10/10
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Tetsuo Handa, 2017/10/10
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Wei Wang, 2017/10/10
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Tetsuo Handa, 2017/10/10
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Wei Wang, 2017/10/10
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Tetsuo Handa, 2017/10/10
- Re: [Qemu-devel] [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG, Wei Wang, 2017/10/10