Re: [PATCH RFC 1/9] migration/vfio: Remove BQL implication in vfio_multifd_switchover_start()

[Peter Xu <peterx@redhat.com>]

On Thu, Aug 28, 2025 at 08:05:46PM +0200, Maciej S. Szmigiero wrote:
> On 27.08.2025 22:59, Peter Xu wrote:
> > We may switch to a BQL-free loadvm model.  Be prepared with it.
> > 
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Cc: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >   hw/vfio/migration-multifd.c | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
> > index e4785031a7..8dc8444f0d 100644
> > --- a/hw/vfio/migration-multifd.c
> > +++ b/hw/vfio/migration-multifd.c
> > @@ -763,16 +763,21 @@ int vfio_multifd_switchover_start(VFIODevice *vbasedev)
> >   {
> >       VFIOMigration *migration = vbasedev->migration;
> >       VFIOMultifd *multifd = migration->multifd;
> > +    bool bql_is_locked = bql_locked();
> >       assert(multifd);
> >       /* The lock order is load_bufs_mutex -> BQL so unlock BQL here first */
> > -    bql_unlock();
> > +    if (bql_is_locked) {
> > +        bql_unlock();
> > +    }
> >       WITH_QEMU_LOCK_GUARD(&multifd->load_bufs_mutex) {
> >           assert(!multifd->load_bufs_thread_running);
> >           multifd->load_bufs_thread_running = true;
> >       }
> > -    bql_lock();
> > +    if (bql_is_locked) {
> > +        bql_lock();
> > +    }
> >       qemu_loadvm_start_load_thread(vfio_load_bufs_thread, vbasedev);
> 
> This patch makes sense to me - I don't see anything obviously wrong here.
> 
> In general, thank you for your series Peter.
> 
> I am actually looking at a similar subject - how to make
> vfio_pci_load_config() and its sub-calls use more fine grained locking
> than BQL so device configuration loading for multiple VFIO devices
> can happen in parallel instead of being serialized by BQL.
> 
> Don't have an ETA for this yet, but it's good that other people also
> working on improving live migration scalability.

Thanks for the early quick comment on this, Maciej.  Not sure if what
you're working on is about memory transactions, if so it sounds challenging
to completely get rid of BQL.  Maybe BQL sections can be shrinked, though.

Replying here just for a quick update on this patch for when I'd repost: I
plan to drop this patch, because after switching to a thread model then
this function will constantly run without BQL. It means what I added here
above will be dead code that always have bql_is_locked==false.

Hence I decided to do it the other way round to add one vfio patch at the
end removing the BQL dependency completely.

Thanks,

-- 
Peter Xu