[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 27/29] memory: replace cpu_physical_memory_reset
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 27/29] memory: replace cpu_physical_memory_reset_dirty() with test-and-clear |
Date: |
Tue, 26 May 2015 20:42:40 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, 04/27 18:28, Paolo Bonzini wrote:
> From: Stefan Hajnoczi <address@hidden>
>
> The cpu_physical_memory_reset_dirty() function is sometimes used
> together with cpu_physical_memory_get_dirty(). This is not atomic since
> two separate accesses to the dirty memory bitmap are made.
>
> Turn cpu_physical_memory_reset_dirty() and
> cpu_physical_memory_clear_dirty_range_type() into the atomic
> cpu_physical_memory_test_and_clear_dirty().
>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> Message-Id: <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> cputlb.c | 4 ++--
> exec.c | 23 +++++++++++++++++------
> include/exec/ram_addr.h | 33 ++++++++++-----------------------
> memory.c | 11 ++++-------
> 4 files changed, 33 insertions(+), 38 deletions(-)
>
> diff --git a/cputlb.c b/cputlb.c
> index a86e76f..75b2dcf 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -125,8 +125,8 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
> can be detected */
> void tlb_protect_code(ram_addr_t ram_addr)
> {
> - cpu_physical_memory_reset_dirty(ram_addr, TARGET_PAGE_SIZE,
> - DIRTY_MEMORY_CODE);
> + cpu_physical_memory_test_and_clear_dirty(ram_addr, TARGET_PAGE_SIZE,
> + DIRTY_MEMORY_CODE);
> }
>
> /* update the TLB so that writes in physical page 'phys_addr' are no longer
> diff --git a/exec.c b/exec.c
> index b7aebbf..daed5a7 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -857,16 +857,27 @@ static void tlb_reset_dirty_range_all(ram_addr_t start,
> ram_addr_t length)
> }
>
> /* Note: start and end must be within the same ram block. */
> -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
> - unsigned client)
> +bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t start,
> + ram_addr_t length,
> + unsigned client)
> {
> - if (length == 0)
> - return;
> - cpu_physical_memory_clear_dirty_range_type(start, length, client);
> + unsigned long end, page;
> + bool dirty;
> +
> + if (length == 0) {
> + return false;
> + }
>
> - if (tcg_enabled()) {
> + end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
> + page = start >> TARGET_PAGE_BITS;
> + dirty = bitmap_test_and_clear_atomic(ram_list.dirty_memory[client],
> + page, end - page);
> +
> + if (dirty && tcg_enabled()) {
> tlb_reset_dirty_range_all(start, length);
> }
> +
> + return dirty;
> }
>
> /* Called from RCU critical section */
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 9bd7827..ea77187 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -194,30 +194,19 @@ static inline void
> cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
> }
> #endif /* not _WIN32 */
>
> -static inline void cpu_physical_memory_clear_dirty_range_type(ram_addr_t
> start,
> - ram_addr_t
> length,
> - unsigned
> client)
> -{
> - unsigned long end, page;
> -
> - assert(client < DIRTY_MEMORY_NUM);
> - end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
> - page = start >> TARGET_PAGE_BITS;
> - bitmap_clear(ram_list.dirty_memory[client], page, end - page);
> -}
> +bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t start,
> + ram_addr_t length,
> + unsigned client);
>
> static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
> ram_addr_t length)
> {
> - cpu_physical_memory_clear_dirty_range_type(start, length,
> DIRTY_MEMORY_MIGRATION);
> - cpu_physical_memory_clear_dirty_range_type(start, length,
> DIRTY_MEMORY_VGA);
> - cpu_physical_memory_clear_dirty_range_type(start, length,
> DIRTY_MEMORY_CODE);
> + cpu_physical_memory_test_and_clear_dirty(start, length,
> DIRTY_MEMORY_MIGRATION);
Is this line too long? Otherwise,
Reviewed-by: Fam Zheng <address@hidden>
> + cpu_physical_memory_test_and_clear_dirty(start, length,
> DIRTY_MEMORY_VGA);
> + cpu_physical_memory_test_and_clear_dirty(start, length,
> DIRTY_MEMORY_CODE);
> }
>
>
> -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
> - unsigned client);
> -
> static inline
> uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest,
> ram_addr_t start,
> @@ -245,12 +234,10 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned
> long *dest,
> }
> } else {
> for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
> - if (cpu_physical_memory_get_dirty(start + addr,
> - TARGET_PAGE_SIZE,
> - DIRTY_MEMORY_MIGRATION)) {
> - cpu_physical_memory_reset_dirty(start + addr,
> - TARGET_PAGE_SIZE,
> - DIRTY_MEMORY_MIGRATION);
> + if (cpu_physical_memory_test_and_clear_dirty(
> + start + addr,
> + TARGET_PAGE_SIZE,
> + DIRTY_MEMORY_MIGRATION)) {
> long k = (start + addr) >> TARGET_PAGE_BITS;
> if (!test_and_set_bit(k, dest)) {
> num_dirty++;
> diff --git a/memory.c b/memory.c
> index 7422790..bb86b4b 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1397,13 +1397,9 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr
> addr,
> bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
> hwaddr size, unsigned client)
> {
> - bool ret;
> assert(mr->terminates);
> - ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client);
> - if (ret) {
> - cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client);
> - }
> - return ret;
> + return cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr,
> + size, client);
> }
>
>
> @@ -1447,7 +1443,8 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr
> addr,
> hwaddr size, unsigned client)
> {
> assert(mr->terminates);
> - cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client);
> + cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size,
> + client);
> }
>
> int memory_region_get_fd(MemoryRegion *mr)
> --
> 1.8.3.1
>
>
>
- Re: [Qemu-devel] [PATCH 27/29] memory: replace cpu_physical_memory_reset_dirty() with test-and-clear,
Fam Zheng <=