| Message ID | 20210920230202.1439-9-longpeng2@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | optimize the downtime for vfio migration | expand |
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, 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"); > }
> -----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"); > > }
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"); }
Commit ecebe53fe993 ("vfio: Avoid disabling and enabling vectors repeatedly in VFIO migration") avoid inefficiently disabling and enabling vectors repeatedly and let the unmasked vectors to be 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 cannot to enable the vectors one by one in the loop now. 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(-)