* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
> > include/migration/migration.h | 3 +
> > include/migration/postcopy-ram.h | 12 ++++
> > migration/postcopy-ram.c | 116
> > +++++++++++++++++++++++++++++++++++++++
> > migration/ram.c | 11 ++++
> > migration/savevm.c | 4 ++
> > trace-events | 2 +
> > 6 files changed, 148 insertions(+)
> >
>
> qemu_hugepage_enable(host_addr, length)?
>
> > +#ifdef MADV_NOHUGEPAGE
> > + if (madvise(host_addr, length, MADV_NOHUGEPAGE)) {
> > + error_report("%s: NOHUGEPAGE: %s", __func__, strerror(errno));
> > + return -1;
> > + }
> > +#endif
>
> qemu_hugepage_disable(host_addr, length)?
I've flipped those both to use the qemu_madvise which is how
it's done in most other places in the codebase (I added
the QEMU_MADV_NOHUGEPAGE since HUGEPAGE was there but not the opposite).
> > +#ifdef MADV_HUGEPAGE
> > + if (madvise(host_addr, length, MADV_HUGEPAGE)) {
> > + error_report("%s HUGEPAGE: %s", __func__, strerror(errno));
> > + return -1;
> > + }
> > +#endif
> > +
> > + /*
> > + * We can also turn off userfault now since we should have all the
> > + * pages. It can be useful to leave it on to debug postcopy
> > + * if you're not sure it's always getting every page.
> > + */
>
> qemu_userfault_unregister(host_addr, length)?
Is it worth wrapping that ioctl, when it's already in a function
that has to do other calls around it?
> > + range_struct.start = (uintptr_t)host_addr;
> > + range_struct.len = length;
> > +
> > + if (ioctl(mis->userfault_fd, UFFDIO_UNREGISTER, &range_struct)) {
> > + error_report("%s: userfault unregister %s", __func__,
> > strerror(errno));
> > +
> > + return -1;
> > + }
>
> >
> > +/*
> > + * Allocate data structures etc needed by incoming migration with
> > postcopy-ram
> > + * postcopy-ram's similarly names postcopy_ram_incoming_init does the work
> > + */
> > +int ram_postcopy_incoming_init(MigrationIncomingState *mis)
> > +{
> > + size_t ram_pages = last_ram_offset() >> TARGET_PAGE_BITS;
> > +
> > + return postcopy_ram_incoming_init(mis, ram_pages);
> > +}
> > +
>
> ram_postocpy_incoming_init()
> and
> postcopy_ram_incoming_init()
>
> ouch Thinking about better names ....
Agreed; suggestions welcome. the 'ram' and 'postcopy_ram' are
both the convention based on the filename.
> > static int ram_load(QEMUFile *f, void *opaque, int version_id)
> > {
> > int flags = 0, ret = 0;
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index e6398dd..f4de52d 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -1238,6 +1238,10 @@ static int
> > loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
> > return -1;
> > }
> >
> > + if (ram_postcopy_incoming_init(mis)) {
> > + return -1;
> > + }
> > +
>
> how/where we know that this is called soon enough?
It's driven by the sequence of commands byte that walk it through
the state machine.
> > postcopy_state_set(mis, POSTCOPY_INCOMING_ADVISE);
> >
> > return 0;
> > diff --git a/trace-events b/trace-events
> > index 5e8a120..2ffc1c6 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -1498,7 +1498,9 @@
> > rdma_start_outgoing_migration_after_rdma_source_init(void) ""
> >
> > # migration/postcopy-ram.c
> > postcopy_discard_send_finish(const char *ramblock, int nwords, int ncmds)
> > "%s mask words sent=%d in %d commands"
> > +postcopy_cleanup_area(const char *ramblock, void *host_addr, size_t
> > offset, size_t length) "%s: %p offset=%zx length=%zx"
> > postcopy_ram_discard_range(void *start, void *end) "%p,%p"
> > +postcopy_init_area(const char *ramblock, void *host_addr, size_t offset,
> > size_t length) "%s: %p offset=%zx length=%zx"
>
> once here, if we have range names before, what about:
>
> postcopy_ram_cleanup_range()
> postcopy_ram_init_range()
Done.
> And let the ram* functions the same?
Not sure which ones those refer to?
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
