From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40031) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZF22G-0008OX-S0 for qemu-devel@nongnu.org; Tue, 14 Jul 2015 11:22:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZF22C-0000v9-Dr for qemu-devel@nongnu.org; Tue, 14 Jul 2015 11:22:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50103) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZF22B-0000uR-Tj for qemu-devel@nongnu.org; Tue, 14 Jul 2015 11:22:16 -0400 From: Juan Quintela In-Reply-To: <1434450415-11339-42-git-send-email-dgilbert@redhat.com> (David Alan Gilbert's message of "Tue, 16 Jun 2015 11:26:54 +0100") References: <1434450415-11339-1-git-send-email-dgilbert@redhat.com> <1434450415-11339-42-git-send-email-dgilbert@redhat.com> Date: Tue, 14 Jul 2015 17:22:13 +0200 Message-ID: <87zj2yepcq.fsf@neno.neno> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v7 41/42] Disable mlock around incoming postcopy 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" > > Userfault doesn't work with mlock; mlock is designed to nail down pages > so they don't move, userfault is designed to tell you when they're not > there. > > munlock the pages we userfault protect before postcopy. > mlock everything again at the end if mlock is enabled. > > Signed-off-by: Dr. David Alan Gilbert > Reviewed-by: David Gibson > --- > include/sysemu/sysemu.h | 1 + > migration/postcopy-ram.c | 24 ++++++++++++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 1af2ea0..c1f3da4 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -171,6 +171,7 @@ extern int boot_menu; > extern bool boot_strict; > extern uint8_t *boot_splash_filedata; > extern size_t boot_splash_filedata_size; > +extern bool enable_mlock; > extern uint8_t qemu_extra_params_fw[2]; > extern QEMUClockType rtc_clock; > extern const char *mem_path; > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index 7eb1fb9..be7e5f2 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -85,6 +85,11 @@ static bool ufd_version_check(int ufd) > return true; > } > > +/* > + * Note: This has the side effect of munlock'ing all of RAM, that's > + * normally fine since if the postcopy succeeds it gets turned back on at the > + * end. > + */ > bool postcopy_ram_supported_by_host(void) > { > long pagesize = getpagesize(); > @@ -113,6 +118,15 @@ bool postcopy_ram_supported_by_host(void) > } > > /* > + * userfault and mlock don't go together; we'll put it back later if > + * it was enabled. > + */ > + if (munlockall()) { > + error_report("%s: munlockall: %s", __func__, strerror(errno)); why is this not proteced by enable_mlock? > + return -1; > + } > + > + /* > * We need to check that the ops we need are supported on anon memory > * To do that we need to register a chunk and see the flags that > * are returned. > @@ -303,6 +317,16 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis) > mis->have_fault_thread = false; > } > > + if (enable_mlock) { > + if (os_mlock() < 0) { > + error_report("mlock: %s", strerror(errno)); > + /* > + * It doesn't feel right to fail at this point, we have a valid > + * VM state. > + */ realtime_init() exit in case of os_mlock() fails, so current code is: - we start qemu with mlock requset - we mlock memory - we start postcopy - we munlock memory - we mlock memory I wmill really, really preffer having a check if memory is mlocked, and it that case, just abort migration altogether. Or better still, wait to enable mlock *until* we have finished postcopy, no? Later, Juan. > + } > + } > + > postcopy_state_set(mis, POSTCOPY_INCOMING_END); > migrate_send_rp_shut(mis, qemu_file_get_error(mis->file) != 0);