Re: [Qemu-devel] [PATCH 12/23] memory: separate building the final memory map into two steps

[Anthony Liguori]

On 07/25/2011 09:02 AM, Avi Kivity wrote:
> Instead of adding and deleting regions in one pass, do a delete
> pass followed by an add pass.  This fixes the following case:
>
> from:
>    0x0000-0x0fff ram  (a1)
>    0x1000-0x1fff mmio (a2)
>    0x2000-0x2fff ram  (a3)
>
> to:
>    0x0000-0x2fff ram  (b1)
>
> The single pass algorithm removed a1, added b2, then removed a2 and a2,

You mean a2 and a3 I suppose?

Regards,

Anthony Liguori

> which caused the wrong memory map to be built.  The two pass algorithm
> removes a1, a2, and a3, then adds b1.
>
> Signed-off-by: Avi Kivity<address@hidden>
> ---
>   memory.c |   38 +++++++++++++++++++++++++++++---------
>   1 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index a5cde0c..009ad33 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -546,10 +546,11 @@ static void address_space_update_ioeventfds(AddressSpace 
> *as)
>       as->ioeventfd_nb = ioeventfd_nb;
>   }
>
> -static void address_space_update_topology(AddressSpace *as)
> +static void address_space_update_topology_pass(AddressSpace *as,
> +                                               FlatView old_view,
> +                                               FlatView new_view,
> +                                               bool adding)
>   {
> -    FlatView old_view = as->current_map;
> -    FlatView new_view = generate_memory_topology(as->root);
>       unsigned iold, inew;
>       FlatRange *frold, *frnew;
>
> @@ -576,15 +577,20 @@ static void address_space_update_topology(AddressSpace 
> *as)
>                       &&  !flatrange_equal(frold, frnew)))) {
>               /* In old, but (not in new, or in new but attributes changed). */
>
> -            as->ops->range_del(as, frold);
> +            if (!adding) {
> +                as->ops->range_del(as, frold);
> +            }
> +
>               ++iold;
>           } else if (frold&&  frnew&&  flatrange_equal(frold, frnew)) {
>               /* In both (logging may have changed) */
>
> -            if (frold->dirty_log_mask&&  !frnew->dirty_log_mask) {
> -                as->ops->log_stop(as, frnew);
> -            } else if (frnew->dirty_log_mask&&  !frold->dirty_log_mask) {
> -                as->ops->log_start(as, frnew);
> +            if (adding) {
> +                if (frold->dirty_log_mask&&  !frnew->dirty_log_mask) {
> +                    as->ops->log_stop(as, frnew);
> +                } else if (frnew->dirty_log_mask&&  !frold->dirty_log_mask) {
> +                    as->ops->log_start(as, frnew);
> +                }
>               }
>
>               ++iold;
> @@ -592,10 +598,24 @@ static void address_space_update_topology(AddressSpace 
> *as)
>           } else {
>               /* In new */
>
> -            as->ops->range_add(as, frnew);
> +            if (adding) {
> +                as->ops->range_add(as, frnew);
> +            }
> +
>               ++inew;
>           }
>       }
> +}
> +
> +
> +static void address_space_update_topology(AddressSpace *as)
> +{
> +    FlatView old_view = as->current_map;
> +    FlatView new_view = generate_memory_topology(as->root);
> +
> +    address_space_update_topology_pass(as, old_view, new_view, false);
> +    address_space_update_topology_pass(as, old_view, new_view, true);
> +
>       as->current_map = new_view;
>       flatview_destroy(&old_view);
>       address_space_update_ioeventfds(as);