[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 6/7] memory: make global_dirty_log a bitmask
From: |
Peter Xu |
Subject: |
Re: [PATCH v3 6/7] memory: make global_dirty_log a bitmask |
Date: |
Mon, 7 Jun 2021 13:47:30 -0400 |
On Mon, Jun 07, 2021 at 09:13:12AM +0800, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>
> dirty rate measurement may start or stop dirty logging during
> calculation. this conflict with migration because stop dirty
> log make migration leave dirty pages out then that'll be a problem.
>
> make global_dirty_log a bitmask can let both migration and dirty
> rate measurement work fine. introduce GLOBAL_DIRTY_MIGRATION and
> GLOBAL_DIRTY_DIRTY_RATE to distinguish what current dirty log aims
> for, migration or dirty rate.
>
> all references to global_dirty_log should be untouched because any bit
> set there should justify that global dirty logging is enabled.
>
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> ---
> include/exec/memory.h | 13 ++++++++++---
> migration/ram.c | 8 ++++----
> softmmu/memory.c | 36 +++++++++++++++++++++++++++---------
> 3 files changed, 41 insertions(+), 16 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index c158fd7084..94c7088299 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -55,7 +55,10 @@ static inline void fuzz_dma_read_cb(size_t addr,
> }
> #endif
>
> -extern bool global_dirty_log;
> +#define GLOBAL_DIRTY_MIGRATION (1U<<0)
> +#define GLOBAL_DIRTY_DIRTY_RATE (1U<<1)
Add some comment for each dirty log reason would be nice.
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index c19b0be6b1..b93baba82d 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -39,7 +39,7 @@
> static unsigned memory_region_transaction_depth;
> static bool memory_region_update_pending;
> static bool ioeventfd_update_pending;
> -bool global_dirty_log;
> +int global_dirty_log;
Maybe unsigned int would make more sense for a bitmask?
>
> static QTAILQ_HEAD(, MemoryListener) memory_listeners
> = QTAILQ_HEAD_INITIALIZER(memory_listeners);
> @@ -2659,14 +2659,20 @@ void memory_global_after_dirty_log_sync(void)
>
> static VMChangeStateEntry *vmstate_change;
>
> -void memory_global_dirty_log_start(void)
> +void memory_global_dirty_log_start(int flags)
> {
> if (vmstate_change) {
> qemu_del_vm_change_state_handler(vmstate_change);
> vmstate_change = NULL;
> }
>
> - global_dirty_log = true;
> + if (flags & GLOBAL_DIRTY_MIGRATION) {
> + global_dirty_log |= GLOBAL_DIRTY_MIGRATION;
> + }
> +
> + if (flags & GLOBAL_DIRTY_DIRTY_RATE) {
> + global_dirty_log |= GLOBAL_DIRTY_DIRTY_RATE;
> + }
Instead of separate "if"s, perhaps something like this?
#define GLOBAL_DIRTY_MASK (0x3)
assert(!(flags & (~GLOBAL_DIRTY_MASK)));
assert(!(global_dirty_log ^ flags));
global_dirty_log |= flags;
>
> MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
>
> @@ -2676,9 +2682,15 @@ void memory_global_dirty_log_start(void)
> memory_region_transaction_commit();
> }
>
> -static void memory_global_dirty_log_do_stop(void)
> +static void memory_global_dirty_log_do_stop(int flags)
> {
> - global_dirty_log = false;
> + if (flags & GLOBAL_DIRTY_MIGRATION) {
> + global_dirty_log &= ~GLOBAL_DIRTY_MIGRATION;
> + }
> +
> + if (flags & GLOBAL_DIRTY_DIRTY_RATE) {
> + global_dirty_log &= ~GLOBAL_DIRTY_DIRTY_RATE;
> + }
Same here? Thanks,
--
Peter Xu
[PATCH v3 1/7] migration/dirtyrate: make sample page count configurable, huangy81, 2021/06/06
[PATCH v3 2/7] hmp: Add "calc_dirty_rate" and "info dirty_rate" cmds, huangy81, 2021/06/06