Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1743180

RE: [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG

Show all headers | View raw

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



 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v16 0/5] Virtio-balloon Enhancement Wei Wang <wei.w.wang@intel.com> - 2017-09-30 06:20 +0200
  [PATCH v16 4/5] mm: support reporting free page blocks Wei Wang <wei.w.wang@intel.com> - 2017-09-30 06:20 +0200
  [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG Wei Wang <wei.w.wang@intel.com> - 2017-09-30 06:30 +0200
    Re: [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG "Michael S. Tsirkin" <mst@redhat.com> - 2017-10-02 06:40 +0200
      RE: [PATCH v16 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG "Wang, Wei W" <wei.w.wang@intel.com> - 2017-10-02 14:40 +0200
  Re: [PATCH v16 0/5] Virtio-balloon Enhancement Damian Tometzki <damian.tometzki@icloud.com> - 2017-10-01 15:30 +0200
  Re: [PATCH v16 0/5] Virtio-balloon Enhancement Damian Tometzki <damian.tometzki@icloud.com> - 2017-10-01 16:20 +0200

csiph-web