Re: [Qemu-devel] [PATCH v7 25/42] Postcopy: Maintain sentmap and calculate discard

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
>
> Where postcopy is preceeded by a period of precopy, the destination will
> have received pages that may have been dirtied on the source after the
> page was sent.  The destination must throw these pages away before
> starting it's CPUs.
>
> Maintain a 'sentmap' of pages that have already been sent.
> Calculate list of sent & dirty pages
> Provide helpers on the destination side to discard these.
>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>

Not a patch without a suggestion O:-)

> ---
>  include/migration/migration.h    |  12 +++
>  include/migration/postcopy-ram.h |  35 +++++++
>  include/qemu/typedefs.h          |   1 +
>  migration/migration.c            |   1 +
>  migration/postcopy-ram.c         | 108 +++++++++++++++++++++
>  migration/ram.c                  | 203 
> ++++++++++++++++++++++++++++++++++++++-
>  migration/savevm.c               |   2 -
>  trace-events                     |   5 +
>  8 files changed, 363 insertions(+), 4 deletions(-)
>
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 2a22381..4c6cf95 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -114,6 +114,13 @@ struct MigrationState
>  
>      /* Flag set once the migration has been asked to enter postcopy */
>      bool start_postcopy;
> +
> +    /* bitmap of pages that have been sent at least once
> +     * only maintained and used in postcopy at the moment
> +     * where it's used to send the dirtymap at the start
> +     * of the postcopy phase
> +     */
> +    unsigned long *sentmap;
>  };

We can use this sentmap for zero page optimization.  If page is on
sentmap, we need to sent a zero page, otherwise, just sent sentmap at
the end of migration and clean everything not there?

> +/*
> + * Discard the contents of memory start..end inclusive.
> + * We can assume that if we've been called postcopy_ram_hosttest returned 
> true
> + */
> +int postcopy_ram_discard_range(MigrationIncomingState *mis, uint8_t *start,
> +                               uint8_t *end)
> +{
> +    trace_postcopy_ram_discard_range(start, end);
> +    if (madvise(start, (end-start)+1, MADV_DONTNEED)) {

Can we s/end/lenght/ and adjust everywhere?


Not here, but putting a comment explaining where magic 12 cames from on
definition of constant?

I think that the sentbitmap bits could we used without the rest.