On 2018/07/24 12:28, Tonghao Zhang wrote:
> On Tue, Jul 24, 2018 at 10:53 AM Toshiaki Makita
> <makita.toshiaki@xxxxxxxxxxxxx> wrote:
>>
>> On 2018/07/24 2:31, Tonghao Zhang wrote:
>>> On Mon, Jul 23, 2018 at 10:20 PM Toshiaki Makita
>>> <toshiaki.makita1@xxxxxxxxx> wrote:
>>>>
>>>> On 18/07/23 (月) 21:43, Tonghao Zhang wrote:
>>>>> On Mon, Jul 23, 2018 at 5:58 PM Toshiaki Makita
>>>>> <makita.toshiaki@xxxxxxxxxxxxx> wrote:
>>>>>>
>>>>>> On 2018/07/22 3:04, xiangxia.m.yue@xxxxxxxxx wrote:
>>>>>>> From: Tonghao Zhang <xiangxia.m.yue@xxxxxxxxx>
>>>>>>>
>>>>>>> Factor out generic busy polling logic and will be
>>>>>>> used for in tx path in the next patch. And with the patch,
>>>>>>> qemu can set differently the busyloop_timeout for rx queue.
>>>>>>>
>>>>>>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@xxxxxxxxx>
>>>>>>> ---
>>>>>> ...
>>>>>>> +static void vhost_net_busy_poll_vq_check(struct vhost_net *net,
>>>>>>> + struct vhost_virtqueue *rvq,
>>>>>>> + struct vhost_virtqueue *tvq,
>>>>>>> + bool rx)
>>>>>>> +{
>>>>>>> + struct socket *sock = rvq->private_data;
>>>>>>> +
>>>>>>> + if (rx) {
>>>>>>> + if (!vhost_vq_avail_empty(&net->dev, tvq)) {
>>>>>>> + vhost_poll_queue(&tvq->poll);
>>>>>>> + } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) {
>>>>>>> + vhost_disable_notify(&net->dev, tvq);
>>>>>>> + vhost_poll_queue(&tvq->poll);
>>>>>>> + }
>>>>>>> + } else if ((sock && sk_has_rx_data(sock->sk)) &&
>>>>>>> + !vhost_vq_avail_empty(&net->dev, rvq)) {
>>>>>>> + vhost_poll_queue(&rvq->poll);
>>>>>>
>>>>>> Now we wait for vq_avail for rx as well, I think you cannot skip
>>>>>> vhost_enable_notify() on tx. Probably you might want to do:
>>>>> I think vhost_enable_notify is needed.
>>>>>
>>>>>> } else if (sock && sk_has_rx_data(sock->sk)) {
>>>>>> if (!vhost_vq_avail_empty(&net->dev, rvq)) {
>>>>>> vhost_poll_queue(&rvq->poll);
>>>>>> } else if (unlikely(vhost_enable_notify(&net->dev, rvq))) {
>>>>>> vhost_disable_notify(&net->dev, rvq);
>>>>>> vhost_poll_queue(&rvq->poll);
>>>>>> }
>>>>>> }
>>>>> As Jason review as before, we only want rx kick when packet is pending at
>>>>> socket but we're out of available buffers. So we just enable notify,
>>>>> but not poll it ?
>>>>>
>>>>> } else if ((sock && sk_has_rx_data(sock->sk)) &&
>>>>> !vhost_vq_avail_empty(&net->dev, rvq)) {
>>>>> vhost_poll_queue(&rvq->poll);
>>>>> else {
>>>>> vhost_enable_notify(&net->dev, rvq);
>>>>> }
>>>>
>>>> When vhost_enable_notify() returns true the avail becomes non-empty
>>>> while we are enabling notify. We may delay the rx process if we don't
>>>> check the return value of vhost_enable_notify().
>>> I got it thanks.
>>>>>> Also it's better to care vhost_net_disable_vq()/vhost_net_enable_vq() on tx?
>>>>> I cant find why it is better, if necessary, we can do it.
>>>>
>>>> The reason is pretty simple... we are busypolling the socket so we don't
>>>> need rx wakeups during it?
>>> OK, but one question, how about rx? do we use the
>>> vhost_net_disable_vq/vhost_net_ensable_vq on rx ?
>>
>> If we are busypolling the sock tx buf? I'm not sure if polling it
>> improves the performance.
> Not the sock tx buff, when we are busypolling in handle_rx, we will
> check the tx vring via vhost_vq_avail_empty.
> So, should we the disable tvq, e.g. vhost_net_disable_vq(net, tvq)?> --
When you want to stop vq kicks from the guest you should call
vhost_disable_notify() and when you want to stop vq wakeups from the
socket you should call vhost_net_disable_vq().
You are polling vq_avail so you want to stop vq kicks thus
vhost_disable_notify() is needed and it is already called.
--
Toshiaki Makita
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
