| Message ID | 1542276484-25508-5-git-send-email-wei.w.wang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-balloon: free page hint support | expand |
On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang 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.wang@intel.com> > CC: Dr. David Alan Gilbert <dgilbert@redhat.com> > CC: Juan Quintela <quintela@redhat.com> > CC: Michael S. Tsirkin <mst@redhat.com> > CC: Peter Xu <peterx@redhat.com> > --- > include/migration/misc.h | 2 ++ > migration/ram.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 50 insertions(+) > > diff --git a/include/migration/misc.h b/include/migration/misc.h > index 4ebf24c..113320e 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 ef69dbe..229b791 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -3131,6 +3131,54 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out) > } > > /* > + * This function clears bits of the free pages reported by the caller from the > + * migration dirty bitmap. @addr is the host address corresponding to the > + * start of the continuous guest free pages, and @len is the total bytes of > + * those pages. > + */ > +void qemu_guest_free_page_hint(void *addr, size_t len) > +{ > + RAMBlock *block; > + ram_addr_t offset; > + size_t used_len, start, npages; > + MigrationState *s = migrate_get_current(); > + > + /* This function is currently expected to be used during live migration */ > + if (!migration_is_setup_or_active(s->state)) { > + return; > + } > + > + for (; len > 0; len -= used_len) { > + block = qemu_ram_block_from_host(addr, false, &offset); > + assert(block); > + > + /* > + * This handles the case that the RAMBlock is resized after the free > + * page hint is reported. > + */ > + if (unlikely(offset > block->used_length)) { > + return; > + } Again, is it possible to resize during migration? So I think the check is fine, but uncertain about the comment. And shall we print something if that happened? We can use error_report_once(), and squashing the above assert: if (!block || offset > block->used_length) { /* should never happen, but if it happens we ignore the hints and warn */ error_report_once("..."); return; } What do you think? > + > + if (len <= block->used_length - offset) { > + used_len = len; > + } else { > + used_len = block->used_length - offset; > + addr += used_len; Maybe moving this line into the for() could be a bit better? for (; len > 0; len -= used_len, addr += used_len) { > + } > + > + start = offset >> TARGET_PAGE_BITS; > + npages = used_len >> TARGET_PAGE_BITS; > + > + qemu_mutex_lock(&ram_state->bitmap_mutex); > + ram_state->migration_dirty_pages -= > + bitmap_count_one_with_offset(block->bmap, start, npages); > + bitmap_clear(block->bmap, start, npages); > + qemu_mutex_unlock(&ram_state->bitmap_mutex); > + } > +} > + > +/* > * Each of ram_save_setup, ram_save_iterate and ram_save_complete has > * long-running RCU critical section. When rcu-reclaims in the code > * start to become numerous it will be necessary to reduce the > -- > 1.8.3.1 > Regards,
On 11/27/2018 02:06 PM, Peter Xu wrote: > On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote: > Again, is it possible to resize during migration? > > So I think the check is fine, but uncertain about the comment. Yes, resize would not happen with the current implementation. But heard it could just be a temporal implementation. Probably we could improve the comment like this: " Though the implementation might not support ram resize currently, this could happen in theory with future updates. So the check here handles the case that RAMBLOCK is resized after the free page hint is reported. " > > And shall we print something if that happened? We can use > error_report_once(), and squashing the above assert: > > if (!block || offset > block->used_length) { > /* should never happen, but if it happens we ignore the hints and warn */ > error_report_once("..."); > return; > } > > What do you think? Sounds good. > >> + >> + if (len <= block->used_length - offset) { >> + used_len = len; >> + } else { >> + used_len = block->used_length - offset; >> + addr += used_len; > Maybe moving this line into the for() could be a bit better? > > for (; len > 0; len -= used_len, addr += used_len) { > Yes, I think it looks better, thanks. Best, Wei
On Tue, Nov 27, 2018 at 02:52:35PM +0800, Wei Wang wrote: > On 11/27/2018 02:06 PM, Peter Xu wrote: > > On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote: > > Again, is it possible to resize during migration? > > > > So I think the check is fine, but uncertain about the comment. > > Yes, resize would not happen with the current implementation. > But heard it could just be a temporal implementation. Probably > we could improve the comment like this: > > " > Though the implementation might not support ram resize currently, > this could happen in theory with future updates. So the check here > handles the case that RAMBLOCK is resized after the free page hint is > reported. > " I'm not familiar with that part, but this seems ok to me. Thanks,
diff --git a/include/migration/misc.h b/include/migration/misc.h index 4ebf24c..113320e 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 ef69dbe..229b791 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -3131,6 +3131,54 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out) } /* + * This function clears bits of the free pages reported by the caller from the + * migration dirty bitmap. @addr is the host address corresponding to the + * start of the continuous guest free pages, and @len is the total bytes of + * those pages. + */ +void qemu_guest_free_page_hint(void *addr, size_t len) +{ + RAMBlock *block; + ram_addr_t offset; + size_t used_len, start, npages; + MigrationState *s = migrate_get_current(); + + /* This function is currently expected to be used during live migration */ + if (!migration_is_setup_or_active(s->state)) { + return; + } + + for (; len > 0; len -= used_len) { + block = qemu_ram_block_from_host(addr, false, &offset); + assert(block); + + /* + * This handles the case that the RAMBlock is resized after the free + * page hint is reported. + */ + if (unlikely(offset > block->used_length)) { + return; + } + + if (len <= block->used_length - offset) { + used_len = len; + } else { + used_len = block->used_length - offset; + addr += used_len; + } + + start = offset >> TARGET_PAGE_BITS; + npages = used_len >> TARGET_PAGE_BITS; + + qemu_mutex_lock(&ram_state->bitmap_mutex); + ram_state->migration_dirty_pages -= + bitmap_count_one_with_offset(block->bmap, start, npages); + bitmap_clear(block->bmap, start, npages); + qemu_mutex_unlock(&ram_state->bitmap_mutex); + } +} + +/* * Each of ram_save_setup, ram_save_iterate and ram_save_complete has * long-running RCU critical section. When rcu-reclaims in the code * start to become numerous it will be necessary to reduce the
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.wang@intel.com> CC: Dr. David Alan Gilbert <dgilbert@redhat.com> CC: Juan Quintela <quintela@redhat.com> CC: Michael S. Tsirkin <mst@redhat.com> CC: Peter Xu <peterx@redhat.com> --- include/migration/misc.h | 2 ++ migration/ram.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+)