在 2024/9/27 23:35, Peter Xu 写道:
On Fri, Sep 27, 2024 at 10:50:01AM +0800, Yong Huang wrote:
On Fri, Sep 27, 2024 at 3:55 AM Peter Xu <pet...@redhat.com> wrote:

On Fri, Sep 27, 2024 at 02:13:47AM +0800, Yong Huang wrote:
On Thu, Sep 26, 2024 at 3:17 AM Peter Xu <pet...@redhat.com> wrote:

On Fri, Sep 20, 2024 at 10:43:31AM +0800, Yong Huang wrote:
Yes, invoke migration_bitmap_sync_precopy more frequently is also my
first idea but it involves bitmap updating and interfere with the
behavior
of page sending, it also affects the migration information stats and
interfere other migration logic such as migration_update_rates().
Could you elaborate?

For example, what happens if we start to sync in ram_save_iterate() for
some time intervals (e.g. 5 seconds)?

I didn't try to sync in ram_save_iterate but in the
migration_bitmap_sync_precopy.

If we use the migration_bitmap_sync_precopy in the ram_save_iterate
function,
This approach seems to be correct. However, the bitmap will be updated as
the
migration thread iterates through each dirty page in the RAMBlock list.
Compared
to the existing implementation, this is different but still
straightforward;
I'll give it a shot soon to see if it works.
It's still serialized in the migration thread, so I'd expect it is similar

What does "serialized" mean?
I meant sync() never happens before concurrently with RAM pages being
iterated, simply because sync() previously only happens in the migration
thread, which is still the same thread that initiate the movement of pages.

How about we:
1. invoke the migration_bitmap_sync_precopy in a timer(bg_sync_timer) hook,
    every 5 seconds.
2. register the bg_sync_timer in the main loop when the machine starts like
     throttle_timer
3. activate the timer when ram_save_iterate gets called and deactivate it in
     the ram_save_cleanup gracefully during migration.

I think it is simple enough and also isn't "serialized"?
If you want to do that with timer that's ok, but then IIUC it doesn't need
to involve ram.c code at all.

You can rely on cpu_throttle_get_percentage() too just like the throttle
timer, and it'll work naturally with migration because outside migration
the throttle will be cleared (cpu_throttle_stop() at finish/fail/cancel..).

Then it also gracefully align the async thread sync() that it only happens
with auto-converge is enabled.  Yeh that may look better.. and stick the
code together with cpu-throttle.c seems nice.

Ok, Thanks for the advices, i'll check it and see how it goes.


Side note: one thing regarind to sync() is ram_init_bitmaps() sync once,
while I don't see why it's necessary.  I remember I tried to remove it but
maybe I hit some issues and I didn't dig further.  If you're working on
sync() anyway not sure whether you'd like to have a look.

Agree, I'll try it after working out current series.


Yong


Reply via email to