Re: [Qemu-devel] [PATCH v2] memory: Reintroduce dirty flag to optimize changes on disabled regions

[Blue Swirl]

Thanks, applied. Win2k initial display update is much faster now.

On Mon, Nov 5, 2012 at 3:45 PM, Jan Kiszka <address@hidden> wrote:
> Cirrus is triggering this, e.g. during Win2k boot: Changes only on
> disabled regions require no topology update when transaction depth drops
> to 0 again.
>
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
>
> Changes in v2:
>  - drop useless memory_region_update_pending = true from
>    address_space_init/destroy
>
>  memory.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index d5150f8..7419853 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -22,7 +22,8 @@
>
>  #include "memory-internal.h"
>
> -unsigned memory_region_transaction_depth = 0;
> +static unsigned memory_region_transaction_depth;
> +static bool memory_region_update_pending;
>  static bool global_dirty_log = false;
>
>  static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners
> @@ -741,7 +742,8 @@ void memory_region_transaction_commit(void)
>
>      assert(memory_region_transaction_depth);
>      --memory_region_transaction_depth;
> -    if (!memory_region_transaction_depth) {
> +    if (!memory_region_transaction_depth && memory_region_update_pending) {
> +        memory_region_update_pending = false;
>          MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
>
>          QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> @@ -1060,6 +1062,7 @@ void memory_region_set_log(MemoryRegion *mr, bool log, 
> unsigned client)
>
>      memory_region_transaction_begin();
>      mr->dirty_log_mask = (mr->dirty_log_mask & ~mask) | (log * mask);
> +    memory_region_update_pending |= mr->enabled;
>      memory_region_transaction_commit();
>  }
>
> @@ -1097,6 +1100,7 @@ void memory_region_set_readonly(MemoryRegion *mr, bool 
> readonly)
>      if (mr->readonly != readonly) {
>          memory_region_transaction_begin();
>          mr->readonly = readonly;
> +        memory_region_update_pending |= mr->enabled;
>          memory_region_transaction_commit();
>      }
>  }
> @@ -1106,6 +1110,7 @@ void memory_region_rom_device_set_readable(MemoryRegion 
> *mr, bool readable)
>      if (mr->readable != readable) {
>          memory_region_transaction_begin();
>          mr->readable = readable;
> +        memory_region_update_pending |= mr->enabled;
>          memory_region_transaction_commit();
>      }
>  }
> @@ -1248,6 +1253,7 @@ void memory_region_add_eventfd(MemoryRegion *mr,
>      memmove(&mr->ioeventfds[i+1], &mr->ioeventfds[i],
>              sizeof(*mr->ioeventfds) * (mr->ioeventfd_nb-1 - i));
>      mr->ioeventfds[i] = mrfd;
> +    memory_region_update_pending |= mr->enabled;
>      memory_region_transaction_commit();
>  }
>
> @@ -1280,6 +1286,7 @@ void memory_region_del_eventfd(MemoryRegion *mr,
>      --mr->ioeventfd_nb;
>      mr->ioeventfds = g_realloc(mr->ioeventfds,
>                                    sizeof(*mr->ioeventfds)*mr->ioeventfd_nb + 
> 1);
> +    memory_region_update_pending |= mr->enabled;
>      memory_region_transaction_commit();
>  }
>
> @@ -1323,6 +1330,7 @@ static void 
> memory_region_add_subregion_common(MemoryRegion *mr,
>      }
>      QTAILQ_INSERT_TAIL(&mr->subregions, subregion, subregions_link);
>  done:
> +    memory_region_update_pending |= mr->enabled && subregion->enabled;
>      memory_region_transaction_commit();
>  }
>
> @@ -1353,6 +1361,7 @@ void memory_region_del_subregion(MemoryRegion *mr,
>      assert(subregion->parent == mr);
>      subregion->parent = NULL;
>      QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
> +    memory_region_update_pending |= mr->enabled && subregion->enabled;
>      memory_region_transaction_commit();
>  }
>
> @@ -1363,6 +1372,7 @@ void memory_region_set_enabled(MemoryRegion *mr, bool 
> enabled)
>      }
>      memory_region_transaction_begin();
>      mr->enabled = enabled;
> +    memory_region_update_pending = true;
>      memory_region_transaction_commit();
>  }
>
> @@ -1397,6 +1407,7 @@ void memory_region_set_alias_offset(MemoryRegion *mr, 
> hwaddr offset)
>
>      memory_region_transaction_begin();
>      mr->alias_offset = offset;
> +    memory_region_update_pending |= mr->enabled;
>      memory_region_transaction_commit();
>  }
>
> --
> 1.7.3.4