| Message ID | 20200918092203.20384-4-chen.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Several optimization and bugfix for COLO compare. | expand |
On 9/18/20 5:22 PM, Zhang Chen wrote: > From: Zhang Chen <chen.zhang@intel.com> > > Detect queued secondary packet to sync VM state in time. > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > --- > net/colo-compare.c | 25 ++++++++++++++++--------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index 3b72309d08..f7271b976f 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier *notify) > static int colo_old_packet_check_one_conn(Connection *conn, > CompareState *s) > { > - GList *result = NULL; > - > - result = g_queue_find_custom(&conn->primary_list, > - &s->compare_timeout, > - (GCompareFunc)colo_old_packet_check_one); > + if (!g_queue_is_empty(&conn->primary_list)) { Looks we don't need to check is_empty > + if (g_queue_find_custom(&conn->primary_list, > + &s->compare_timeout, > + (GCompareFunc)colo_old_packet_check_one)) > + goto out; > + } > > - if (result) { > - /* Do checkpoint will flush old packet */ > - colo_compare_inconsistency_notify(s); > - return 0; > + if (!g_queue_is_empty(&conn->secondary_list)) { Ditto Thanks > + if (g_queue_find_custom(&conn->secondary_list, > + &s->compare_timeout, > + (GCompareFunc)colo_old_packet_check_one)) > + goto out; > } > > return 1; > + > +out: > + /* Do checkpoint will flush old packet */ > + colo_compare_inconsistency_notify(s); > + return 0; > } > > /*
> -----Original Message----- > From: Li Zhijian <lizhijian@cn.fujitsu.com> > Sent: Tuesday, September 22, 2020 2:47 PM > To: Zhang, Chen <chen.zhang@intel.com>; Jason Wang > <jasowang@redhat.com>; qemu-dev <qemu-devel@nongnu.org> > Cc: Zhang Chen <zhangckid@gmail.com> > Subject: Re: [PATCH 3/4] net/colo-compare.c: Add secondary old packet > detection > > > > On 9/18/20 5:22 PM, Zhang Chen wrote: > > From: Zhang Chen <chen.zhang@intel.com> > > > > Detect queued secondary packet to sync VM state in time. > > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > > --- > > net/colo-compare.c | 25 ++++++++++++++++--------- > > 1 file changed, 16 insertions(+), 9 deletions(-) > > > > diff --git a/net/colo-compare.c b/net/colo-compare.c index > > 3b72309d08..f7271b976f 100644 > > --- a/net/colo-compare.c > > +++ b/net/colo-compare.c > > @@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier > *notify) > > static int colo_old_packet_check_one_conn(Connection *conn, > > CompareState *s) > > { > > - GList *result = NULL; > > - > > - result = g_queue_find_custom(&conn->primary_list, > > - &s->compare_timeout, > > - (GCompareFunc)colo_old_packet_check_one); > > + if (!g_queue_is_empty(&conn->primary_list)) { > Looks we don't need to check is_empty Re-checked glib code, it just checked the queue rather than inside content. Maybe check empty before that will benefit performance. Thanks Zhang Chen > > > + if (g_queue_find_custom(&conn->primary_list, > > + &s->compare_timeout, > > + (GCompareFunc)colo_old_packet_check_one)) > > + goto out; > > + } > > > > - if (result) { > > - /* Do checkpoint will flush old packet */ > > - colo_compare_inconsistency_notify(s); > > - return 0; > > + if (!g_queue_is_empty(&conn->secondary_list)) { > Ditto > > Thanks > > + if (g_queue_find_custom(&conn->secondary_list, > > + &s->compare_timeout, > > + (GCompareFunc)colo_old_packet_check_one)) > > + goto out; > > } > > > > return 1; > > + > > +out: > > + /* Do checkpoint will flush old packet */ > > + colo_compare_inconsistency_notify(s); > > + return 0; > > } > > > > /* > >
On 9/23/20 2:47 PM, Zhang, Chen wrote: > >> -----Original Message----- >> From: Li Zhijian <lizhijian@cn.fujitsu.com> >> Sent: Tuesday, September 22, 2020 2:47 PM >> To: Zhang, Chen <chen.zhang@intel.com>; Jason Wang >> <jasowang@redhat.com>; qemu-dev <qemu-devel@nongnu.org> >> Cc: Zhang Chen <zhangckid@gmail.com> >> Subject: Re: [PATCH 3/4] net/colo-compare.c: Add secondary old packet >> detection >> >> >> >> On 9/18/20 5:22 PM, Zhang Chen wrote: >>> From: Zhang Chen <chen.zhang@intel.com> >>> >>> Detect queued secondary packet to sync VM state in time. >>> >>> Signed-off-by: Zhang Chen <chen.zhang@intel.com> >>> --- >>> net/colo-compare.c | 25 ++++++++++++++++--------- >>> 1 file changed, 16 insertions(+), 9 deletions(-) >>> >>> diff --git a/net/colo-compare.c b/net/colo-compare.c index >>> 3b72309d08..f7271b976f 100644 >>> --- a/net/colo-compare.c >>> +++ b/net/colo-compare.c >>> @@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier >> *notify) >>> static int colo_old_packet_check_one_conn(Connection *conn, >>> CompareState *s) >>> { >>> - GList *result = NULL; >>> - >>> - result = g_queue_find_custom(&conn->primary_list, >>> - &s->compare_timeout, >>> - (GCompareFunc)colo_old_packet_check_one); >>> + if (!g_queue_is_empty(&conn->primary_list)) { >> Looks we don't need to check is_empty > Re-checked glib code, it just checked the queue rather than inside content. > Maybe check empty before that will benefit performance. Yeah, you are right Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com> Thank > > Thanks > Zhang Chen > >>> + if (g_queue_find_custom(&conn->primary_list, >>> + &s->compare_timeout, >>> + (GCompareFunc)colo_old_packet_check_one)) >>> + goto out; >>> + } >>> >>> - if (result) { >>> - /* Do checkpoint will flush old packet */ >>> - colo_compare_inconsistency_notify(s); >>> - return 0; >>> + if (!g_queue_is_empty(&conn->secondary_list)) { >> Ditto >> >> Thanks >>> + if (g_queue_find_custom(&conn->secondary_list, >>> + &s->compare_timeout, >>> + (GCompareFunc)colo_old_packet_check_one)) >>> + goto out; >>> } >>> >>> return 1; >>> + >>> +out: >>> + /* Do checkpoint will flush old packet */ >>> + colo_compare_inconsistency_notify(s); >>> + return 0; >>> } >>> >>> /* >> > >
diff --git a/net/colo-compare.c b/net/colo-compare.c index 3b72309d08..f7271b976f 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier *notify) static int colo_old_packet_check_one_conn(Connection *conn, CompareState *s) { - GList *result = NULL; - - result = g_queue_find_custom(&conn->primary_list, - &s->compare_timeout, - (GCompareFunc)colo_old_packet_check_one); + if (!g_queue_is_empty(&conn->primary_list)) { + if (g_queue_find_custom(&conn->primary_list, + &s->compare_timeout, + (GCompareFunc)colo_old_packet_check_one)) + goto out; + } - if (result) { - /* Do checkpoint will flush old packet */ - colo_compare_inconsistency_notify(s); - return 0; + if (!g_queue_is_empty(&conn->secondary_list)) { + if (g_queue_find_custom(&conn->secondary_list, + &s->compare_timeout, + (GCompareFunc)colo_old_packet_check_one)) + goto out; } return 1; + +out: + /* Do checkpoint will flush old packet */ + colo_compare_inconsistency_notify(s); + return 0; } /*