Re: [virtio-comment] RE: [virtio-dev] RE: [virtio-comment] RE: [virtio-dev] RE: [virtio-comment] [PATCH v2] virtio-net: support setting coalescing params for multiple vqs

[Jason Wang <jasowang@redhat.com>]

On Thu, Jan 25, 2024 at 11:05âAM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>
>
> å 2024/1/24 äå9:18, Parav Pandit åé:
> >> From: Heng Qi <hengqi@linux.alibaba.com>
> >> Sent: Wednesday, January 24, 2024 6:31 PM
> >>
> >>
> >> å 2024/1/22 äå1:03, Parav Pandit åé:
> >>>> From: Heng Qi <hengqi@linux.alibaba.com>
> >>>> Sent: Monday, January 22, 2024 8:27 AM
> >>>>
> >>>> å 2024/1/20 äå5:59, Parav Pandit åé:
> >>>>>> From: Heng Qi <hengqi@linux.alibaba.com>
> >>>>>> Sent: Wednesday, January 17, 2024 10:22 AM
> >>>>>>
> >>>>>> å 2024/1/15 äå9:21, Parav Pandit åé:
> >>>>>>>> From: virtio-comment@lists.oasis-open.org
> >>>>>>>> <virtio-comment@lists.oasis- open.org> On Behalf Of Heng Qi
> >>>>>>>> Sent: Monday, January 15, 2024 6:36 PM
> >>>>>>>>
> >>>>>>>> Currently, when each time the driver attempts to update the
> >>>>>>>> coalescing parameters for a vq, it needs to kick the device and
> >>>>>>>> wait for the ctrlq response to return.
> >>>>>>> It does not need to wait. This is some driver limitation that does
> >>>>>>> not use
> >>>>>> the queue as "queue".
> >>>>>>> Such driver limitation should be removed in the driver. It does
> >>>>>>> not qualify
> >>>>>> as limitation.
> >>>>>>
> >>>>>> Yes, we don't have to wait.
> >>>>>>
> >>>>>> But in general, for user commands, it is necessary to obtain the
> >>>>>> final results synchronously.
> >>>>> Yes. Use initiated command can enqueue the request to cvq. Go to
> >>>>> sleep
> >>>> for several micro to milliseconds.
> >>>>>> The user command cannot return before the final result is obtained.
> >>>>>> And wait is not the problem this patch solves.
> >>>>>>
> >>>>> By not holding the rtnl lock, rest of the context that needs to
> >>>>> enqueue the
> >>>> request can progress such as that of netdim.
> >>>>
> >>>> Would like to see the using of rtnl lock changed.
> >>>>
> >>> Inside the virtnet_rx_dim_work() there should be rtnl lock call.
> >>> A virtio_device level lock to be used for cvq. :)
> >>>
> >>>> In addition, I have made batching and asynchronousization of the
> >>>> netdim command, you can refer to this patch:
> >>>> https://lore.kernel.org/all/1705410693-118895-4-git-send-email-
> >>>> hengqi@linux.alibaba.com/
> >>>>
> >>> In the listed above driver patch the motivation "to optimize the CPU
> >>> overhead of the DIM worker caused by the guest being busy waiting for
> >>> the command response result."
> >>>
> >>> Is not right.
> >>> Because guest is still busy waiting.
> >> There is no busy wait for guests, see get_cvq_work().
> >>
> > Ok. not always busy waiting, sometimes it does.
>
> Busy waiting will only occur when the user command or dim command cannot
> find the available buffer for cvq.
>
> The user command is still in polling mode for now, I have not tried to
> optimize this. Now it's about improving dim performance.
>
> > virtnet_cvq_response() should not have flag..
>
> The flag is mainly used to identify whether it is a user command. If so,
> the previous polling mode will still be maintained.
>
> >
> > Who ever gets the OS global rtnl lock is calling virtnet_cvq_response() and checking and releasing.
> > Shouldnât be done this way with try lock etc.
> > Rtnl lock is not supposed to protect low level driver some ctrlvq response flag.
>
> To summarize, we now want to make some improvements to cvq:
> 1. Reasonable timeout in busy waiting mode or interrupt-based etc.

Let's use interrupt to avoid tricky code.

> 2. Batch processing (the core problem is how to get results from user
> commands synchronously)
> 3. Remove rtnl_lockâs protection for ctrlq.

Virtio-comment is not the right place to discuss these. Let's move it to netdev.

Thanks