| Message ID | 1434450415-11339-34-git-send-email-dgilbert@redhat.com |
|---|---|
| State | New |
| Headers | show |
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > postcopy_place_page (etc) provide a way for postcopy to place a page > into guests memory atomically (using the copy ioctl on the ufd). > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- a/include/migration/postcopy-ram.h > +++ b/include/migration/postcopy-ram.h > @@ -69,4 +69,20 @@ void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds, > void postcopy_discard_send_finish(MigrationState *ms, > PostcopyDiscardState *pds); > > +/* > + * Place a page (from) at (host) efficiently > + * There are restrictions on how 'from' must be mapped, in general best > + * to use other postcopy_ routines to allocate. > + * returns 0 on success > + */ > +int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, > + bool all_zero); > + > +/* > + * Allocate a page of memory that can be mapped at a later point in time > + * using postcopy_place_page > + * Returns: Pointer to allocated page > + */ > +void *postcopy_get_tmp_page(MigrationIncomingState *mis); > + I don't think that this makes sense, but wouldn't have been a good idea to ask for the address that we want as a hint. That could help with fragmentation, no? > +/* > + * Place a host page (from) at (host) atomically > + * all_zero: Hint that the page being placed is 0 throughout > + * returns 0 on success > + */ > +int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, > + bool all_zero) postcop_place_page() and postcop_place_zero_page()? They just share a trace point :p > +{ > + if (!all_zero) { > + struct uffdio_copy copy_struct; > + > + copy_struct.dst = (uint64_t)(uintptr_t)host; > + copy_struct.src = (uint64_t)(uintptr_t)from; > + copy_struct.len = getpagesize(); > + copy_struct.mode = 0; > + > + /* copy also acks to the kernel waking the stalled thread up > + * TODO: We can inhibit that ack and only do it if it was requested > + * which would be slightly cheaper, but we'd have to be careful > + * of the order of updating our page state. > + */ > + if (ioctl(mis->userfault_fd, UFFDIO_COPY, ©_struct)) { > + int e = errno; > + error_report("%s: %s copy host: %p from: %p", > + __func__, strerror(e), host, from); > + > + return -e; > + } > + } else { > + struct uffdio_zeropage zero_struct; > + > + zero_struct.range.start = (uint64_t)(uintptr_t)host; > + zero_struct.range.len = getpagesize(); > + zero_struct.mode = 0; > + > + if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) { > + int e = errno; > + error_report("%s: %s zero host: %p from: %p", > + __func__, strerror(e), host, from); > + > + return -e; > + } > + } > + > + trace_postcopy_place_page(host, all_zero); > + return 0; > +} I really think that the userfault code should be in a linux specific file, but that can be done late, so I will not insist O:-) Later, Juan.
On (Tue) 14 Jul 2015 [12:05:33], Juan Quintela wrote: > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > > +/* > > + * Place a host page (from) at (host) atomically > > + * all_zero: Hint that the page being placed is 0 throughout > > + * returns 0 on success > > + */ > > +int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, > > + bool all_zero) > > postcop_place_page() and postcop_place_zero_page()? They just share a > trace point :p Yea, I thought the same. Amit
On (Tue) 16 Jun 2015 [11:26:46], Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > postcopy_place_page (etc) provide a way for postcopy to place a page > into guests memory atomically (using the copy ioctl on the ufd). > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Amit Shah <amit.shah@redhat.com> Amit
* Juan Quintela (quintela@redhat.com) wrote: > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > postcopy_place_page (etc) provide a way for postcopy to place a page > > into guests memory atomically (using the copy ioctl on the ufd). > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > --- a/include/migration/postcopy-ram.h > > +++ b/include/migration/postcopy-ram.h > > @@ -69,4 +69,20 @@ void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds, > > void postcopy_discard_send_finish(MigrationState *ms, > > PostcopyDiscardState *pds); > > > > +/* > > + * Place a page (from) at (host) efficiently > > + * There are restrictions on how 'from' must be mapped, in general best > > + * to use other postcopy_ routines to allocate. > > + * returns 0 on success > > + */ > > +int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, > > + bool all_zero); > > + > > +/* > > + * Allocate a page of memory that can be mapped at a later point in time > > + * using postcopy_place_page > > + * Returns: Pointer to allocated page > > + */ > > +void *postcopy_get_tmp_page(MigrationIncomingState *mis); > > + > > I don't think that this makes sense, but wouldn't have been a good idea > to ask for the address that we want as a hint. That could help with > fragmentation, no? I think that we may be able to do something if we were to transmit huge pages (which is a separate problem); but at the moment all get_tmp_page does it an mmap with the right set of flags, and that mmap only happens once for all the pages; it's only the backing page that gets moved, that mmap is reused for the whole run. > > +/* > > + * Place a host page (from) at (host) atomically > > + * all_zero: Hint that the page being placed is 0 throughout > > + * returns 0 on success > > + */ > > +int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, > > + bool all_zero) > > postcop_place_page() and postcop_place_zero_page()? They just share a > trace point :p Done. > > +{ > > + if (!all_zero) { > > + struct uffdio_copy copy_struct; > > + > > + copy_struct.dst = (uint64_t)(uintptr_t)host; > > + copy_struct.src = (uint64_t)(uintptr_t)from; > > + copy_struct.len = getpagesize(); > > + copy_struct.mode = 0; > > + > > + /* copy also acks to the kernel waking the stalled thread up > > + * TODO: We can inhibit that ack and only do it if it was requested > > + * which would be slightly cheaper, but we'd have to be careful > > + * of the order of updating our page state. > > + */ > > + if (ioctl(mis->userfault_fd, UFFDIO_COPY, ©_struct)) { > > + int e = errno; > > + error_report("%s: %s copy host: %p from: %p", > > + __func__, strerror(e), host, from); > > + > > + return -e; > > + } > > + } else { > > + struct uffdio_zeropage zero_struct; > > + > > + zero_struct.range.start = (uint64_t)(uintptr_t)host; > > + zero_struct.range.len = getpagesize(); > > + zero_struct.mode = 0; > > + > > + if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) { > > + int e = errno; > > + error_report("%s: %s zero host: %p from: %p", > > + __func__, strerror(e), host, from); > > + > > + return -e; > > + } > > + } > > + > > + trace_postcopy_place_page(host, all_zero); > > + return 0; > > +} > > I really think that the userfault code should be in a linux specific > file, but that can be done late, so I will not insist O:-) I think it will make sense once we have another OSs view of what the interface should look like, and then we can get an abstraction that works for both and move the implementations out. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/include/migration/migration.h b/include/migration/migration.h index 20d1b39..8d2e5c8 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -79,6 +79,7 @@ struct MigrationIncomingState { QEMUFile *return_path; QemuMutex rp_mutex; /* We send replies from multiple threads */ PostcopyState postcopy_state; + void *postcopy_tmp_page; /* See savevm.c */ LoadStateEntry_Head loadvm_handlers; diff --git a/include/migration/postcopy-ram.h b/include/migration/postcopy-ram.h index 8a8616b..bac79c5 100644 --- a/include/migration/postcopy-ram.h +++ b/include/migration/postcopy-ram.h @@ -69,4 +69,20 @@ void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds, void postcopy_discard_send_finish(MigrationState *ms, PostcopyDiscardState *pds); +/* + * Place a page (from) at (host) efficiently + * There are restrictions on how 'from' must be mapped, in general best + * to use other postcopy_ routines to allocate. + * returns 0 on success + */ +int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, + bool all_zero); + +/* + * Allocate a page of memory that can be mapped at a later point in time + * using postcopy_place_page + * Returns: Pointer to allocated page + */ +void *postcopy_get_tmp_page(MigrationIncomingState *mis); + #endif diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index 7158d08..6345480 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -279,6 +279,10 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis) return -1; } + if (mis->postcopy_tmp_page) { + munmap(mis->postcopy_tmp_page, getpagesize()); + mis->postcopy_tmp_page = NULL; + } return 0; } @@ -345,6 +349,77 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis) return 0; } +/* + * Place a host page (from) at (host) atomically + * all_zero: Hint that the page being placed is 0 throughout + * returns 0 on success + */ +int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, + bool all_zero) +{ + if (!all_zero) { + struct uffdio_copy copy_struct; + + copy_struct.dst = (uint64_t)(uintptr_t)host; + copy_struct.src = (uint64_t)(uintptr_t)from; + copy_struct.len = getpagesize(); + copy_struct.mode = 0; + + /* copy also acks to the kernel waking the stalled thread up + * TODO: We can inhibit that ack and only do it if it was requested + * which would be slightly cheaper, but we'd have to be careful + * of the order of updating our page state. + */ + if (ioctl(mis->userfault_fd, UFFDIO_COPY, ©_struct)) { + int e = errno; + error_report("%s: %s copy host: %p from: %p", + __func__, strerror(e), host, from); + + return -e; + } + } else { + struct uffdio_zeropage zero_struct; + + zero_struct.range.start = (uint64_t)(uintptr_t)host; + zero_struct.range.len = getpagesize(); + zero_struct.mode = 0; + + if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) { + int e = errno; + error_report("%s: %s zero host: %p from: %p", + __func__, strerror(e), host, from); + + return -e; + } + } + + trace_postcopy_place_page(host, all_zero); + return 0; +} + +/* + * Returns a target page of memory that can be mapped at a later point in time + * using postcopy_place_page + * The same address is used repeatedly, postcopy_place_page just takes the + * backing page away. + * Returns: Pointer to allocated page + * + */ +void *postcopy_get_tmp_page(MigrationIncomingState *mis) +{ + if (!mis->postcopy_tmp_page) { + mis->postcopy_tmp_page = mmap(NULL, getpagesize(), + PROT_READ | PROT_WRITE, MAP_PRIVATE | + MAP_ANONYMOUS, -1, 0); + if (!mis->postcopy_tmp_page) { + error_report("%s: %s", __func__, strerror(errno)); + return NULL; + } + } + + return mis->postcopy_tmp_page; +} + #else /* No target OS support, stubs just fail */ bool postcopy_ram_supported_by_host(void) @@ -374,6 +449,18 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis) { assert(0); } + +int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, + bool all_zero) +{ + assert(0); +} + +void *postcopy_get_tmp_page(MigrationIncomingState *mis) +{ + assert(0); +} + #endif /* ------------------------------------------------------------------------- */ diff --git a/trace-events b/trace-events index cb707c7..d9c5a51 100644 --- a/trace-events +++ b/trace-events @@ -1515,6 +1515,7 @@ postcopy_discard_send_finish(const char *ramblock, int nwords, int ncmds) "%s ma 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" +postcopy_place_page(void *host_addr, bool all_zero) "host=%p all_zero=%d" # kvm-all.c kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"