Re: [Qemu-devel] [PATCH 3/7] arch_init: add ram_madvise_free()

[Anthony Liguori <aliguori@us.ibm.com>]

Lei Li <lilei@linux.vnet.ibm.com> writes:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  arch_init.c                   |   13 +++++++++++++
>  include/migration/migration.h |    3 +++
>  2 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 872020e..fc66bd2 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -524,6 +524,19 @@ uint64_t ram_bytes_total(void)
>      return total;
>  }
>  
> +void ram_madvise_free(ram_addr_t size)
> +{
> +    void *ram;
> +    RAMBlock *block = NULL;
> +
> +    ram = memory_region_get_ram_ptr(block->mr);
> +
> +    /* XXX. Here just simplely madvise(.., MADV_DONTNEED) the whole ram
> +     * pages, need more work to keep MADV_DONTNEED ram pages that
> +     * already sent. */
> +    qemu_madvise(ram, size, MADV_DONTNEED);
> +}
> +

I don't think this is right at all.  There's no guarantee we have a
single linear mapping of all ram.

I think you need something a bit more clever than this.

>  static void migration_end(void)
>  {
>      if (migration_bitmap) {
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 8866c3c..9cc5285 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -21,6 +21,7 @@
>  #include "qapi/error.h"
>  #include "migration/vmstate.h"
>  #include "qapi-types.h"
> +#include "exec/memory.h"
>  
>  enum {
>      MIG_STATE_ERROR,
> @@ -100,6 +101,8 @@ uint64_t ram_bytes_remaining(void);
>  uint64_t ram_bytes_transferred(void);
>  uint64_t ram_bytes_total(void);
>  
> +void ram_madvise_free(ram_addr_t size);
> +

If you introduce new interfaces, please include documentation in the
header.

Regards,

Anthony Liguori

>  extern SaveVMHandlers savevm_ram_handlers;
>  
>  uint64_t dup_mig_bytes_transferred(void);
> -- 
> 1.7.7.6