From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEx5s-0005rd-Mu for qemu-devel@nongnu.org; Tue, 14 Jul 2015 06:05:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZEx5k-0007f4-RA for qemu-devel@nongnu.org; Tue, 14 Jul 2015 06:05:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44378) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEx5k-0007ey-E5 for qemu-devel@nongnu.org; Tue, 14 Jul 2015 06:05:36 -0400 From: Juan Quintela In-Reply-To: <1434450415-11339-34-git-send-email-dgilbert@redhat.com> (David Alan Gilbert's message of "Tue, 16 Jun 2015 11:26:46 +0100") References: <1434450415-11339-1-git-send-email-dgilbert@redhat.com> <1434450415-11339-34-git-send-email-dgilbert@redhat.com> Date: Tue, 14 Jul 2015 12:05:33 +0200 Message-ID: <87615ngiky.fsf@neno.neno> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v7 33/42] postcopy_ram.c: place_page and helpers Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, liang.z.li@intel.com, qemu-devel@nongnu.org, luis@cs.umu.se, amit.shah@redhat.com, pbonzini@redhat.com, david@gibson.dropbear.id.au "Dr. David Alan Gilbert (git)" wrote: > From: "Dr. David Alan Gilbert" > > 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 > --- 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.