* Wei Wang (wei.w.w...@intel.com) wrote:
> This patch adds an API to clear bits corresponding to guest free pages
> from the dirty bitmap. Spilt the free page block if it crosses the QEMU
> RAMBlock boundary.
>
> Signed-off-by: Wei Wang <wei.w.w...@intel.com>
> CC: Dr. David Alan Gilbert <dgilb...@redhat.com>
> CC: Juan Quintela <quint...@redhat.com>
> CC: Michael S. Tsirkin <m...@redhat.com>
> ---
> include/migration/misc.h | 2 ++
> migration/ram.c | 21 +++++++++++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 77fd4f5..fae1acf 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -14,11 +14,13 @@
> #ifndef MIGRATION_MISC_H
> #define MIGRATION_MISC_H
>
> +#include "exec/cpu-common.h"
> #include "qemu/notify.h"
>
> /* migration/ram.c */
>
> void ram_mig_init(void);
> +void qemu_guest_free_page_hint(void *addr, size_t len);
>
> /* migration/block.c */
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 5e33e5c..e172798 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2189,6 +2189,27 @@ static int ram_init_all(RAMState **rsp)
> return 0;
> }
>
This could do with some comments
> +void qemu_guest_free_page_hint(void *addr, size_t len)
> +{
> + RAMBlock *block;
> + ram_addr_t offset;
> + size_t used_len, start, npages;
From your use I think the addr and len are coming raw from the guest;
so we need to take some care.
> +
> + for (used_len = len; len > 0; len -= used_len) {
That initialisation of used_len is unusual; I'd rather put that
in the body.
> + block = qemu_ram_block_from_host(addr, false, &offset);
CHeck for block != 0
> + if (unlikely(offset + len > block->used_length)) {
I think to make that overflow safe, that should be:
if (len > (block->used_length - offset)) {
But we'll need another test before it, because qemu_ram_block_from_host
seems to check max_length not used_length, so we need to check
for offset > block->used_length first
> + used_len = block->used_length - offset;
> + addr += used_len;
> + }
> +
> + start = offset >> TARGET_PAGE_BITS;
> + npages = used_len >> TARGET_PAGE_BITS;
> + ram_state->migration_dirty_pages -=
> + bitmap_count_one_with_offset(block->bmap, start,
> npages);
> + bitmap_clear(block->bmap, start, npages);
If this is happening while the migration is running, this isn't safe -
the migration code could clear a bit at about the same point this
happens, so that the count returned by bitmap_count_one_with_offset
wouldn't match the word that was cleared by bitmap_clear.
The only way I can see to fix it is to run over the range using
bitmap_test_and_clear_atomic, using the return value to decrement
the number of dirty pages.
But you also need to be careful with the update of the
migration_dirty_pages value itself, because that's also being read
by the migration thread.
Dave
> + }
> +}
> +
> /*
> * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> * long-running RCU critical section. When rcu-reclaims in the code
> --
> 1.8.3.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
