| Message ID | 1720792931-456433-3-git-send-email-steven.sistare@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Live update: vdpa | expand |
Steve Sistare <steven.sistare@oracle.com> writes: > CPR preserves memory in place, so there is no need to track dirty memory. > By skipping it, CPR can support devices that do not support tracking. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > --- > system/memory.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/system/memory.c b/system/memory.c > index b7548bf112..aef584e638 100644 > --- a/system/memory.c > +++ b/system/memory.c > @@ -27,6 +27,7 @@ > > #include "exec/memory-internal.h" > #include "exec/ram_addr.h" > +#include "migration/misc.h" > #include "sysemu/kvm.h" > #include "sysemu/runstate.h" > #include "sysemu/tcg.h" > @@ -2947,6 +2948,11 @@ bool memory_global_dirty_log_start(unsigned int flags, Error **errp) > > assert(flags && !(flags & (~GLOBAL_DIRTY_MASK))); > > + /* CPR preserves memory in place, so no need to track dirty memory */ > + if (migrate_mode() != MIG_MODE_NORMAL) { > + return true; > + } How this interacts with DIRTY_RATE and DIRTY_LIMIT? The former at least seems to never overlap with CPR, right? I'm wondering whether this check would be more appropriate up in ram.c along with the similar migrate_background_snapshot() check. (I wish we had made the global_dirty_log_change() function a bit more flexible. It would have been a nice place to put this and the snapshot check. Not worth the risk of changing it now...) Also, not tracking dirty memory implies also not doing the bitmap sync? We skip it for bg_snapshot, but not for CPR. > + > if (vmstate_change) { > /* If there is postponed stop(), operate on it first */ > postponed_stop_flags &= ~flags; > @@ -3021,6 +3027,11 @@ static void memory_vm_change_state_handler(void *opaque, bool running, > > void memory_global_dirty_log_stop(unsigned int flags) > { > + /* CPR preserves memory in place, so no need to track dirty memory */ > + if (migrate_mode() != MIG_MODE_NORMAL) { > + return; > + } > + > if (!runstate_is_running()) { > /* Postpone the dirty log stop, e.g., to when VM starts again */ > if (vmstate_change) {
On 8/12/2024 2:57 PM, Fabiano Rosas wrote: > Steve Sistare <steven.sistare@oracle.com> writes: > >> CPR preserves memory in place, so there is no need to track dirty memory. >> By skipping it, CPR can support devices that do not support tracking. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >> --- >> system/memory.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/system/memory.c b/system/memory.c >> index b7548bf112..aef584e638 100644 >> --- a/system/memory.c >> +++ b/system/memory.c >> @@ -27,6 +27,7 @@ >> >> #include "exec/memory-internal.h" >> #include "exec/ram_addr.h" >> +#include "migration/misc.h" >> #include "sysemu/kvm.h" >> #include "sysemu/runstate.h" >> #include "sysemu/tcg.h" >> @@ -2947,6 +2948,11 @@ bool memory_global_dirty_log_start(unsigned int flags, Error **errp) >> >> assert(flags && !(flags & (~GLOBAL_DIRTY_MASK))); >> >> + /* CPR preserves memory in place, so no need to track dirty memory */ >> + if (migrate_mode() != MIG_MODE_NORMAL) { >> + return true; >> + } > > How this interacts with DIRTY_RATE and DIRTY_LIMIT? The former at least > seems to never overlap with CPR, right? I'm wondering whether this check > would be more appropriate up in ram.c along with the similar > migrate_background_snapshot() check. Agreed. I previously pushed the CPR check down to memory_global_dirty_log_start to catch all callers, but the only callers that matter are ram_init_bitmaps and ram_save_cleanup, so I will hoist the check to those callers. > (I wish we had made the global_dirty_log_change() function a bit more > flexible. It would have been a nice place to put this and the snapshot > check. Not worth the risk of changing it now...) > > Also, not tracking dirty memory implies also not doing the bitmap sync? > We skip it for bg_snapshot, but not for CPR. Good catch, I must skip it for CPR also: -------------------------- @@ -3250,7 +3261,8 @@ static int ram_save_complete(QEMUFile *f, void *opaque) rs->last_stage = !migration_in_colo_state(); WITH_RCU_READ_LOCK_GUARD() { - if (!migration_in_postcopy()) { + /* We don't use dirty log with CPR. */ + if (!migration_in_postcopy() && migrate_mode() == MIG_MODE_NORMAL) { migration_bitmap_sync_precopy(rs, true); } --------------------------- - Steve >> + >> if (vmstate_change) { >> /* If there is postponed stop(), operate on it first */ >> postponed_stop_flags &= ~flags; >> @@ -3021,6 +3027,11 @@ static void memory_vm_change_state_handler(void *opaque, bool running, >> >> void memory_global_dirty_log_stop(unsigned int flags) >> { >> + /* CPR preserves memory in place, so no need to track dirty memory */ >> + if (migrate_mode() != MIG_MODE_NORMAL) { >> + return; >> + } >> + >> if (!runstate_is_running()) { >> /* Postpone the dirty log stop, e.g., to when VM starts again */ >> if (vmstate_change) {
diff --git a/system/memory.c b/system/memory.c index b7548bf112..aef584e638 100644 --- a/system/memory.c +++ b/system/memory.c @@ -27,6 +27,7 @@ #include "exec/memory-internal.h" #include "exec/ram_addr.h" +#include "migration/misc.h" #include "sysemu/kvm.h" #include "sysemu/runstate.h" #include "sysemu/tcg.h" @@ -2947,6 +2948,11 @@ bool memory_global_dirty_log_start(unsigned int flags, Error **errp) assert(flags && !(flags & (~GLOBAL_DIRTY_MASK))); + /* CPR preserves memory in place, so no need to track dirty memory */ + if (migrate_mode() != MIG_MODE_NORMAL) { + return true; + } + if (vmstate_change) { /* If there is postponed stop(), operate on it first */ postponed_stop_flags &= ~flags; @@ -3021,6 +3027,11 @@ static void memory_vm_change_state_handler(void *opaque, bool running, void memory_global_dirty_log_stop(unsigned int flags) { + /* CPR preserves memory in place, so no need to track dirty memory */ + if (migrate_mode() != MIG_MODE_NORMAL) { + return; + } + if (!runstate_is_running()) { /* Postpone the dirty log stop, e.g., to when VM starts again */ if (vmstate_change) {
CPR preserves memory in place, so there is no need to track dirty memory. By skipping it, CPR can support devices that do not support tracking. Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- system/memory.c | 11 +++++++++++ 1 file changed, 11 insertions(+)