From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39905) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gTjTx-0001mM-P0 for qemu-devel@nongnu.org; Mon, 03 Dec 2018 03:21:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gTjNr-0006BZ-IQ for qemu-devel@nongnu.org; Mon, 03 Dec 2018 03:15:18 -0500 Received: from mga14.intel.com ([192.55.52.115]:16513) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gTjNr-0006BO-AY for qemu-devel@nongnu.org; Mon, 03 Dec 2018 03:15:15 -0500 Message-ID: <5C04E73F.9040202@intel.com> Date: Mon, 03 Dec 2018 16:20:15 +0800 From: Wei Wang MIME-Version: 1.0 References: <1543803511-34793-1-git-send-email-wei.w.wang@intel.com> <1543803511-34793-7-git-send-email-wei.w.wang@intel.com> <20181203053118.GD27620@xz-x1> In-Reply-To: <20181203053118.GD27620@xz-x1> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v10 6/7] migration/ram.c: add a function to disable the bulk stage List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: qemu-devel@nongnu.org, virtio-dev@lists.oasis-open.org, mst@redhat.com, quintela@redhat.com, dgilbert@redhat.com, pbonzini@redhat.com, liliang.opensource@gmail.com, nilal@redhat.com, riel@redhat.com On 12/03/2018 01:31 PM, Peter Xu wrote: > On Mon, Dec 03, 2018 at 10:18:30AM +0800, Wei Wang wrote: >> This patch adds a function to enable a precopy notifier callback outside >> the migration subsystem to disable the bulk stage flag. This is needed by >> the free page optimization offered by virtio-balloon. >> >> Signed-off-by: Wei Wang >> CC: Dr. David Alan Gilbert >> CC: Juan Quintela >> CC: Michael S. Tsirkin >> CC: Peter Xu >> --- >> include/migration/misc.h | 1 + >> migration/ram.c | 9 +++++++++ >> 2 files changed, 10 insertions(+) >> >> diff --git a/include/migration/misc.h b/include/migration/misc.h >> index 15f8d00..47e7ff5 100644 >> --- a/include/migration/misc.h >> +++ b/include/migration/misc.h >> @@ -37,6 +37,7 @@ void precopy_infrastructure_init(void); >> void precopy_add_notifier(NotifierWithReturn *n); >> void precopy_remove_notifier(NotifierWithReturn *n); >> int precopy_notify(PrecopyNotifyReason reason, Error **errp); >> +void precopy_disable_bulk_stage(void); >> >> void ram_mig_init(void); >> void qemu_guest_free_page_hint(void *addr, size_t len); >> diff --git a/migration/ram.c b/migration/ram.c >> index b90a3f2..739dc97 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -354,6 +354,15 @@ int precopy_notify(PrecopyNotifyReason reason, Error **errp) >> return notifier_with_return_list_notify(&precopy_notifier_list, &pnd); >> } >> >> +void precopy_disable_bulk_stage(void) >> +{ >> + if (!ram_state) { >> + return; >> + } >> + >> + ram_state->ram_bulk_stage = false; >> +} >> + > This one is a bit tricky. E.g., I think it'll at least affect XBZRLE > and compression somehow. Since we will have the on-off switch for > balloon free page hinting so the user can at least decide what > features to use, with that I don't see much issue with it so far. But > I'd also like to see how other people see this change. > Yes. I think it would be better that this optimization could co-exist with the compression feature. How about adding a new flag "ram_state->free_page_optimization_enabled"? We will then need a change in migration_bitmap_find_dirty: - if (rs->ram_bulk_stage && start > 0) { + if (!rs->free_page_optimization_enabled && rs->ram_bulk_stage && start > 0) { next = start + 1; } else { next = find_next_bit(bitmap, size, start); } Best, Wei From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: virtio-dev-return-5112-cohuck=redhat.com@lists.oasis-open.org Sender: List-Post: List-Help: List-Unsubscribe: List-Subscribe: Received: from lists.oasis-open.org (oasis-open.org [10.110.1.242]) by lists.oasis-open.org (Postfix) with ESMTP id 5430B985B7E for ; Mon, 3 Dec 2018 08:15:16 +0000 (UTC) Message-ID: <5C04E73F.9040202@intel.com> Date: Mon, 03 Dec 2018 16:20:15 +0800 From: Wei Wang MIME-Version: 1.0 References: <1543803511-34793-1-git-send-email-wei.w.wang@intel.com> <1543803511-34793-7-git-send-email-wei.w.wang@intel.com> <20181203053118.GD27620@xz-x1> In-Reply-To: <20181203053118.GD27620@xz-x1> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: [virtio-dev] Re: [PATCH v10 6/7] migration/ram.c: add a function to disable the bulk stage To: Peter Xu Cc: qemu-devel@nongnu.org, virtio-dev@lists.oasis-open.org, mst@redhat.com, quintela@redhat.com, dgilbert@redhat.com, pbonzini@redhat.com, liliang.opensource@gmail.com, nilal@redhat.com, riel@redhat.com List-ID: On 12/03/2018 01:31 PM, Peter Xu wrote: > On Mon, Dec 03, 2018 at 10:18:30AM +0800, Wei Wang wrote: >> This patch adds a function to enable a precopy notifier callback outside >> the migration subsystem to disable the bulk stage flag. This is needed by >> the free page optimization offered by virtio-balloon. >> >> Signed-off-by: Wei Wang >> CC: Dr. David Alan Gilbert >> CC: Juan Quintela >> CC: Michael S. Tsirkin >> CC: Peter Xu >> --- >> include/migration/misc.h | 1 + >> migration/ram.c | 9 +++++++++ >> 2 files changed, 10 insertions(+) >> >> diff --git a/include/migration/misc.h b/include/migration/misc.h >> index 15f8d00..47e7ff5 100644 >> --- a/include/migration/misc.h >> +++ b/include/migration/misc.h >> @@ -37,6 +37,7 @@ void precopy_infrastructure_init(void); >> void precopy_add_notifier(NotifierWithReturn *n); >> void precopy_remove_notifier(NotifierWithReturn *n); >> int precopy_notify(PrecopyNotifyReason reason, Error **errp); >> +void precopy_disable_bulk_stage(void); >> >> void ram_mig_init(void); >> void qemu_guest_free_page_hint(void *addr, size_t len); >> diff --git a/migration/ram.c b/migration/ram.c >> index b90a3f2..739dc97 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -354,6 +354,15 @@ int precopy_notify(PrecopyNotifyReason reason, Error **errp) >> return notifier_with_return_list_notify(&precopy_notifier_list, &pnd); >> } >> >> +void precopy_disable_bulk_stage(void) >> +{ >> + if (!ram_state) { >> + return; >> + } >> + >> + ram_state->ram_bulk_stage = false; >> +} >> + > This one is a bit tricky. E.g., I think it'll at least affect XBZRLE > and compression somehow. Since we will have the on-off switch for > balloon free page hinting so the user can at least decide what > features to use, with that I don't see much issue with it so far. But > I'd also like to see how other people see this change. > Yes. I think it would be better that this optimization could co-exist with the compression feature. How about adding a new flag "ram_state->free_page_optimization_enabled"? We will then need a change in migration_bitmap_find_dirty: - if (rs->ram_bulk_stage && start > 0) { + if (!rs->free_page_optimization_enabled && rs->ram_bulk_stage && start > 0) { next = start + 1; } else { next = find_next_bit(bitmap, size, start); } Best, Wei --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org