hw/virtio/vhost-user.c | 423 +++++++++++++++++++-------------- hw/virtio/vhost.c | 6 +- include/hw/virtio/vhost-user.h | 3 + 3 files changed, 259 insertions(+), 173 deletions(-)
From: Prasad Pandit <pjp@fedoraproject.org>
Hello,
* virsh(1) offers multiple options to initiate Postcopy migration:
1) virsh migrate --postcopy --postcopy-after-precopy
2) virsh migrate --postcopy + virsh migrate-postcopy
3) virsh migrate --postcopy --timeout <N> --timeout-postcopy
When Postcopy migration is invoked via method (2) or (3) above,
the guest on the destination host seems to hang or get stuck sometimes.
* During Postcopy migration, multiple threads are spawned on the destination
host to start the guest and setup devices. One such thread starts vhost
device via vhost_dev_start() function and another called fault_thread handles
page faults in user space using kernel's userfaultfd(2) system.
When fault_thread exits upon completion of Postcopy migration, it sends a
'postcopy_end' message to the vhost-user device. But sometimes 'postcopy_end'
message is sent while vhost device is being setup via vhost_dev_start().
Thread-1 Thread-2
vhost_dev_start postcopy_ram_incoming_cleanup
vhost_device_iotlb_miss postcopy_notify
vhost_backend_update_device_iotlb vhost_user_postcopy_notifier
vhost_user_send_device_iotlb_msg vhost_user_postcopy_end
process_message_reply process_message_reply
vhost_user_read vhost_user_read
vhost_user_read_header vhost_user_read_header
"Fail to update device iotlb" "Failed to receive reply to postcopy_end"
This creates confusion when vhost device receives 'postcopy_end' message while
it is still trying to update IOTLB entries.
This seems to leave the guest in a stranded/hung state because fault_thread
has exited saying Postcopy migration has ended, but vhost-device is probably
still expecting updates. QEMU logs following errors on the destination host
===
...
qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x0 instead of 0x5.
qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb
qemu-kvm: vhost_user_postcopy_end: 700871,700900: Failed to receive reply to postcopy_end
qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x0 instead of 0x5.
qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb
qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x8 instead of 0x5.
qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb
qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x16 instead of 0x5.
qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb
qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x0 instead of 0x5.
qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb
===
* Couple of patches here help to fix/handle these errors.
Thank you.
---
Prasad Pandit (2):
vhost-user: add a write-read lock
vhost: fail device start if iotlb update fails
hw/virtio/vhost-user.c | 423 +++++++++++++++++++--------------
hw/virtio/vhost.c | 6 +-
include/hw/virtio/vhost-user.h | 3 +
3 files changed, 259 insertions(+), 173 deletions(-)
--
2.45.2
On Thu, Jul 11, 2024 at 06:44:22PM +0530, Prasad Pandit wrote: > From: Prasad Pandit <pjp@fedoraproject.org> > > Hello, > > * virsh(1) offers multiple options to initiate Postcopy migration: > > 1) virsh migrate --postcopy --postcopy-after-precopy > 2) virsh migrate --postcopy + virsh migrate-postcopy > 3) virsh migrate --postcopy --timeout <N> --timeout-postcopy > > When Postcopy migration is invoked via method (2) or (3) above, > the guest on the destination host seems to hang or get stuck sometimes. > > * During Postcopy migration, multiple threads are spawned on the destination > host to start the guest and setup devices. One such thread starts vhost > device via vhost_dev_start() function and another called fault_thread handles Hmm, I thought it was one of the vcpu threads that invoked vhost_dev_start(), rather than any migration thread? > page faults in user space using kernel's userfaultfd(2) system. > > When fault_thread exits upon completion of Postcopy migration, it sends a > 'postcopy_end' message to the vhost-user device. But sometimes 'postcopy_end' > message is sent while vhost device is being setup via vhost_dev_start(). > > Thread-1 Thread-2 > > vhost_dev_start postcopy_ram_incoming_cleanup > vhost_device_iotlb_miss postcopy_notify > vhost_backend_update_device_iotlb vhost_user_postcopy_notifier > vhost_user_send_device_iotlb_msg vhost_user_postcopy_end > process_message_reply process_message_reply > vhost_user_read vhost_user_read > vhost_user_read_header vhost_user_read_header > "Fail to update device iotlb" "Failed to receive reply to postcopy_end" > > This creates confusion when vhost device receives 'postcopy_end' message while > it is still trying to update IOTLB entries. > > This seems to leave the guest in a stranded/hung state because fault_thread > has exited saying Postcopy migration has ended, but vhost-device is probably > still expecting updates. QEMU logs following errors on the destination host > === > ... > qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x0 instead of 0x5. > qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb > qemu-kvm: vhost_user_postcopy_end: 700871,700900: Failed to receive reply to postcopy_end > qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x0 instead of 0x5. > qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb > qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x8 instead of 0x5. > qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb > qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x16 instead of 0x5. > qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb > qemu-kvm: vhost_user_read_header: 700871,700871: Failed to read msg header. Flags 0x0 instead of 0x5. > qemu-kvm: vhost_device_iotlb_miss: 700871,700871: Fail to update device iotlb > === > > * Couple of patches here help to fix/handle these errors. I remember after you added the rwlock, there's still a hang issue. Did you investigated that? Or do you mean this series will fix all the problems? Thanks, > > Thank you. > --- > Prasad Pandit (2): > vhost-user: add a write-read lock > vhost: fail device start if iotlb update fails > > hw/virtio/vhost-user.c | 423 +++++++++++++++++++-------------- > hw/virtio/vhost.c | 6 +- > include/hw/virtio/vhost-user.h | 3 + > 3 files changed, 259 insertions(+), 173 deletions(-) > > -- > 2.45.2 > -- Peter Xu
On Thu, 11 Jul 2024 at 21:08, Peter Xu <peterx@redhat.com> wrote:
> Hmm, I thought it was one of the vcpu threads that invoked
> vhost_dev_start(), rather than any migration thread?
[QEMU=vhost-user-front-end] <===========> [QEMU=vhost-user-front-end]
^
|
|
|
|
|
|
V
[external-process=vhost-user-back-end]
[external-process=vhost-user-back-end]
===
vhost-user-protocol:
-> https://www.qemu.org/docs/master/interop/vhost-user.html#vhost-user-proto
* It is not clear which thread calls vhost_dev_start() routine, it
could be a vCPU thread. Sending 'postcopy_end' message to the
'vhost-user-back-end', hints that the device was being migrated and
migration finished before the device set-up was done. The protocol
above says
"...The nature of the channel is implementation-defined, but it
must generally behave like a pipe: The writing end will write all the
data it has into it, signalling the end of data by closing its end.
The reading end must read all of this data (until encountering the end
of file) and process it."
* It does not mention sending the 'postcopy_end' message. But it talks
about the front-end sending 'VHOST_USER_CHECK_DEVICE_STATE' to the
back-end to check if the migration of the device state was successful
or not.
> I remember after you added the rwlock, there's still a hang issue.
> Did you investigated that? Or do you mean this series will fix all the problems?
* No, this series does not fix the guest hang issue. Root cause of
that is still a mystery. If migration is ending abruptly before all of
the guest state is migrated, the guest hang scenario seems possible.
Adding vhost-user-rw-lock does not address the issue of end of
migration.
* From the protocol page above, it is not clear if the front-end
should allow/have multiple threads talking to the same vhost-user
device.
Thank you.
---
- Prasad
On Mon, Jul 15, 2024 at 03:44:06PM +0530, Prasad Pandit wrote: > > I remember after you added the rwlock, there's still a hang issue. > > Did you investigated that? Or do you mean this series will fix all the problems? > > * No, this series does not fix the guest hang issue. Root cause of > that is still a mystery. If migration is ending abruptly before all of > the guest state is migrated, the guest hang scenario seems possible. > Adding vhost-user-rw-lock does not address the issue of end of > migration. IMHO it's better we debug and fix all the issues before merging this one, otherwise we may overlook something. You could pass over the patch to whoever going to debug this, so it will be included in the whole set to be posted when the bug is completely fixed. > * From the protocol page above, it is not clear if the front-end > should allow/have multiple threads talking to the same vhost-user > device. The protocol should have no restriction on the thread model of a front-end. It only describes the wire protocol. IIUC the protocol was designed to be serialized by nature (where there's no request ID, so we can't match reply to any of the previous response), then the front-end can manage the threads well to serialize all the requests, like using this rwlock. Thanks, -- Peter Xu
Hello Peter, On Mon, 15 Jul 2024 at 19:10, Peter Xu <peterx@redhat.com> wrote: > IMHO it's better we debug and fix all the issues before merging this one, > otherwise we may overlook something. * Well we don't know where the issue is, not sure where the fix may go in, ex. if the issue turns out to be how virsh(1) invokes migrate-postcopy, fix may go in virsh(1). Patches in this series anyway don't help to fix the migration convergence issue, so they could be reviewed independently I guess. > You could pass over the patch to whoever going to debug this, so it will be included in the whole set to be > posted when the bug is completely fixed. * Yes, this patch series is linked there. > The protocol should have no restriction on the thread model of a front-end. > It only describes the wire protocol. > > IIUC the protocol was designed to be serialized by nature (where there's no > request ID, so we can't match reply to any of the previous response), then > the front-end can manage the threads well to serialize all the requests, > like using this rwlock. * I see, okay. The simple protocol definition seems to indicate that it is meant for one front-end/back-end pair. If we are dividing the front-end across multiple threads, maybe we need a document to describe those threads and how they work, at least for the QEMU (front-end) side. Because the back-end could be a non-QEMU process, we can not do much there. (just thinking) Thank you. --- - Prasad
On Tue, Jul 16, 2024 at 03:44:54PM +0530, Prasad Pandit wrote: > Hello Peter, > > On Mon, 15 Jul 2024 at 19:10, Peter Xu <peterx@redhat.com> wrote: > > IMHO it's better we debug and fix all the issues before merging this one, > > otherwise we may overlook something. > > * Well we don't know where the issue is, not sure where the fix may go > in, ex. if the issue turns out to be how virsh(1) invokes > migrate-postcopy, fix may go in virsh(1). Patches in this series > anyway don't help to fix the migration convergence issue, so they > could be reviewed independently I guess. I still think we should find a complete solution before merging anything, because I'm not 100% confident the issue to be further investigated is irrelevant to this patch. No strong opinions, I'll leave that to Michael to decide. > > > You could pass over the patch to whoever going to debug this, so it will be included in the whole set to be > > posted when the bug is completely fixed. > > * Yes, this patch series is linked there. > > > The protocol should have no restriction on the thread model of a front-end. > > It only describes the wire protocol. > > > > IIUC the protocol was designed to be serialized by nature (where there's no > > request ID, so we can't match reply to any of the previous response), then > > the front-end can manage the threads well to serialize all the requests, > > like using this rwlock. > > * I see, okay. The simple protocol definition seems to indicate that > it is meant for one front-end/back-end pair. If we are dividing the > front-end across multiple threads, maybe we need a document to > describe those threads and how they work, at least for the QEMU > (front-end) side. Because the back-end could be a non-QEMU process, we > can not do much there. (just thinking) IMHO that's not part of the protocol but impl details, so the current doc looks all fine to me. Thanks, -- Peter Xu
On Tue, Jul 16, 2024 at 06:02:50PM -0400, Peter Xu wrote: > On Tue, Jul 16, 2024 at 03:44:54PM +0530, Prasad Pandit wrote: > > Hello Peter, > > > > On Mon, 15 Jul 2024 at 19:10, Peter Xu <peterx@redhat.com> wrote: > > > IMHO it's better we debug and fix all the issues before merging this one, > > > otherwise we may overlook something. > > > > * Well we don't know where the issue is, not sure where the fix may go > > in, ex. if the issue turns out to be how virsh(1) invokes > > migrate-postcopy, fix may go in virsh(1). Patches in this series > > anyway don't help to fix the migration convergence issue, so they > > could be reviewed independently I guess. > > I still think we should find a complete solution before merging anything, > because I'm not 100% confident the issue to be further investigated is > irrelevant to this patch. > > No strong opinions, I'll leave that to Michael to decide. > > > > > > You could pass over the patch to whoever going to debug this, so it will be included in the whole set to be > > > posted when the bug is completely fixed. > > > > * Yes, this patch series is linked there. > > > > > The protocol should have no restriction on the thread model of a front-end. > > > It only describes the wire protocol. > > > > > > IIUC the protocol was designed to be serialized by nature (where there's no > > > request ID, so we can't match reply to any of the previous response), then > > > the front-end can manage the threads well to serialize all the requests, > > > like using this rwlock. > > > > * I see, okay. The simple protocol definition seems to indicate that > > it is meant for one front-end/back-end pair. If we are dividing the > > front-end across multiple threads, maybe we need a document to > > describe those threads and how they work, at least for the QEMU > > (front-end) side. Because the back-end could be a non-QEMU process, we > > can not do much there. (just thinking) > > IMHO that's not part of the protocol but impl details, so the current doc > looks all fine to me. > > Thanks, > > -- > Peter Xu I just want to understand how we managed to have two threads talking in parallel. BQL is normally enough, which path manages to invoke vhost-user with BQL not taken? Just check BQL taken on each vhost user invocation and you will figure it out. -- MST
Hi, Michael,
On Wed, Jul 17, 2024 at 04:55:52AM -0400, Michael S. Tsirkin wrote:
> I just want to understand how we managed to have two threads
> talking in parallel. BQL is normally enough, which path
> manages to invoke vhost-user with BQL not taken?
> Just check BQL taken on each vhost user invocation and
> you will figure it out.
Prasad mentioned how the race happened in the cover letter:
https://lore.kernel.org/r/20240711131424.181615-1-ppandit@redhat.com
Thread-1 Thread-2
vhost_dev_start postcopy_ram_incoming_cleanup
vhost_device_iotlb_miss postcopy_notify
vhost_backend_update_device_iotlb vhost_user_postcopy_notifier
vhost_user_send_device_iotlb_msg vhost_user_postcopy_end
process_message_reply process_message_reply
vhost_user_read vhost_user_read
vhost_user_read_header vhost_user_read_header
"Fail to update device iotlb" "Failed to receive reply to postcopy_end"
The normal case should be that thread-2 is postcopy_ram_listen_thread(),
and this happens when postcopy migration is close to the end.
Thanks,
--
Peter Xu
On Wed, Jul 17, 2024 at 09:33:01AM -0400, Peter Xu wrote: > Hi, Michael, > > On Wed, Jul 17, 2024 at 04:55:52AM -0400, Michael S. Tsirkin wrote: > > I just want to understand how we managed to have two threads > > talking in parallel. BQL is normally enough, which path > > manages to invoke vhost-user with BQL not taken? > > Just check BQL taken on each vhost user invocation and > > you will figure it out. > > Prasad mentioned how the race happened in the cover letter: > > https://lore.kernel.org/r/20240711131424.181615-1-ppandit@redhat.com > > Thread-1 Thread-2 > > vhost_dev_start postcopy_ram_incoming_cleanup > vhost_device_iotlb_miss postcopy_notify > vhost_backend_update_device_iotlb vhost_user_postcopy_notifier > vhost_user_send_device_iotlb_msg vhost_user_postcopy_end > process_message_reply process_message_reply > vhost_user_read vhost_user_read > vhost_user_read_header vhost_user_read_header > "Fail to update device iotlb" "Failed to receive reply to postcopy_end" > > The normal case should be that thread-2 is postcopy_ram_listen_thread(), > and this happens when postcopy migration is close to the end. > > Thanks, > > -- > Peter Xu OK, so postcopy_ram_ things run without the BQL? -- MST
On Wed, Jul 17, 2024 at 09:40:06AM -0400, Michael S. Tsirkin wrote: > On Wed, Jul 17, 2024 at 09:33:01AM -0400, Peter Xu wrote: > > Hi, Michael, > > > > On Wed, Jul 17, 2024 at 04:55:52AM -0400, Michael S. Tsirkin wrote: > > > I just want to understand how we managed to have two threads > > > talking in parallel. BQL is normally enough, which path > > > manages to invoke vhost-user with BQL not taken? > > > Just check BQL taken on each vhost user invocation and > > > you will figure it out. > > > > Prasad mentioned how the race happened in the cover letter: > > > > https://lore.kernel.org/r/20240711131424.181615-1-ppandit@redhat.com > > > > Thread-1 Thread-2 > > > > vhost_dev_start postcopy_ram_incoming_cleanup > > vhost_device_iotlb_miss postcopy_notify > > vhost_backend_update_device_iotlb vhost_user_postcopy_notifier > > vhost_user_send_device_iotlb_msg vhost_user_postcopy_end > > process_message_reply process_message_reply > > vhost_user_read vhost_user_read > > vhost_user_read_header vhost_user_read_header > > "Fail to update device iotlb" "Failed to receive reply to postcopy_end" > > > > The normal case should be that thread-2 is postcopy_ram_listen_thread(), > > and this happens when postcopy migration is close to the end. > > > > Thanks, > > > > -- > > Peter Xu > > > OK, so postcopy_ram_ things run without the BQL? There are a lot of postcopy_ram_* functions, I didn't check all of them but I think it's true in this case. Thanks. -- Peter Xu
© 2016 - 2024 Red Hat, Inc.