Re: [PATCH RFC 3/4] vl: Sync dirty bits for system resets during precopy

[Dr. David Alan Gilbert]

* Peter Xu (address@hidden) wrote:
> System resets will also reset system memory layout.  Although the memory 
> layout
> after the reset should probably the same as before the reset, we still need to
> do frequent memory section removals and additions during the reset process.
> Those operations could accidentally lose per-mem-section information like KVM
> memslot dirty bitmaps.
> 
> Previously we keep those dirty bitmaps by sync it during memory removal.
> However that's hard to make it right after all [1].  Instead, we sync dirty
> pages before system reset if we know we're during a precopy migration.  This
> should solve the same problem explicitly.
> 
> [1] https://lore.kernel.org/qemu-devel/20200327150425.GJ422390@xz-x1/

I think the problem is knowing whether this is sufficient or whether you
definitely need to do it for other cases of removal/readd.

However, assuming we need to do it during reset, how do we know this is
the right point to do it, and not something further inside the reset
process?  Is this just on the basis that vcpus are stopped?

Dave

> Signed-off-by: Peter Xu <address@hidden>
> ---
>  softmmu/vl.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 32c0047889..8f864fee43 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1387,6 +1387,22 @@ void qemu_system_reset(ShutdownCause reason)
>  
>      cpu_synchronize_all_states();
>  
> +    /*
> +     * System reboot could reset memory layout.  Although the final status of
> +     * the memory layout should be the same as before the reset, the memory
> +     * sections can still be removed and added back frequently due to the 
> reset
> +     * process.  This could potentially drop dirty bits in track for those
> +     * memory sections before the reset.
> +     *
> +     * Do a global dirty sync before the reset happens if we are during a
> +     * precopy, so we don't lose the dirty bits during the memory shuffles.
> +     */
> +    if (migration_is_precopy()) {
> +        WITH_RCU_READ_LOCK_GUARD() {
> +            migration_bitmap_sync_precopy();
> +        }
> +    }
> +
>      if (mc && mc->reset) {
>          mc->reset(current_machine);
>      } else {
> -- 
> 2.24.1
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK