From edcb926f9d4f61a3b779466970a0f4ec823b46a8 Mon Sep 17 00:00:00 2001 From: Richard W.M. Jones Date: Apr 19 2016 10:53:54 +0000 Subject: Fix 200ms performance problem when waiting for monitor socket of new domains. --- diff --git a/0001-Add-functions-for-handling-exponential-backoff-loops.patch b/0001-Add-functions-for-handling-exponential-backoff-loops.patch new file mode 100644 index 0000000..06c09de --- /dev/null +++ b/0001-Add-functions-for-handling-exponential-backoff-loops.patch @@ -0,0 +1,291 @@ +From beaa447a2982bc78adb26c183560d0ee566c1268 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Fri, 8 Apr 2016 12:11:10 +0100 +Subject: [PATCH] Add functions for handling exponential backoff loops. + +In a few places in libvirt we busy-wait for events, for example qemu +creating a monitor socket. This is problematic because: + + - We need to choose a sufficiently small polling period so that + libvirt doesn't add unnecessary delays. + + - We need to choose a sufficiently large polling period so that + the effect of busy-waiting doesn't affect the system. + +The solution to this conflict is to use an exponential backoff. + +This patch adds two functions to hide the details, and modifies a few +places where we currently busy-wait. + +Signed-off-by: Richard W.M. Jones +--- + src/fdstream.c | 10 +++--- + src/libvirt_private.syms | 2 ++ + src/qemu/qemu_agent.c | 11 ++++--- + src/qemu/qemu_monitor.c | 12 ++++--- + src/util/virtime.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ + src/util/virtime.h | 11 +++++++ + 6 files changed, 113 insertions(+), 14 deletions(-) + +diff --git a/src/fdstream.c b/src/fdstream.c +index ef118b5..a6a0fbe 100644 +--- a/src/fdstream.c ++++ b/src/fdstream.c +@@ -42,6 +42,7 @@ + #include "virfile.h" + #include "configmake.h" + #include "virstring.h" ++#include "virtime.h" + + #define VIR_FROM_THIS VIR_FROM_STREAMS + +@@ -520,8 +521,7 @@ int virFDStreamConnectUNIX(virStreamPtr st, + bool abstract) + { + struct sockaddr_un sa; +- size_t i = 0; +- int timeout = 3; ++ virTimeBackOffVar timeout; + int ret; + + int fd = socket(AF_UNIX, SOCK_STREAM, 0); +@@ -541,7 +541,9 @@ int virFDStreamConnectUNIX(virStreamPtr st, + goto error; + } + +- do { ++ if (virTimeBackOffStart(&timeout, 1, 3*1000 /* ms */) < 0) ++ goto error; ++ while (virTimeBackOffWait(&timeout)) { + ret = connect(fd, (struct sockaddr *)&sa, sizeof(sa)); + if (ret == 0) + break; +@@ -553,7 +555,7 @@ int virFDStreamConnectUNIX(virStreamPtr st, + } + + goto error; +- } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0)); ++ } + + if (virFDStreamOpenInternal(st, fd, NULL, -1, 0) < 0) + goto error; +diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms +index b626fd2..61fc500 100644 +--- a/src/libvirt_private.syms ++++ b/src/libvirt_private.syms +@@ -2373,6 +2373,8 @@ virThreadPoolSendJob; + + + # util/virtime.h ++virTimeBackOffStart; ++virTimeBackOffWait; + virTimeFieldsNow; + virTimeFieldsNowRaw; + virTimeFieldsThen; +diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c +index 559d79f..0610f75 100644 +--- a/src/qemu/qemu_agent.c ++++ b/src/qemu/qemu_agent.c +@@ -173,9 +173,8 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) + { + struct sockaddr_un addr; + int monfd; +- int timeout = 3; /* In seconds */ +- int ret; +- size_t i = 0; ++ virTimeBackOffVar timeout; ++ int ret = -1; + + *inProgress = false; + +@@ -207,7 +206,9 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) + goto error; + } + +- do { ++ if (virTimeBackOffStart(&timeout, 1, 3*1000 /* ms */) < 0) ++ goto error; ++ while (virTimeBackOffWait(&timeout)) { + ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); + + if (ret == 0) +@@ -232,7 +233,7 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) + _("failed to connect to monitor socket")); + goto error; + +- } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0)); ++ } + + if (ret != 0) { + virReportSystemError(errno, "%s", +diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c +index 83551a8..2847cef 100644 +--- a/src/qemu/qemu_monitor.c ++++ b/src/qemu/qemu_monitor.c +@@ -42,6 +42,7 @@ + #include "virobject.h" + #include "virprobe.h" + #include "virstring.h" ++#include "virtime.h" + + #ifdef WITH_DTRACE_PROBES + # include "libvirt_qemu_probes.h" +@@ -327,9 +328,8 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid) + { + struct sockaddr_un addr; + int monfd; +- int timeout = 30; /* In seconds */ +- int ret; +- size_t i = 0; ++ virTimeBackOffVar timeout; ++ int ret = -1; + + if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { + virReportSystemError(errno, +@@ -345,7 +345,9 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid) + goto error; + } + +- do { ++ if (virTimeBackOffStart(&timeout, 1, 30*1000 /* ms */) < 0) ++ goto error; ++ while (virTimeBackOffWait(&timeout)) { + ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); + + if (ret == 0) +@@ -362,7 +364,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid) + _("failed to connect to monitor socket")); + goto error; + +- } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0)); ++ } + + if (ret != 0) { + virReportSystemError(errno, "%s", +diff --git a/src/util/virtime.c b/src/util/virtime.c +index 9d365d5..aac9691 100644 +--- a/src/util/virtime.c ++++ b/src/util/virtime.c +@@ -34,14 +34,18 @@ + #include + + #include ++#include + #include + + #include "virtime.h" + #include "viralloc.h" + #include "virerror.h" ++#include "virlog.h" + + #define VIR_FROM_THIS VIR_FROM_NONE + ++VIR_LOG_INIT("util.time"); ++ + /* We prefer clock_gettime if available because that is officially + * async signal safe according to POSIX. Many platforms lack it + * though, so fallback to gettimeofday everywhere else +@@ -363,3 +367,80 @@ virTimeLocalOffsetFromUTC(long *offset) + *offset = current - utc; + return 0; + } ++ ++/** ++ * virTimeBackOffStart: ++ * @var: Timeout variable (with type virTimeBackOffVar). ++ * @first: Initial time to wait (milliseconds). ++ * @timeout: Timeout (milliseconds). ++ * ++ * Initialize the timeout variable @var and start the timer running. ++ * ++ * Returns 0 on success, -1 on error and raises a libvirt error. ++ */ ++int ++virTimeBackOffStart(virTimeBackOffVar *var, ++ unsigned long long first, unsigned long long timeout) ++{ ++ if (virTimeMillisNow(&var->start_t) < 0) ++ return -1; ++ ++ var->next = first; ++ var->limit_t = var->start_t + timeout; ++ return 0; ++} ++ ++/** ++ * virTimeBackOffWait ++ * @var: Timeout variable (with type virTimeBackOffVar *). ++ * ++ * You must initialize @var first by calling the following function, ++ * which also starts the timer: ++ * ++ * if (virTimeBackOffStart(&var, first, timeout) < 0) { ++ * // handle errors ++ * } ++ * ++ * Then you use a while loop: ++ * ++ * while (virTimeBackOffWait(&var)) { ++ * //... ++ * } ++ * ++ * The while loop that runs the body of the code repeatedly, with an ++ * exponential backoff. It first waits for first milliseconds, then ++ * runs the body, then waits for 2*first ms, then runs the body again. ++ * Then 4*first ms, and so on. ++ * ++ * When timeout milliseconds is reached, the while loop ends. ++ * ++ * The body should use "break" or "goto" when whatever condition it is ++ * testing for succeeds (or there is an unrecoverable error). ++ */ ++bool ++virTimeBackOffWait(virTimeBackOffVar *var) ++{ ++ unsigned long long t, next; ++ ++ ignore_value(virTimeMillisNowRaw(&t)); ++ ++ VIR_DEBUG("t=%llu, limit=%llu", t, var->limit_t); ++ ++ if (t > var->limit_t) ++ return 0; /* ends the while loop */ ++ ++ next = var->next; ++ var->next *= 2; ++ ++ /* If sleeping would take us beyond the limit, then shorten the ++ * sleep. This is so we always run the body just before the final ++ * timeout. ++ */ ++ if (t + next > var->limit_t) ++ next = var->limit_t - t; ++ ++ VIR_DEBUG("sleeping for %llu ms", next); ++ ++ usleep(next * 1000); ++ return 1; ++} +diff --git a/src/util/virtime.h b/src/util/virtime.h +index 8ebad38..fbcd3ba 100644 +--- a/src/util/virtime.h ++++ b/src/util/virtime.h +@@ -64,4 +64,15 @@ char *virTimeStringThen(unsigned long long when); + int virTimeLocalOffsetFromUTC(long *offset) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + ++typedef struct { ++ unsigned long long start_t; ++ unsigned long long next; ++ unsigned long long limit_t; ++} virTimeBackOffVar; ++ ++int virTimeBackOffStart(virTimeBackOffVar *var, ++ unsigned long long first, unsigned long long timeout); ++ ++bool virTimeBackOffWait(virTimeBackOffVar *var); ++ + #endif +-- +2.7.4 + diff --git a/libvirt.spec b/libvirt.spec index ea810ae..11446a3 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -380,7 +380,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 1.3.3 -Release: 2%{?dist}%{?extra_release} +Release: 3%{?dist}%{?extra_release} License: LGPLv2+ Group: Development/Libraries BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root @@ -397,6 +397,11 @@ Patch0001: 0001-qemu-support-virt-2.6-machine-type-on-arm.patch Patch0002: 0002-build-cleanup-GCC-4.6-Wlogical-op-workaround.patch Patch0003: 0003-build-add-GCC-6.0-Wlogical-op-workaround.patch +# Fix 200ms performance problem when waiting for monitor socket of +# new domains. +# Upstream commit beaa447a2982bc78adb26c183560d0ee566c1268. +Patch0004: 0001-Add-functions-for-handling-exponential-backoff-loops.patch + %if %{with_libvirtd} Requires: libvirt-daemon = %{version}-%{release} %if %{with_network} @@ -2405,6 +2410,9 @@ exit 0 %doc examples/systemtap %changelog +* Tue Apr 19 2016 Cole Robinson - 1.3.3-3 +- Fix 200ms performance problem when waiting for monitor socket of new domains. + * Thu Apr 14 2016 Cole Robinson - 1.3.3-2 - libvirt assigns same address to two PCI devices (bz #1325085) - Fix build with -Werror