[2/3] vhost: better detection of available buffers
diff mbox series

Message ID 1478677113-13126-2-git-send-email-jasowang@redhat.com
State New, archived
Headers show
Series
  • [1/3] tuntap: rx batching
Related show

Commit Message

Jason Wang Nov. 9, 2016, 7:38 a.m. UTC
We should use vq->last_avail_idx instead of vq->avail_idx in the
checking of vhost_vq_avail_empty() since latter is the cached avail
index from guest but we want to know if there's pending available
buffers in the virtqueue.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael S. Tsirkin Nov. 9, 2016, 7:57 p.m. UTC | #1
On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
> We should use vq->last_avail_idx instead of vq->avail_idx in the
> checking of vhost_vq_avail_empty() since latter is the cached avail
> index from guest but we want to know if there's pending available
> buffers in the virtqueue.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I'm not sure why is this patch here. Is it related to
batching somehow?


> ---
>  drivers/vhost/vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index c6f2d89..fdf4cdf 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>  	if (r)
>  		return false;
>  
> -	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
> +	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
>  }
>  EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);

That might be OK for TX but it's probably wrong for RX
where the fact that used != avail does not mean
we have enough space to store the packet.

Maybe we should just rename this to vhost_vq_avail_unchanged
to clarify usage.


>  
> -- 
> 2.7.4
Jason Wang Nov. 11, 2016, 2:18 a.m. UTC | #2
On 2016年11月10日 03:57, Michael S. Tsirkin wrote:
> On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
>> We should use vq->last_avail_idx instead of vq->avail_idx in the
>> checking of vhost_vq_avail_empty() since latter is the cached avail
>> index from guest but we want to know if there's pending available
>> buffers in the virtqueue.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> I'm not sure why is this patch here. Is it related to
> batching somehow?

Yes, we need to know whether or not there's still buffers left in the 
virtqueue, so need to check last_avail_idx. Otherwise, we're checking if 
guest has submitted new buffers.

>
>
>> ---
>>   drivers/vhost/vhost.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index c6f2d89..fdf4cdf 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>>   	if (r)
>>   		return false;
>>   
>> -	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
>> +	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
>>   }
>>   EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
> That might be OK for TX but it's probably wrong for RX
> where the fact that used != avail does not mean
> we have enough space to store the packet.

Right, but it's no harm since it was just a hint, handle_rx() can handle 
this situation.

>
> Maybe we should just rename this to vhost_vq_avail_unchanged
> to clarify usage.
>

Ok.

>>   
>> -- 
>> 2.7.4
Michael S. Tsirkin Nov. 11, 2016, 3:41 a.m. UTC | #3
On Fri, Nov 11, 2016 at 10:18:37AM +0800, Jason Wang wrote:
> 
> 
> On 2016年11月10日 03:57, Michael S. Tsirkin wrote:
> > On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
> > > We should use vq->last_avail_idx instead of vq->avail_idx in the
> > > checking of vhost_vq_avail_empty() since latter is the cached avail
> > > index from guest but we want to know if there's pending available
> > > buffers in the virtqueue.
> > > 
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > I'm not sure why is this patch here. Is it related to
> > batching somehow?
> 
> Yes, we need to know whether or not there's still buffers left in the
> virtqueue, so need to check last_avail_idx. Otherwise, we're checking if
> guest has submitted new buffers.
> 
> > 
> > 
> > > ---
> > >   drivers/vhost/vhost.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index c6f2d89..fdf4cdf 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> > >   	if (r)
> > >   		return false;
> > > -	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
> > > +	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
> > >   }
> > >   EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
> > That might be OK for TX but it's probably wrong for RX
> > where the fact that used != avail does not mean
> > we have enough space to store the packet.
> 
> Right, but it's no harm since it was just a hint, handle_rx() can handle
> this situation.

Means busy polling will cause useless load on the CPU though.

> > 
> > Maybe we should just rename this to vhost_vq_avail_unchanged
> > to clarify usage.
> > 
> 
> Ok.
> 
> > > -- 
> > > 2.7.4
Jason Wang Nov. 11, 2016, 4:18 a.m. UTC | #4
On 2016年11月11日 11:41, Michael S. Tsirkin wrote:
> On Fri, Nov 11, 2016 at 10:18:37AM +0800, Jason Wang wrote:
>> >
>> >
>> >On 2016年11月10日 03:57, Michael S. Tsirkin wrote:
>>> > >On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
>>>> > > >We should use vq->last_avail_idx instead of vq->avail_idx in the
>>>> > > >checking of vhost_vq_avail_empty() since latter is the cached avail
>>>> > > >index from guest but we want to know if there's pending available
>>>> > > >buffers in the virtqueue.
>>>> > > >
>>>> > > >Signed-off-by: Jason Wang<jasowang@redhat.com>
>>> > >I'm not sure why is this patch here. Is it related to
>>> > >batching somehow?
>> >
>> >Yes, we need to know whether or not there's still buffers left in the
>> >virtqueue, so need to check last_avail_idx. Otherwise, we're checking if
>> >guest has submitted new buffers.
>> >
>>> > >
>>> > >
>>>> > > >---
>>>> > > >   drivers/vhost/vhost.c | 2 +-
>>>> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
>>>> > > >
>>>> > > >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>>> > > >index c6f2d89..fdf4cdf 100644
>>>> > > >--- a/drivers/vhost/vhost.c
>>>> > > >+++ b/drivers/vhost/vhost.c
>>>> > > >@@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>>>> > > >   	if (r)
>>>> > > >   		return false;
>>>> > > >-	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
>>>> > > >+	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
>>>> > > >   }
>>>> > > >   EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
>>> > >That might be OK for TX but it's probably wrong for RX
>>> > >where the fact that used != avail does not mean
>>> > >we have enough space to store the packet.
>> >
>> >Right, but it's no harm since it was just a hint, handle_rx() can handle
>> >this situation.
> Means busy polling will cause useless load on the CPU though.
>

Right, but,it's not easy to have 100% correct hint here. Needs more thought.
Michael S. Tsirkin Nov. 11, 2016, 4:20 p.m. UTC | #5
On Fri, Nov 11, 2016 at 12:18:50PM +0800, Jason Wang wrote:
> 
> 
> On 2016年11月11日 11:41, Michael S. Tsirkin wrote:
> > On Fri, Nov 11, 2016 at 10:18:37AM +0800, Jason Wang wrote:
> > > >
> > > >
> > > >On 2016年11月10日 03:57, Michael S. Tsirkin wrote:
> > > > > >On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
> > > > > > > >We should use vq->last_avail_idx instead of vq->avail_idx in the
> > > > > > > >checking of vhost_vq_avail_empty() since latter is the cached avail
> > > > > > > >index from guest but we want to know if there's pending available
> > > > > > > >buffers in the virtqueue.
> > > > > > > >
> > > > > > > >Signed-off-by: Jason Wang<jasowang@redhat.com>
> > > > > >I'm not sure why is this patch here. Is it related to
> > > > > >batching somehow?
> > > >
> > > >Yes, we need to know whether or not there's still buffers left in the
> > > >virtqueue, so need to check last_avail_idx. Otherwise, we're checking if
> > > >guest has submitted new buffers.
> > > >
> > > > > >
> > > > > >
> > > > > > > >---
> > > > > > > >   drivers/vhost/vhost.c | 2 +-
> > > > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > >
> > > > > > > >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > > > > >index c6f2d89..fdf4cdf 100644
> > > > > > > >--- a/drivers/vhost/vhost.c
> > > > > > > >+++ b/drivers/vhost/vhost.c
> > > > > > > >@@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> > > > > > > >   	if (r)
> > > > > > > >   		return false;
> > > > > > > >-	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
> > > > > > > >+	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
> > > > > > > >   }
> > > > > > > >   EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
> > > > > >That might be OK for TX but it's probably wrong for RX
> > > > > >where the fact that used != avail does not mean
> > > > > >we have enough space to store the packet.
> > > >
> > > >Right, but it's no harm since it was just a hint, handle_rx() can handle
> > > >this situation.
> > Means busy polling will cause useless load on the CPU though.
> > 
> 
> Right, but,it's not easy to have 100% correct hint here. Needs more thought.

What's wrong with what we have? It polls until value changes.
Jason Wang Nov. 15, 2016, 3:16 a.m. UTC | #6
On 2016年11月12日 00:20, Michael S. Tsirkin wrote:
> On Fri, Nov 11, 2016 at 12:18:50PM +0800, Jason Wang wrote:
>>
>> On 2016年11月11日 11:41, Michael S. Tsirkin wrote:
>>> On Fri, Nov 11, 2016 at 10:18:37AM +0800, Jason Wang wrote:
>>>>>
>>>>> On 2016年11月10日 03:57, Michael S. Tsirkin wrote:
>>>>>>> On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
>>>>>>>>> We should use vq->last_avail_idx instead of vq->avail_idx in the
>>>>>>>>> checking of vhost_vq_avail_empty() since latter is the cached avail
>>>>>>>>> index from guest but we want to know if there's pending available
>>>>>>>>> buffers in the virtqueue.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>>>>>>> I'm not sure why is this patch here. Is it related to
>>>>>>> batching somehow?
>>>>> Yes, we need to know whether or not there's still buffers left in the
>>>>> virtqueue, so need to check last_avail_idx. Otherwise, we're checking if
>>>>> guest has submitted new buffers.
>>>>>
>>>>>>>
>>>>>>>>> ---
>>>>>>>>>    drivers/vhost/vhost.c | 2 +-
>>>>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>>>>>>>> index c6f2d89..fdf4cdf 100644
>>>>>>>>> --- a/drivers/vhost/vhost.c
>>>>>>>>> +++ b/drivers/vhost/vhost.c
>>>>>>>>> @@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>>>>>>>>>    	if (r)
>>>>>>>>>    		return false;
>>>>>>>>> -	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
>>>>>>>>> +	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
>>>>>>>>>    }
>>>>>>>>>    EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
>>>>>>> That might be OK for TX but it's probably wrong for RX
>>>>>>> where the fact that used != avail does not mean
>>>>>>> we have enough space to store the packet.
>>>>> Right, but it's no harm since it was just a hint, handle_rx() can handle
>>>>> this situation.
>>> Means busy polling will cause useless load on the CPU though.
>>>
>> Right, but,it's not easy to have 100% correct hint here. Needs more thought.
> What's wrong with what we have? It polls until value changes.
>

But as you said, this does not mean (in mergeable cases) we have enough 
space to store the packet.
Michael S. Tsirkin Nov. 15, 2016, 3:28 a.m. UTC | #7
On Tue, Nov 15, 2016 at 11:16:59AM +0800, Jason Wang wrote:
> 
> 
> On 2016年11月12日 00:20, Michael S. Tsirkin wrote:
> > On Fri, Nov 11, 2016 at 12:18:50PM +0800, Jason Wang wrote:
> > > 
> > > On 2016年11月11日 11:41, Michael S. Tsirkin wrote:
> > > > On Fri, Nov 11, 2016 at 10:18:37AM +0800, Jason Wang wrote:
> > > > > > 
> > > > > > On 2016年11月10日 03:57, Michael S. Tsirkin wrote:
> > > > > > > > On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
> > > > > > > > > > We should use vq->last_avail_idx instead of vq->avail_idx in the
> > > > > > > > > > checking of vhost_vq_avail_empty() since latter is the cached avail
> > > > > > > > > > index from guest but we want to know if there's pending available
> > > > > > > > > > buffers in the virtqueue.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > > > > > > > I'm not sure why is this patch here. Is it related to
> > > > > > > > batching somehow?
> > > > > > Yes, we need to know whether or not there's still buffers left in the
> > > > > > virtqueue, so need to check last_avail_idx. Otherwise, we're checking if
> > > > > > guest has submitted new buffers.
> > > > > > 
> > > > > > > > 
> > > > > > > > > > ---
> > > > > > > > > >    drivers/vhost/vhost.c | 2 +-
> > > > > > > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > > > > > > > index c6f2d89..fdf4cdf 100644
> > > > > > > > > > --- a/drivers/vhost/vhost.c
> > > > > > > > > > +++ b/drivers/vhost/vhost.c
> > > > > > > > > > @@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> > > > > > > > > >    	if (r)
> > > > > > > > > >    		return false;
> > > > > > > > > > -	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
> > > > > > > > > > +	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
> > > > > > > > > >    }
> > > > > > > > > >    EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
> > > > > > > > That might be OK for TX but it's probably wrong for RX
> > > > > > > > where the fact that used != avail does not mean
> > > > > > > > we have enough space to store the packet.
> > > > > > Right, but it's no harm since it was just a hint, handle_rx() can handle
> > > > > > this situation.
> > > > Means busy polling will cause useless load on the CPU though.
> > > > 
> > > Right, but,it's not easy to have 100% correct hint here. Needs more thought.
> > What's wrong with what we have? It polls until value changes.
> > 
> 
> But as you said, this does not mean (in mergeable cases) we have enough
> space to store the packet.

Absolutely but it checks once and then only re-checks after value
changes again.
Jason Wang Nov. 15, 2016, 8 a.m. UTC | #8
On 2016年11月15日 11:28, Michael S. Tsirkin wrote:
> On Tue, Nov 15, 2016 at 11:16:59AM +0800, Jason Wang wrote:
>>
>> On 2016年11月12日 00:20, Michael S. Tsirkin wrote:
>>> On Fri, Nov 11, 2016 at 12:18:50PM +0800, Jason Wang wrote:
>>>> On 2016年11月11日 11:41, Michael S. Tsirkin wrote:
>>>>> On Fri, Nov 11, 2016 at 10:18:37AM +0800, Jason Wang wrote:
>>>>>>> On 2016年11月10日 03:57, Michael S. Tsirkin wrote:
>>>>>>>>> On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
>>>>>>>>>>> We should use vq->last_avail_idx instead of vq->avail_idx in the
>>>>>>>>>>> checking of vhost_vq_avail_empty() since latter is the cached avail
>>>>>>>>>>> index from guest but we want to know if there's pending available
>>>>>>>>>>> buffers in the virtqueue.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>>>>>>>>> I'm not sure why is this patch here. Is it related to
>>>>>>>>> batching somehow?
>>>>>>> Yes, we need to know whether or not there's still buffers left in the
>>>>>>> virtqueue, so need to check last_avail_idx. Otherwise, we're checking if
>>>>>>> guest has submitted new buffers.
>>>>>>>
>>>>>>>>>>> ---
>>>>>>>>>>>     drivers/vhost/vhost.c | 2 +-
>>>>>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>>>>>>>>>> index c6f2d89..fdf4cdf 100644
>>>>>>>>>>> --- a/drivers/vhost/vhost.c
>>>>>>>>>>> +++ b/drivers/vhost/vhost.c
>>>>>>>>>>> @@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>>>>>>>>>>>     	if (r)
>>>>>>>>>>>     		return false;
>>>>>>>>>>> -	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
>>>>>>>>>>> +	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
>>>>>>>>>>>     }
>>>>>>>>>>>     EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
>>>>>>>>> That might be OK for TX but it's probably wrong for RX
>>>>>>>>> where the fact that used != avail does not mean
>>>>>>>>> we have enough space to store the packet.
>>>>>>> Right, but it's no harm since it was just a hint, handle_rx() can handle
>>>>>>> this situation.
>>>>> Means busy polling will cause useless load on the CPU though.
>>>>>
>>>> Right, but,it's not easy to have 100% correct hint here. Needs more thought.
>>> What's wrong with what we have? It polls until value changes.
>>>
>> But as you said, this does not mean (in mergeable cases) we have enough
>> space to store the packet.
> Absolutely but it checks once and then only re-checks after value
> changes again.
>

Since get_rx_bufs() does not get enough buffers, we will wait for the 
kick in this case. For busy polling, we probably want to stay in the 
busy loop here.
Michael S. Tsirkin Nov. 15, 2016, 2:46 p.m. UTC | #9
On Tue, Nov 15, 2016 at 04:00:21PM +0800, Jason Wang wrote:
> 
> 
> On 2016年11月15日 11:28, Michael S. Tsirkin wrote:
> > On Tue, Nov 15, 2016 at 11:16:59AM +0800, Jason Wang wrote:
> > > 
> > > On 2016年11月12日 00:20, Michael S. Tsirkin wrote:
> > > > On Fri, Nov 11, 2016 at 12:18:50PM +0800, Jason Wang wrote:
> > > > > On 2016年11月11日 11:41, Michael S. Tsirkin wrote:
> > > > > > On Fri, Nov 11, 2016 at 10:18:37AM +0800, Jason Wang wrote:
> > > > > > > > On 2016年11月10日 03:57, Michael S. Tsirkin wrote:
> > > > > > > > > > On Wed, Nov 09, 2016 at 03:38:32PM +0800, Jason Wang wrote:
> > > > > > > > > > > > We should use vq->last_avail_idx instead of vq->avail_idx in the
> > > > > > > > > > > > checking of vhost_vq_avail_empty() since latter is the cached avail
> > > > > > > > > > > > index from guest but we want to know if there's pending available
> > > > > > > > > > > > buffers in the virtqueue.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > > > > > > > > > I'm not sure why is this patch here. Is it related to
> > > > > > > > > > batching somehow?
> > > > > > > > Yes, we need to know whether or not there's still buffers left in the
> > > > > > > > virtqueue, so need to check last_avail_idx. Otherwise, we're checking if
> > > > > > > > guest has submitted new buffers.
> > > > > > > > 
> > > > > > > > > > > > ---
> > > > > > > > > > > >     drivers/vhost/vhost.c | 2 +-
> > > > > > > > > > > >     1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > > > > > > > > > index c6f2d89..fdf4cdf 100644
> > > > > > > > > > > > --- a/drivers/vhost/vhost.c
> > > > > > > > > > > > +++ b/drivers/vhost/vhost.c
> > > > > > > > > > > > @@ -2230,7 +2230,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> > > > > > > > > > > >     	if (r)
> > > > > > > > > > > >     		return false;
> > > > > > > > > > > > -	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
> > > > > > > > > > > > +	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
> > > > > > > > > > > >     }
> > > > > > > > > > > >     EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
> > > > > > > > > > That might be OK for TX but it's probably wrong for RX
> > > > > > > > > > where the fact that used != avail does not mean
> > > > > > > > > > we have enough space to store the packet.
> > > > > > > > Right, but it's no harm since it was just a hint, handle_rx() can handle
> > > > > > > > this situation.
> > > > > > Means busy polling will cause useless load on the CPU though.
> > > > > > 
> > > > > Right, but,it's not easy to have 100% correct hint here. Needs more thought.
> > > > What's wrong with what we have? It polls until value changes.
> > > > 
> > > But as you said, this does not mean (in mergeable cases) we have enough
> > > space to store the packet.
> > Absolutely but it checks once and then only re-checks after value
> > changes again.
> > 
> 
> Since get_rx_bufs() does not get enough buffers, we will wait for the kick
> in this case. For busy polling, we probably want to stay in the busy loop
> here.

That's what I'm saying. You don't want to re-poll the queue
if available idx was unchanged.

Patch
diff mbox series

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c6f2d89..fdf4cdf 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2230,7 +2230,7 @@  bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
 	if (r)
 		return false;
 
-	return vhost16_to_cpu(vq, avail_idx) == vq->avail_idx;
+	return vhost16_to_cpu(vq, avail_idx) == vq->last_avail_idx;
 }
 EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);