diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 2e38088d0f..8da07adfe7 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1354,8 +1354,8 @@ static int method_reload(sd_bus_message *message, void *userdata, sd_bus_error * * is finished. That way the caller knows when the reload * finished. */ - assert(!m->queued_message); - r = sd_bus_message_new_method_return(message, &m->queued_message); + assert(!m->pending_reload_message); + r = sd_bus_message_new_method_return(message, &m->pending_reload_message); if (r < 0) return r; diff --git a/src/core/dbus.c b/src/core/dbus.c index c2f77b8eef..6c00f734a4 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -48,23 +48,22 @@ static void destroy_bus(Manager *m, sd_bus **bus); -int bus_send_queued_message(Manager *m) { +int bus_send_pending_reload_message(Manager *m) { int r; assert(m); - if (!m->queued_message) + if (!m->pending_reload_message) return 0; - /* If we cannot get rid of this message we won't dispatch any - * D-Bus messages, so that we won't end up wanting to queue - * another message. */ + /* If we cannot get rid of this message we won't dispatch any D-Bus messages, so that we won't end up wanting + * to queue another message. */ - r = sd_bus_send(NULL, m->queued_message, NULL); + r = sd_bus_send(NULL, m->pending_reload_message, NULL); if (r < 0) - log_warning_errno(r, "Failed to send queued message: %m"); + log_warning_errno(r, "Failed to send queued message, ignoring: %m"); - m->queued_message = sd_bus_message_unref(m->queued_message); + m->pending_reload_message = sd_bus_message_unref(m->pending_reload_message); return 0; } @@ -1073,8 +1072,8 @@ static void destroy_bus(Manager *m, sd_bus **bus) { u->bus_track = sd_bus_track_unref(u->bus_track); /* Get rid of queued message on this bus */ - if (m->queued_message && sd_bus_message_get_bus(m->queued_message) == *bus) - m->queued_message = sd_bus_message_unref(m->queued_message); + if (m->pending_reload_message && sd_bus_message_get_bus(m->pending_reload_message) == *bus) + m->pending_reload_message = sd_bus_message_unref(m->pending_reload_message); /* Possibly flush unwritten data, but only if we are * unprivileged, since we don't want to sync here */ diff --git a/src/core/dbus.h b/src/core/dbus.h index 382a96da7d..f1c0fa86c0 100644 --- a/src/core/dbus.h +++ b/src/core/dbus.h @@ -5,7 +5,7 @@ #include "manager.h" -int bus_send_queued_message(Manager *m); +int bus_send_pending_reload_message(Manager *m); int bus_init_private(Manager *m); int bus_init_api(Manager *m); diff --git a/src/core/manager.c b/src/core/manager.c index bee415c725..a7ee6a6523 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2157,57 +2157,71 @@ static unsigned manager_dispatch_dbus_queue(Manager *m) { assert(m); + /* Avoid recursion */ if (m->dispatching_dbus_queue) return 0; - /* Anything to do at all? */ - if (!m->dbus_unit_queue && !m->dbus_job_queue && !m->send_reloading_done && !m->queued_message) - return 0; + /* When we are reloading, let's not wait with generating signals, since we need to exit the manager as quickly + * as we can. There's no point in throttling generation of signals in that case. */ + if (MANAGER_IS_RELOADING(m) || m->send_reloading_done || m->pending_reload_message) + budget = (unsigned) -1; /* infinite budget in this case */ + else { + /* Anything to do at all? */ + if (!m->dbus_unit_queue && !m->dbus_job_queue) + return 0; - /* Do we have overly many messages queued at the moment? If so, let's not enqueue more on top, let's sit this - * cycle out, and process things in a later cycle when the queues got a bit emptier. */ - if (manager_bus_n_queued_write(m) > MANAGER_BUS_BUSY_THRESHOLD) - return 0; + /* Do we have overly many messages queued at the moment? If so, let's not enqueue more on top, let's + * sit this cycle out, and process things in a later cycle when the queues got a bit emptier. */ + if (manager_bus_n_queued_write(m) > MANAGER_BUS_BUSY_THRESHOLD) + return 0; - /* Only process a certain number of units/jobs per event loop iteration. Even if the bus queue wasn't overly - * full before this call we shouldn't increase it in size too wildly in one step, and we shouldn't monopolize - * CPU time with generating these messages. Note the difference in counting of this "budget" and the - * "threshold" above: the "budget" is decreased only once per generated message, regardless how many - * busses/direct connections it is enqueued on, while the "threshold" is applied to each queued instance of bus - * message, i.e. if the same message is enqueued to five busses/direct connections it will be counted five - * times. This difference in counting ("references" vs. "instances") is primarily a result of the fact that - * it's easier to implement it this way, however it also reflects the thinking that the "threshold" should put - * a limit on used queue memory, i.e. space, while the "budget" should put a limit on time. Also note that - * the "threshold" is currently chosen much higher than the "budget". */ - budget = MANAGER_BUS_MESSAGE_BUDGET; + /* Only process a certain number of units/jobs per event loop iteration. Even if the bus queue wasn't + * overly full before this call we shouldn't increase it in size too wildly in one step, and we + * shouldn't monopolize CPU time with generating these messages. Note the difference in counting of + * this "budget" and the "threshold" above: the "budget" is decreased only once per generated message, + * regardless how many busses/direct connections it is enqueued on, while the "threshold" is applied to + * each queued instance of bus message, i.e. if the same message is enqueued to five busses/direct + * connections it will be counted five times. This difference in counting ("references" + * vs. "instances") is primarily a result of the fact that it's easier to implement it this way, + * however it also reflects the thinking that the "threshold" should put a limit on used queue memory, + * i.e. space, while the "budget" should put a limit on time. Also note that the "threshold" is + * currently chosen much higher than the "budget". */ + budget = MANAGER_BUS_MESSAGE_BUDGET; + } m->dispatching_dbus_queue = true; - while (budget > 0 && (u = m->dbus_unit_queue)) { + while (budget != 0 && (u = m->dbus_unit_queue)) { assert(u->in_dbus_queue); bus_unit_send_change_signal(u); - n++, budget--; + n++; + + if (budget != (unsigned) -1) + budget--; } - while (budget > 0 && (j = m->dbus_job_queue)) { + while (budget != 0 && (j = m->dbus_job_queue)) { assert(j->in_dbus_queue); bus_job_send_change_signal(j); - n++, budget--; + n++; + + if (budget != (unsigned) -1) + budget--; } m->dispatching_dbus_queue = false; - if (budget > 0 && m->send_reloading_done) { + if (m->send_reloading_done) { m->send_reloading_done = false; bus_manager_send_reloading(m, false); - n++, budget--; + n++; } - if (budget > 0 && m->queued_message) { - bus_send_queued_message(m); + if (m->pending_reload_message) { + bus_send_pending_reload_message(m); n++; } diff --git a/src/core/manager.h b/src/core/manager.h index 45fa5d35d8..1977735da1 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -248,7 +248,7 @@ struct Manager { /* This is used during reloading: before the reload we queue * the reply message here, and afterwards we send it */ - sd_bus_message *queued_message; + sd_bus_message *pending_reload_message; Hashmap *watch_bus; /* D-Bus names => Unit object n:1 */