Re: [virtio-comment] Re: [PATCH v3 6/8] admin: Add theory of operation for write recording commands

[Jason Wang <jasowang@redhat.com>]

On Wed, Nov 22, 2023 at 12:35âPM Parav Pandit <parav@nvidia.com> wrote:
>
>
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Wednesday, November 22, 2023 9:48 AM
> >
> > On Wed, Nov 22, 2023 at 12:29âAM Parav Pandit <parav@nvidia.com> wrote:
> > >
> > >
> > > > From: Jason Wang <jasowang@redhat.com>
> > > > Sent: Tuesday, November 21, 2023 10:47 AM
> > > >
> > > > On Fri, Nov 17, 2023 at 8:51âPM Parav Pandit <parav@nvidia.com> wrote:
> > > > >
> > > > >
> > > > > > From: virtio-comment@lists.oasis-open.org
> > > > > > <virtio-comment@lists.oasis- open.org> On Behalf Of Michael S.
> > > > > > Tsirkin
> > > > > > Sent: Friday, November 17, 2023 6:11 PM
> > > > > >
> > > > > > On Fri, Nov 17, 2023 at 12:22:59PM +0000, Parav Pandit wrote:
> > > > > > >
> > > > > > >
> > > > > > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > > > > > Sent: Friday, November 17, 2023 5:03 PM
> > > > > > > > To: Parav Pandit <parav@nvidia.com>
> > > > > > >
> > > > > > > > > Somehow the claim of shadow vq is great without sharing
> > > > > > > > > any performance
> > > > > > > > numbers is what I don't agree with.
> > > > > > > >
> > > > > > > > It's upstream in QEMU. Test it youself.
> > > > > > > >
> > > > > > > We did few minutes back.
> > > > > > > It results in a call trace.
> > > > > > > Vhost_vdpa_setup_vq_irq crashes on list corruption on net-next.
> > > > > >
> > > > > > Wrong list for this bug report.
> > > > > >
> > > > > > > We are stopping any shadow vq tests on unstable stuff.
> > > > > >
> > > > > > If you don't want to benchmark against alternatives how are you
> > > > > > going to prove your stuff is worth everyone's time?
> > > > >
> > > > > Comparing performance of the functional things count.
> > > > > You suggest shadow vq, frankly you should post the grand numbers
> > > > > of
> > > > shadow vq.
> > > >
> > > > We need an apple to apple comparison. Otherwise you may argue with
> > > > that, no?
> > > >
> > > When the requirements are met the comparison can be made of the
> > solution.
> > > And I donât see that the basic requirements are matching for two different
> > use cases.
> > > So no point in discussing one OS specific implementation as reference
> > point.
> >
> > Shadow virtqueue is not OS specific, it's a common method. If you disagree,
> > please explain why.
> >
> As you claim shadow virtqueue is generic not dependent on OS, how does I benchmark on QNX today?

You know Qemu is portable? How did you benchmark QNX with your proposal?

>
> > > Otherwise I will end up adding vfio link in the commit log in next version as
> > you are asking similar things here and being non neutral to your ask.
> >
> > When doing a benchmark, you need to describe your setups, no? So any
> > benchmark is setup specific, nothing wrong.
> >
> > It looks to me you claim your method is better, but refuse to give proofs.
> >
> I gave details to Michael in the email. Please refer.
>
> > >
> > > Anyway, please bring the perf data whichever you want to compare in
> > another forum. It is not the criteria anyway.
> >
> > So how can you prove your method is the best one? You have posted the
> > series for months, and so far I still don't see any rationale about why you
> > choose to go that way.
> It is explained in theory of operation.
> You refuse to read it.

Can you tell me which part in your v4 explains why you choose to go this way?

"""
+During the device migration flow, a passthrough device may write data to the
+guest virtual machine's memory, a source hypervisor needs to keep
track of these
+written memory to migrate such memory to destination hypervisor.
+Some systems may not be able to keep track of such memory write addresses at
+hypervisor level. In such a scenario, a device records and reports these
+written memory addresses to the owner device. The owner driver enables write
+recording for one or more physical address ranges per device during device
+migration flow. The owner driver periodically queries these written physical
+address records from the device. As the driver reads the written
address records,
+the device clears those records from the device.
+Once the device reports zero or small number of written address
records, the device
+mode is set to \field{Stop} or \field{Freeze}. Once the device is set
to \field{Stop}
+or \field{Freeze} mode, and once all the IOVA records are read, the
driver stops
+the write recording in the device.
"""

>
> >
> > This is very odd as we've gone through several methods one or two years ago
> > when discussing vDPA live migration.
> >
> It does not matter as this is not vdpa forum.
>
> > >
> > > > >
> > > > > It is really not my role to report bug of unstable stuff and
> > > > > compare the perf
> > > > against.
> > > >
> > > > Qemu/KVM is highly relevant here no? And it's the way to develop the
> > > > community. The shadow vq code is handy.
> > > It is relevant for direct mapped device.
> >
> > Let's focus on the function then discuss the use cases. If you can't prove your
> > proposal has a proper function, what's the point of discussing the use cases?
> >
> The proper function is described.
> You choose to not accept in favour of considering on the vdpa.

Is the discussion here any relevant to vDPA. I'm telling you to use
the platform IOMMU, how is it related to vDPA?

>
> > > There is absolutely no point of converting virtio device to another
> > virtualization layer and run again and get another virtio device.
> > > So for direct mapping use case shadow vq is not relevant.
> >
> > It is needed because shadow virtqueue is the baseline. Most of the issues
> > don't exist in the case of shadow virtqueue.
> >
> I disagree.
> For direct mapping there is no virtio specific OS layer involved.

Again, you need to prove if direct mapping can work.

And I've pointed out the "direct mapping" of vq for dirty page
tracking can work with "direct mapping" of vIOMMU.

> Hence shadow vq specific implementation is not appliable.
>
> > We don't want to end up with a solution that
> >
> > 1) can't outperform shadow virtqueue
> Disagree. There is no shadow vq in direct mapping. No comparison.
>
> > 2) have more issues than shadow virtqueue
> >
> There are none.

I think I've pointed out sufficient issues, if you choose to ignore
those issues, the discussion is going nowhere.

>
> > > For other use cases, please continue.
> > >
> > > >
> > > > Just an email to Qemu should be fine, we're not asking you to fix the bug.
> > > >
> > > > Btw, how do you define stable? E.g do you think the Linus tree is stable?
> > > >
> > > Basic test with iperf is not working. Crashing it.
> >
> > As a kernel developer, dealing with crashing at any layer is pretty common.
> > No?
> >
> So, kernel developers do not ask to compare the crashing code.

Report it and let the community fix it then compare again.

This is how the community runs.

Thanks