On Tue, Feb 04, 2025 at 03:57:37PM +0100, Maciej S. Szmigiero wrote: > The vfio_migration_cleanup() used to just close a migration FD, while > RAM might end up calling qemu_ram_msync(), which sounds like something > that should be called under BQL. > > But I am not sure whether that lack of BQL around qemu_ram_msync() > actually causes problems.
I believe msync() is thread-safe. So that doesn't need BQL, AFAICT. Personally I actually prefer not having the BQL requirement if ever possible in any vmstate hooks. I think the only challenge here is if VFIO will start to need BQL for some specific code path that you added in this series, it means VFIO needs to detect bql_locked() to make sure it won't deadlock.. and only take BQL if it's not taken. >From that POV, it might be easier for you to define that hook as "always do cleanup() with BQL" globally, just to avoid one bql_locked() usage in vfio specific hook. We pay that with slow RAM sync in corner cases like pmem that could potentially block VM from making progress (e.g. vcpu concurrently accessing MMIO regions). Thanks, -- Peter Xu
