From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44633) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dVzQn-0007wd-JE for qemu-devel@nongnu.org; Fri, 14 Jul 2017 08:10:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dVzQl-000108-0d for qemu-devel@nongnu.org; Fri, 14 Jul 2017 08:10:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56788) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dVzQk-0000za-E5 for qemu-devel@nongnu.org; Fri, 14 Jul 2017 08:10:46 -0400 Date: Fri, 14 Jul 2017 13:10:41 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20170714121040.GC2091@work-vm> References: <1499925175-21218-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <1499925175-21218-2-git-send-email-zhangchen.fnst@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1499925175-21218-2-git-send-email-zhangchen.fnst@cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH V2 1/4] net/colo-compare.c: Add checkpoint min period to optimize performance List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhang Chen Cc: qemu devel , Jason Wang , Li Zhijian , zhanghailiang * Zhang Chen (zhangchen.fnst@cn.fujitsu.com) wrote: > If colo-compare find out the first different packet that means > the following packet almost is different. we needn't do a lot > of checkpoint in this time, so we set the no-need-checkpoint > peroid, default just set 3 second. > > Signed-off-by: Zhang Chen > --- > net/colo-compare.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index 6d500e1..0f8e198 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -40,6 +40,9 @@ > /* TODO: Should be configurable */ > #define REGULAR_PACKET_CHECK_MS 3000 > > +/* TODO: Should be configurable */ Yes it should! > +#define CHECKPOINT_MIN_TIME 3000 > + > /* > + CompareState ++ > | | > @@ -455,6 +458,7 @@ static void colo_compare_connection(void *opaque, void *user_data) > Packet *pkt = NULL; > GList *result = NULL; > int ret; > + static int64_t checkpoint_time_ms; > > while (!g_queue_is_empty(&conn->primary_list) && > !g_queue_is_empty(&conn->secondary_list)) { > @@ -494,7 +498,14 @@ static void colo_compare_connection(void *opaque, void *user_data) > */ > trace_colo_compare_main("packet different"); > g_queue_push_tail(&conn->primary_list, pkt); > - /* TODO: colo_notify_checkpoint();*/ > + > + if (pkt->creation_ms - checkpoint_time_ms > CHECKPOINT_MIN_TIME) { > + /* > + * TODO: Notify colo frame to do checkpoint. > + * colo_compare_inconsistent_notify(); > + */ > + checkpoint_time_ms = pkt->creation_ms; > + } You need to be careful how this interacts with the actual start of the checkpoint. Lets say you have two miscompared packets close to each other: miscompare! checkpoint miscompare! ignore it because it was close to the 1st one That means we never trigger the 2nd checkpoint and it'll carry on until the maximum checkpoint length. But also, I think you need to consider what happens to future packets being compared; you can't release any packets now until the checkpoint as soon as you know there's a miscompare. Dave > break; > } > } > -- > 2.7.4 > > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK