From 9a8671644621346835e984784ae73ff2c2b68e8f Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Fri, 6 Feb 2009 14:43:52 +0000 Subject: [PATCH] Fix 100% libvirt CPU usage when --timeout is set --- ChangeLog | 10 ++++++++++ qemud/event.c | 8 ++++++-- qemud/qemud.c | 50 ++++++++++++++++++++++++++++++++------------------ 3 files changed, 48 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index d27f6fe38e..f49e8c21a9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +Fri Feb 6 14:43:10 GMT 2009 Daniel P. Berrange + + Fix 100% CPU bound loop when libvirtd --timeout is used + * qemud/event.c: Don't assume pthread_t is equivalent to an + int, explicitly track whether event loop is active with a + flag independantly of the threadLeader variable + * qemud/qemud.c: Don't register/unregister shutdown timer + on each loop. Register once, and activate/deactivate when + required + Thu Feb 5 19:28:10 GMT 2009 John Levon * src/domain_conf.c: Check the last error, not the last diff --git a/qemud/event.c b/qemud/event.c index 6f0ef9c102..550d28c07e 100644 --- a/qemud/event.c +++ b/qemud/event.c @@ -68,6 +68,7 @@ struct virEventTimeout { /* State for the main event loop */ struct virEventLoop { pthread_mutex_t lock; + int running; pthread_t leader; int wakeupfd[2]; int handlesCount; @@ -521,6 +522,7 @@ int virEventRunOnce(void) { int ret, timeout, nfds; virEventLock(); + eventLoop.running = 1; eventLoop.leader = pthread_self(); if ((nfds = virEventMakePollFDs(&fds)) < 0) { virEventUnlock(); @@ -572,7 +574,7 @@ int virEventRunOnce(void) { return -1; } - eventLoop.leader = 0; + eventLoop.running = 0; virEventUnlock(); return 0; } @@ -611,7 +613,9 @@ int virEventInit(void) static int virEventInterruptLocked(void) { char c = '\0'; - if (pthread_self() == eventLoop.leader) + + if (!eventLoop.running || + pthread_self() == eventLoop.leader) return 0; if (safewrite(eventLoop.wakeupfd[1], &c, sizeof(c)) != sizeof(c)) diff --git a/qemud/qemud.c b/qemud/qemud.c index effb3368e2..a4add5a92e 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -2031,11 +2031,15 @@ static int qemudOneLoop(void) { return 0; } -static void qemudInactiveTimer(int timer ATTRIBUTE_UNUSED, void *data) { +static void qemudInactiveTimer(int timerid, void *data) { struct qemud_server *server = (struct qemud_server *)data; - DEBUG0("Got inactive timer expiry"); - if (!virStateActive()) { - DEBUG0("No state active, shutting down"); + + if (virStateActive() || + server->clients) { + DEBUG0("Timer expired but still active, not shutting down"); + virEventUpdateTimeoutImpl(timerid, -1); + } else { + DEBUG0("Timer expired and inactive, shutting down"); server->shutdown = 1; } } @@ -2066,9 +2070,18 @@ static void qemudFreeClient(struct qemud_client *client) { static int qemudRunLoop(struct qemud_server *server) { int timerid = -1; int ret = -1, i; + int timerActive = 0; virMutexLock(&server->lock); + if (timeout > 0 && + (timerid = virEventAddTimeoutImpl(-1, + qemudInactiveTimer, + server, NULL)) < 0) { + VIR_ERROR0(_("Failed to register shutdown timeout")); + return -1; + } + if (min_workers > max_workers) max_workers = min_workers; @@ -2089,11 +2102,21 @@ static int qemudRunLoop(struct qemud_server *server) { * if any drivers have active state, if not * shutdown after timeout seconds */ - if (timeout > 0 && !virStateActive() && !server->clients) { - timerid = virEventAddTimeoutImpl(timeout*1000, - qemudInactiveTimer, - server, NULL); - DEBUG("Scheduling shutdown timer %d", timerid); + if (timeout > 0) { + if (timerActive) { + if (server->clients) { + DEBUG("Deactivating shutdown timer %d", timerid); + virEventUpdateTimeoutImpl(timerid, -1); + timerActive = 0; + } + } else { + if (!virStateActive() && + !server->clients) { + DEBUG("Activating shutdown timer %d", timerid); + virEventUpdateTimeoutImpl(timerid, timeout * 1000); + timerActive = 1; + } + } } virMutexUnlock(&server->lock); @@ -2147,15 +2170,6 @@ static int qemudRunLoop(struct qemud_server *server) { } } - /* Unregister any timeout that's active, since we - * just had an event processed - */ - if (timerid != -1) { - DEBUG("Removing shutdown timer %d", timerid); - virEventRemoveTimeoutImpl(timerid); - timerid = -1; - } - if (server->shutdown) { ret = 0; break;