[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v3 8/9] Revert "vfio: Avoid disabling and enabling vectors re
From: |
Longpeng (Mike, Cloud Infrastructure Service Product Dept.) |
Subject: |
RE: [PATCH v3 8/9] Revert "vfio: Avoid disabling and enabling vectors repeatedly in VFIO migration" |
Date: |
Fri, 8 Oct 2021 01:32:33 +0000 |
> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Saturday, October 2, 2021 7:04 AM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <longpeng2@huawei.com>
> Cc: philmd@redhat.com; pbonzini@redhat.com; marcel.apfelbaum@gmail.com;
> mst@redhat.com; qemu-devel@nongnu.org; Gonglei (Arei)
> <arei.gonglei@huawei.com>; chenjiashang <chenjiashang@huawei.com>
> Subject: Re: [PATCH v3 8/9] Revert "vfio: Avoid disabling and enabling vectors
> repeatedly in VFIO migration"
>
> On Tue, 21 Sep 2021 07:02:01 +0800
> "Longpeng(Mike)" <longpeng2@huawei.com> wrote:
>
> > Commit ecebe53fe993 ("vfio: Avoid disabling and enabling vectors
> > repeatedly in VFIO migration") avoid inefficiently disabling and
>
> s/avoid/avoids/
>
> > enabling vectors repeatedly and let the unmasked vectors to be
>
> s/let/lets/ s/to//
>
> > enabled one by one.
> >
> > But we want to batch multiple routes and defer the commit, and only
> > commit once out side the loop of setting vector notifiers, so we
>
> s/out side/outside/
>
> > cannot to enable the vectors one by one in the loop now.
>
> s/to//
>
Thanks. All the typos and grammatical errors you pointed out will
be fixed in v4.
> Thanks,
> Alex
>
> >
> > Revert that commit and we will take another way in the next patch,
> > it can not only avoid disabling/enabling vectors repeatedly, but
> > also satisfy our requirement of defer to commit.
> >
> > Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> > ---
> > hw/vfio/pci.c | 20 +++-----------------
> > 1 file changed, 3 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index 8fe238b11d..2de1cc5425 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -610,9 +610,6 @@ static __attribute__((unused)) void
> vfio_commit_kvm_msi_virq(VFIOPCIDevice *vdev
> >
> > static void vfio_msix_enable(VFIOPCIDevice *vdev)
> > {
> > - PCIDevice *pdev = &vdev->pdev;
> > - unsigned int nr, max_vec = 0;
> > -
> > vfio_disable_interrupts(vdev);
> >
> > vdev->msi_vectors = g_new0(VFIOMSIVector, vdev->msix->entries);
> > @@ -631,22 +628,11 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
> > * triggering to userspace, then immediately release the vector,
> > leaving
> > * the physical device with no vectors enabled, but MSI-X enabled, just
> > * like the guest view.
> > - * If there are already unmasked vectors (in migration resume phase and
> > - * some guest startups) which will be enabled soon, we can allocate all
> > - * of them here to avoid inefficiently disabling and enabling vectors
> > - * repeatedly later.
> > */
> > - if (!pdev->msix_function_masked) {
> > - for (nr = 0; nr < msix_nr_vectors_allocated(pdev); nr++) {
> > - if (!msix_is_masked(pdev, nr)) {
> > - max_vec = nr;
> > - }
> > - }
> > - }
> > - vfio_msix_vector_do_use(pdev, max_vec, NULL, NULL);
> > - vfio_msix_vector_release(pdev, max_vec);
> > + vfio_msix_vector_do_use(&vdev->pdev, 0, NULL, NULL);
> > + vfio_msix_vector_release(&vdev->pdev, 0);
> >
> > - if (msix_set_vector_notifiers(pdev, vfio_msix_vector_use,
> > + if (msix_set_vector_notifiers(&vdev->pdev, vfio_msix_vector_use,
> > vfio_msix_vector_release, NULL)) {
> > error_report("vfio: msix_set_vector_notifiers failed");
> > }