| Message ID | 20231124171430.2964464-1-eperezma@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Consolidate common vdpa members in VhostVDPAShared | expand |
Hi Eugenio QE performed regression testing after applying this patch. This series patch introduced a qemu core dump bug, for the core dump information please review the attached file. Tested-by: Lei Yang <leiyang@redhat.com> On Sat, Nov 25, 2023 at 1:14 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > Current memory operations like pinning may take a lot of time at the > destination. Currently they are done after the source of the migration is > stopped, and before the workload is resumed at the destination. This is a > period where neigher traffic can flow, nor the VM workload can continue > (downtime). > > We can do better as we know the memory layout of the guest RAM at the > destination from the moment the migration starts. Moving that operation allows > QEMU to communicate the kernel the maps while the workload is still running in > the source, so Linux can start mapping them. Ideally, all IOMMU is configured, > but if the vDPA parent driver uses on-chip IOMMU and .set_map we're still > saving all the pinning time. > > This is a first required step to consolidate all the members in a common > struct. This is needed because the destination does not know what vhost_vdpa > struct will have the registered listener member, so it is easier to place them > in a shared struct rather to keep them in vhost_vdpa struct. > > v1 from RFC: > * Fix vhost_vdpa_net_cvq_start checking for always_svq instead of > shadow_data. This could cause CVQ not being shadowed if > vhost_vdpa_net_cvq_start was called in the middle of a migration. > > Eugenio Pérez (13): > vdpa: add VhostVDPAShared > vdpa: move iova tree to the shared struct > vdpa: move iova_range to vhost_vdpa_shared > vdpa: move shadow_data to vhost_vdpa_shared > vdpa: use vdpa shared for tracing > vdpa: move file descriptor to vhost_vdpa_shared > vdpa: move iotlb_batch_begin_sent to vhost_vdpa_shared > vdpa: move backend_cap to vhost_vdpa_shared > vdpa: remove msg type of vhost_vdpa > vdpa: move iommu_list to vhost_vdpa_shared > vdpa: use VhostVDPAShared in vdpa_dma_map and unmap > vdpa: use dev_shared in vdpa_iommu > vdpa: move memory listener to vhost_vdpa_shared > > include/hw/virtio/vhost-vdpa.h | 36 +++++--- > hw/virtio/vdpa-dev.c | 7 +- > hw/virtio/vhost-vdpa.c | 160 +++++++++++++++++---------------- > net/vhost-vdpa.c | 117 ++++++++++++------------ > hw/virtio/trace-events | 14 +-- > 5 files changed, 174 insertions(+), 160 deletions(-) > > -- > 2.39.3 > >
On Thu, Nov 30, 2023 at 4:22 AM Lei Yang <leiyang@redhat.com> wrote: > > Hi Eugenio > > QE performed regression testing after applying this patch. This series > patch introduced a qemu core dump bug, for the core dump information > please review the attached file. > Hi Lei, thank you very much for the testing! Can you describe the test steps that lead to the crash? I think you removed the vdpa device via QMP, but I'd like to be sure. Thanks! > Tested-by: Lei Yang <leiyang@redhat.com> > > > > > On Sat, Nov 25, 2023 at 1:14 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > Current memory operations like pinning may take a lot of time at the > > destination. Currently they are done after the source of the migration is > > stopped, and before the workload is resumed at the destination. This is a > > period where neigher traffic can flow, nor the VM workload can continue > > (downtime). > > > > We can do better as we know the memory layout of the guest RAM at the > > destination from the moment the migration starts. Moving that operation allows > > QEMU to communicate the kernel the maps while the workload is still running in > > the source, so Linux can start mapping them. Ideally, all IOMMU is configured, > > but if the vDPA parent driver uses on-chip IOMMU and .set_map we're still > > saving all the pinning time. > > > > This is a first required step to consolidate all the members in a common > > struct. This is needed because the destination does not know what vhost_vdpa > > struct will have the registered listener member, so it is easier to place them > > in a shared struct rather to keep them in vhost_vdpa struct. > > > > v1 from RFC: > > * Fix vhost_vdpa_net_cvq_start checking for always_svq instead of > > shadow_data. This could cause CVQ not being shadowed if > > vhost_vdpa_net_cvq_start was called in the middle of a migration. > > > > Eugenio Pérez (13): > > vdpa: add VhostVDPAShared > > vdpa: move iova tree to the shared struct > > vdpa: move iova_range to vhost_vdpa_shared > > vdpa: move shadow_data to vhost_vdpa_shared > > vdpa: use vdpa shared for tracing > > vdpa: move file descriptor to vhost_vdpa_shared > > vdpa: move iotlb_batch_begin_sent to vhost_vdpa_shared > > vdpa: move backend_cap to vhost_vdpa_shared > > vdpa: remove msg type of vhost_vdpa > > vdpa: move iommu_list to vhost_vdpa_shared > > vdpa: use VhostVDPAShared in vdpa_dma_map and unmap > > vdpa: use dev_shared in vdpa_iommu > > vdpa: move memory listener to vhost_vdpa_shared > > > > include/hw/virtio/vhost-vdpa.h | 36 +++++--- > > hw/virtio/vdpa-dev.c | 7 +- > > hw/virtio/vhost-vdpa.c | 160 +++++++++++++++++---------------- > > net/vhost-vdpa.c | 117 ++++++++++++------------ > > hw/virtio/trace-events | 14 +-- > > 5 files changed, 174 insertions(+), 160 deletions(-) > > > > -- > > 2.39.3 > > > >
On Thu, Nov 30, 2023 at 3:38 PM Eugenio Perez Martin <eperezma@redhat.com> wrote: > > On Thu, Nov 30, 2023 at 4:22 AM Lei Yang <leiyang@redhat.com> wrote: > > > > Hi Eugenio > > > > QE performed regression testing after applying this patch. This series > > patch introduced a qemu core dump bug, for the core dump information > > please review the attached file. > > > > Hi Lei, thank you very much for the testing! > Hi Eugenio > Can you describe the test steps that lead to the crash? I think you > removed the vdpa device via QMP, but I'd like to be sure. Yes, you're right, the core dump occurs when hot unplug nic, please review the following simple test steps: Test Steps: 1. create two vdpa device(vdpa0 and vdpa1) with multi queues 2. Boot a guest with vdpa0 3. set_link false to vdpa0 4. hotplug vdpa1 5. stop and resume guest via QMP 6. hotunplug vdpa1, hit core dump in this time Thanks Lei > > Thanks! > > > Tested-by: Lei Yang <leiyang@redhat.com> > > > > > > > > > > On Sat, Nov 25, 2023 at 1:14 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > > > Current memory operations like pinning may take a lot of time at the > > > destination. Currently they are done after the source of the migration is > > > stopped, and before the workload is resumed at the destination. This is a > > > period where neigher traffic can flow, nor the VM workload can continue > > > (downtime). > > > > > > We can do better as we know the memory layout of the guest RAM at the > > > destination from the moment the migration starts. Moving that operation allows > > > QEMU to communicate the kernel the maps while the workload is still running in > > > the source, so Linux can start mapping them. Ideally, all IOMMU is configured, > > > but if the vDPA parent driver uses on-chip IOMMU and .set_map we're still > > > saving all the pinning time. > > > > > > This is a first required step to consolidate all the members in a common > > > struct. This is needed because the destination does not know what vhost_vdpa > > > struct will have the registered listener member, so it is easier to place them > > > in a shared struct rather to keep them in vhost_vdpa struct. > > > > > > v1 from RFC: > > > * Fix vhost_vdpa_net_cvq_start checking for always_svq instead of > > > shadow_data. This could cause CVQ not being shadowed if > > > vhost_vdpa_net_cvq_start was called in the middle of a migration. > > > > > > Eugenio Pérez (13): > > > vdpa: add VhostVDPAShared > > > vdpa: move iova tree to the shared struct > > > vdpa: move iova_range to vhost_vdpa_shared > > > vdpa: move shadow_data to vhost_vdpa_shared > > > vdpa: use vdpa shared for tracing > > > vdpa: move file descriptor to vhost_vdpa_shared > > > vdpa: move iotlb_batch_begin_sent to vhost_vdpa_shared > > > vdpa: move backend_cap to vhost_vdpa_shared > > > vdpa: remove msg type of vhost_vdpa > > > vdpa: move iommu_list to vhost_vdpa_shared > > > vdpa: use VhostVDPAShared in vdpa_dma_map and unmap > > > vdpa: use dev_shared in vdpa_iommu > > > vdpa: move memory listener to vhost_vdpa_shared > > > > > > include/hw/virtio/vhost-vdpa.h | 36 +++++--- > > > hw/virtio/vdpa-dev.c | 7 +- > > > hw/virtio/vhost-vdpa.c | 160 +++++++++++++++++---------------- > > > net/vhost-vdpa.c | 117 ++++++++++++------------ > > > hw/virtio/trace-events | 14 +-- > > > 5 files changed, 174 insertions(+), 160 deletions(-) > > > > > > -- > > > 2.39.3 > > > > > > >
On Sat, Nov 25, 2023 at 1:14 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > Current memory operations like pinning may take a lot of time at the > destination. Currently they are done after the source of the migration is > stopped, and before the workload is resumed at the destination. This is a > period where neigher traffic can flow, nor the VM workload can continue > (downtime). > > We can do better as we know the memory layout of the guest RAM at the > destination from the moment the migration starts. Moving that operation allows > QEMU to communicate the kernel the maps while the workload is still running in > the source, so Linux can start mapping them. Ideally, all IOMMU is configured, > but if the vDPA parent driver uses on-chip IOMMU and .set_map we're still > saving all the pinning time. > > This is a first required step to consolidate all the members in a common > struct. This is needed because the destination does not know what vhost_vdpa > struct will have the registered listener member, so it is easier to place them > in a shared struct rather to keep them in vhost_vdpa struct. > > v1 from RFC: > * Fix vhost_vdpa_net_cvq_start checking for always_svq instead of > shadow_data. This could cause CVQ not being shadowed if > vhost_vdpa_net_cvq_start was called in the middle of a migration. With the renaming of the VhostVDPAShared to VhostVDPAParent. Acked-by: Jason Wang <jasowang@redhat.com> Thanks > > Eugenio Pérez (13): > vdpa: add VhostVDPAShared > vdpa: move iova tree to the shared struct > vdpa: move iova_range to vhost_vdpa_shared > vdpa: move shadow_data to vhost_vdpa_shared > vdpa: use vdpa shared for tracing > vdpa: move file descriptor to vhost_vdpa_shared > vdpa: move iotlb_batch_begin_sent to vhost_vdpa_shared > vdpa: move backend_cap to vhost_vdpa_shared > vdpa: remove msg type of vhost_vdpa > vdpa: move iommu_list to vhost_vdpa_shared > vdpa: use VhostVDPAShared in vdpa_dma_map and unmap > vdpa: use dev_shared in vdpa_iommu > vdpa: move memory listener to vhost_vdpa_shared > > include/hw/virtio/vhost-vdpa.h | 36 +++++--- > hw/virtio/vdpa-dev.c | 7 +- > hw/virtio/vhost-vdpa.c | 160 +++++++++++++++++---------------- > net/vhost-vdpa.c | 117 ++++++++++++------------ > hw/virtio/trace-events | 14 +-- > 5 files changed, 174 insertions(+), 160 deletions(-) > > -- > 2.39.3 > >