| Message ID | 20241024213056.1395400-1-peterx@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Migration: Make misc.h helpers available for whole VM lifecycle | expand |
On Thu, Oct 24, 2024 at 05:30:48PM -0400, Peter Xu wrote: > Based-on: <20241024165627.1372621-1-peterx@redhat.com> > CI: https://gitlab.com/peterx/qemu/-/pipelines/1511349805 > > This is a follow up of below patch from Avihai as a replacement: > > https://lore.kernel.org/qemu-devel/20241020130108.27148-3-avihaih@nvidia.com/ > > This is v3 of the series, and it happened again: changelog doesn't make > sense to compare to v2 because it's rewritten. Meanwhile, now this series > is based on the other TYPE_SINGLETON series I posted just now: > > https://lore.kernel.org/r/20241024165627.1372621-1-peterx@redhat.com > > It turns out I found more things to cleanup, as the versions spin up. > > Patch 1: I found that object_ref() in migration thread is actually also > racy.. this should fix it. > > Patch 2-7: It turns out I decided to clean things up first, then it'll > make my last patch easier on adding the mutex protection for > the current_migration reference > > Patch 8: The fix for NULL-dereference / race for the exported > functions. VFIO can hit it only because VFIO's specialty > in using migration helpers in vmstate handlers, I guess. I > found most functions always safe because even if they're used > outside migration/ they're most likely still invoked with > migration thread context. So I found only two functions that > really need protections, exactly what VFIO is using. I queued patch 1-5. Ideally 6+7 can be a single patch, then we don't need to move DEVICE into migration_is_active() at all. Also that might break VFIO as Avihai pointed out on VGA sync. Avihai, I'd still appreciate if you could consider look at vfio to behave like what kvm/vhost/.. is doing, by accepting log_sync() before log_start(). Patch 8 still rely on singleton series, which will become rfc to be reposted, so probably no rush on that. > > Comments welcomed, thanks. > > Peter Xu (8): > migration: Take migration object refcount earlier for threads > migration: Unexport dirty_bitmap_mig_init() > migration: Unexport ram_mig_init() > migration: Drop migration_is_setup_or_active() > migration: Drop migration_is_idle() > migration: Drop migration_is_device() > migration: Unexport migration_is_active() > migration: Protect updates to current_migration with a mutex > > include/migration/misc.h | 11 +--- > migration/migration.h | 8 +++ > migration/ram.h | 1 + > hw/vfio/common.c | 6 +- > hw/virtio/virtio-mem.c | 2 +- > migration/migration.c | 118 +++++++++++++++++++-------------------- > migration/ram.c | 7 +-- > net/vhost-vdpa.c | 3 +- > system/dirtylimit.c | 3 +- > system/qdev-monitor.c | 4 +- > 10 files changed, 81 insertions(+), 82 deletions(-) > > -- > 2.45.0 >
Based-on: <20241024165627.1372621-1-peterx@redhat.com> CI: https://gitlab.com/peterx/qemu/-/pipelines/1511349805 This is a follow up of below patch from Avihai as a replacement: https://lore.kernel.org/qemu-devel/20241020130108.27148-3-avihaih@nvidia.com/ This is v3 of the series, and it happened again: changelog doesn't make sense to compare to v2 because it's rewritten. Meanwhile, now this series is based on the other TYPE_SINGLETON series I posted just now: https://lore.kernel.org/r/20241024165627.1372621-1-peterx@redhat.com It turns out I found more things to cleanup, as the versions spin up. Patch 1: I found that object_ref() in migration thread is actually also racy.. this should fix it. Patch 2-7: It turns out I decided to clean things up first, then it'll make my last patch easier on adding the mutex protection for the current_migration reference Patch 8: The fix for NULL-dereference / race for the exported functions. VFIO can hit it only because VFIO's specialty in using migration helpers in vmstate handlers, I guess. I found most functions always safe because even if they're used outside migration/ they're most likely still invoked with migration thread context. So I found only two functions that really need protections, exactly what VFIO is using. Comments welcomed, thanks. Peter Xu (8): migration: Take migration object refcount earlier for threads migration: Unexport dirty_bitmap_mig_init() migration: Unexport ram_mig_init() migration: Drop migration_is_setup_or_active() migration: Drop migration_is_idle() migration: Drop migration_is_device() migration: Unexport migration_is_active() migration: Protect updates to current_migration with a mutex include/migration/misc.h | 11 +--- migration/migration.h | 8 +++ migration/ram.h | 1 + hw/vfio/common.c | 6 +- hw/virtio/virtio-mem.c | 2 +- migration/migration.c | 118 +++++++++++++++++++-------------------- migration/ram.c | 7 +-- net/vhost-vdpa.c | 3 +- system/dirtylimit.c | 3 +- system/qdev-monitor.c | 4 +- 10 files changed, 81 insertions(+), 82 deletions(-)