Re: [PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE: add new used buffer notification suppression mechanism

Michael S. Tsirkin
2024-07-05T07:57:47Z

On Thu, Jul 04, 2024 at 02:27:20AM +0000, Lege Wang wrote:
> hi,
>
> > -----Original Message-----
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, July 3, 2024 8:34 PM
> > To: Lege Wang <lege.wang@jaguarmicro.com>
> > Cc: virtio-comment@lists.linux.dev; vattunuru@marvell.com;
> > ndabilpuram@marvell.com; parav@nvidia.com; Leo Liu
> > <leo.liu@jaguarmicro.com>; Angus Chen <angus.chen@jaguarmicro.com>
> > Subject: Re: [PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE: add new used
> > buffer notification suppression mechanism
> >
> > External Mail: This email originated from OUTSIDE of the organization!
> > Do not click links, open attachments or provide ANY information unless you
> > recognize the sender and know the content is safe.
> >
> >
> > On Wed, Jul 03, 2024 at 12:14:01PM +0000, Lege Wang wrote:
> > > hi,
> > >
> > > > > > After reading this and the text, I can't figure out how is this feature
> > > > > > supposed to work without races. What if the device uses a buffer
> > while
> > > > > > the driver notification is in flight to the device?
> > > > > Hmm, I think it doesn't matter, Only the used buffer enable notification is
> > > > > transmitted to device successfully, device starts to judge whether it's time
> > > > > to send used buffer notification to driver according to its internal
> > interrupt
> > > > policy.
> > > >
> > > > I don't understand what you are saying. If device has some magical
> > > > interrupt policy why do we need a notification from the driver
> > > > at all.
> > > >
> > > > Conversely, if device relies on the notification and thinks that an
> > > > interrupt is not needed (because it did not get the notification yet),
> > > > while at the same time the driver is blocked waiting for an interrupt,
> > > > we get a deadlock.
> > > No, I'm not saying that our interrupt policy is magical?? Let me explain a bit
> > > more about its simple mechanism here, there're two basic parameters to
> > control
> > > whether device should send used buffer notification to driver:
> > > 1. number of used ring entries written since last used buffer notification to
> > driver,
> > > say threshold value is N.
> > > 2. Max wait time to send a used buffer notification to driver.
> > >
> > > The work flow would look like below:
> > > 1. driver enables used buffer notification initially when enabling virtio device
> > > 2. driver is blocked waiting interrupt.
> > > 3. if the number of used ring entries written by device is equal to or greater
> > than above threshold N,
> > > a used buffered notification is sent to driver immediately, or if this
> > condition is not met, but the period
> > > specified by max wait time has reached, device will still send one used
> > buffer notification to driver.
> > > And device will recount number of used ring entries written by device
> > from zero and start another
> > > Max wait time timer.
> > > 4. driver is waken up by interrupt, after it has processed used ring entries, it'll
> > start to wait for interrupt
> > > again and enable used buffer notification.
> > > 5. go to step 2.
> > >
> > > >From above work flow, I don't think the deadlock you describe would
> > happen.
> > >
> > > Regards,
> > > Xiaoguang Wang
> >
> > 1. device sends an interrupt
> > 2. device writes a used entry
> >
> > Will another interrupt be sent, with driver doing nothing at all?
> Unless driver enables device's used buffer notification, the device will
> not send an interrupt.
>
> Regards,
> Xiaoguang Wang

how about this:
1. device sends an interrupt
2. device writes a used entry
2. driver enables interrupt

> >
> >
> >
> > > >
> > > >
> > > > --
> > > > MST