On 27.08.25 23:59, Peter Xu wrote:
Migration module was there for 10+ years. Initially, it was in most cases
based on coroutines. As more features were added into the framework, like
postcopy, multifd, etc.. it became a mixture of threads and coroutines.
I'm guessing coroutines just can't fix all issues that migration want to
resolve.
After all these years, migration is now heavily based on a threaded model.
Now there's still a major part of migration framework that is still not
thread-based, which is precopy load. We do load in a separate thread in
postcopy since the 1st day postcopy was introduced, however that requires a
separate state transition from precopy loading all devices first, which
still happens in the main thread of a coroutine.
This patch tries to move the migration incoming side to be run inside a
separate thread (mig/dst/main) just like the src (mig/src/main). The
entrance to be migration_incoming_thread().
Quite a few things are needed to make it fly..
BQL Analysis
============
Firstly, when moving it over to the thread, it means the thread cannot take
BQL during the whole process of loading anymore, because otherwise it can
block main thread from using the BQL for all kinds of other concurrent
tasks (for example, processing QMP / HMP commands).
Here the first question to ask is: what needs BQL during precopy load, and
what doesn't?
Most of the load process shouldn't need BQL, especially when it's about
RAM. After all, RAM is still the major chunk of data to move for a live
migration process. VFIO started to change that, though, but still, VFIO is
per-device so that shouldn't need BQL either in most cases.
Generic device loads will need BQL, likely not when receiving VMSDs, but
when applying them. One example is any post_load() could potentially
inject memory regions causing memory transactions to happen. That'll need
to update the global address spaces, hence requires BQL. The other one is
CPU sync operations, even if the sync alone may not need BQL (which is
still to be further justified), run_on_cpu() will need it.
For that, qemu_loadvm_state() and qemu_loadvm_state_main() functions need
to now take a "bql_held" parameter saying whether bql is held. We could
use things like BQL_LOCK_GUARD(), but this patch goes with explicit
lockings rather than relying on bql_locked TLS variable. In case of
migration, we always know whether BQL is held in different context as long
as we can still pass that information downwards.
Agree, but I think it's better to make new macros following same pattern, i.e.
WITH_BQL_HELD(bql_held) {
action();
}
instead of
WITH_BQL_HELD(bql_held, actions());
..
Or I'm missing something and we already have a precedent of the latter
notation?
--
Best regards,
Vladimir
