Re: [virtio-dev] Re: [PATCH v6] virtio-snd: add virtio sound device specification

[Anton Yakovlev <anton.yakovlev@opensynergy.com>]

On 03.03.2020 15:52, Gerd Hoffmann wrote:
> On Tue, Mar 03, 2020 at 01:27:38PM +0100, Anton Yakovlev wrote:
> > On 03.03.2020 13:10, Gerd Hoffmann wrote:
> > > On Tue, Mar 03, 2020 at 12:28:32PM +0100, Anton Yakovlev wrote:
> > > > Hi,
> > > >
> > > > On 03.03.2020 12:20, Gerd Hoffmann wrote:
> > > > >      Hi,
> > > > >
> > > > > > 4. Introduce the polling mode feature for a message-based transport.
> > > > >
> > > > > BTW: is that driver -> device or device -> driver or both?
> > > > > In case both: should we have two separate feature flags maybe?
> > > >
> > > > It's driver -> device direction. The driver needs notifications from the
> > > > device in any case, so it makes no sense to suppress notifications in this
> > > > direction (device -> driver).
> > >
> > > Hmm, unless I missed something it doesn't matter whenever the guest
> > > kernel checks virtqueues from irq handler or timer ...
> >
> > Because in the general case, it is impossible to predict when a notification
> > will come from the device. Even if we are talking about a period time, this
> > does not mean that the device will send notifications precisely at these
> > intervals. And this means that the driver must do a tricky poll, trying
> > to guess when the notification will arrive, i.e. just waste CPU resources.
>
> Well, the simplest way to handle this would be that each time the driver
> submits a buffer to the device it also checks whenever the device has
> completed buffers.  No irq, no timer -> no extra wakeups to collect
> completed buffers.
>
> Drawback is that completed buffers might be unnoticed for a while then
> due to jitter, the driver might see sometimes no buffers and sometimes
> multiple buffers.  For latency-sensitive workloads this will probably
> not work very well.
>
> So, not sure whenever it makes sense to have that as an option or
> whenever guest drivers would just use notifications unconditionally
> anyway.

Well, right from the beginning the driver enqueues buffers at stream start. At
this moment there are no completed buffers in the queue (the first one should
arrive after one period). So, no completed buffers and no notifications from
the device. The only option is to set a hrtimer. We tried such approach and
it's just bad for an audio in a vm. Because it's impossible to predict when
timer will fire. We tried to set a 10ms hrtimer and it could wake up after
10ms or after 80ms in the worst case. And what's the point then? Interrupts
for message-based transport enforced not only for making a driver design much
simpler, but also to guarantee that the driver will receive completed buffer
exactly at right time.


> But in any case we should clarify in the spec that this means the driver
> doesn't notify and the device polls.

I think it's not necessary. Now the spec states

---
\item VIRTIO_SND_PCM_F_MSG_POLLING - suppress available buffer notifications
for tx and rx queues,
---

So, it's about available buffers, not used ones. It should be pretty obvious, 
that is this case the device must poll virtqueue for new messages.


> cheers,
>    Gerd

--
Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

Phone: +49 30 60 98 54 0
E-Mail: anton.yakovlev@opensynergy.com

www.opensynergy.com

Handelsregister/Commercial Registry: Amtsgericht Charlottenburg, HRB 108616B
GeschÃftsfÃhrer/Managing Director: Regis Adjamah