Skip to content

Commit 649a5ff

Browse files
poetteringpoettering
authored
Merge pull request systemd#8171 from poettering/sd-bus-queue-limit
try not to overload pid1's bus message write queue
2 parents 8c89cb5 + e0a0858 commit 649a5ff

File tree

8 files changed

+237
-9
lines changed

8 files changed

+237
-9
lines changed

‎man/rules/meson.build‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ manpages = [
186186
['SD_BUS_ERROR_END', 'SD_BUS_ERROR_MAP', 'sd_bus_error_map'],
187187
''],
188188
['sd_bus_get_fd', '3', [], ''],
189+
['sd_bus_get_n_queued_read', '3', ['sd_bus_get_n_queued_write'], ''],
189190
['sd_bus_is_open', '3', ['sd_bus_is_ready'], ''],
190191
['sd_bus_message_append', '3', ['sd_bus_message_appendv'], ''],
191192
['sd_bus_message_append_array',

‎man/sd_bus_get_n_queued_read.xml‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
<?xml version='1.0'?> <!--*- Mode: nxml; nxml-child-indent: 2; indent-tabs-mode: nil -*-->
2+
<!DOCTYPE refentry PUBLIC "-//OASIS//DTD DocBook XML V4.2//EN"
3+
"http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd">
4+
5+
<!--
6+
SPDX-License-Identifier: LGPL-2.1+
7+
8+
This file is part of systemd.
9+
10+
Copyright 2018 Lennart Poettering
11+
12+
systemd is free software; you can redistribute it and/or modify it
13+
under the terms of the GNU Lesser General Public License as published by
14+
the Free Software Foundation; either version 2.1 of the License, or
15+
(at your option) any later version.
16+
17+
systemd is distributed in the hope that it will be useful, but
18+
WITHOUT ANY WARRANTY; without even the implied warranty of
19+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
20+
Lesser General Public License for more details.
21+
22+
You should have received a copy of the GNU Lesser General Public License
23+
along with systemd; If not, see <http://www.gnu.org/licenses/>.
24+
-->
25+
26+
<refentry id="sd_bus_get_n_queued_read">
27+
28+
<refentryinfo>
29+
<title>sd_bus_get_fd</title>
30+
<productname>systemd</productname>
31+
32+
<authorgroup>
33+
<author>
34+
<contrib>Developer</contrib>
35+
<firstname>Lennart</firstname>
36+
<surname>Poettering</surname>
37+
<email>lennart@poettering.net</email>
38+
</author>
39+
</authorgroup>
40+
</refentryinfo>
41+
42+
<refmeta>
43+
<refentrytitle>sd_bus_get_n_queued_read</refentrytitle>
44+
<manvolnum>3</manvolnum>
45+
</refmeta>
46+
47+
<refnamediv>
48+
<refname>sd_bus_get_n_queued_read</refname>
49+
<refname>sd_bus_get_n_queued_write</refname>
50+
51+
<refpurpose>Get the number of pending bus messages in the read and write queues of a bus connection object</refpurpose>
52+
</refnamediv>
53+
54+
<refsynopsisdiv>
55+
<funcsynopsis>
56+
<funcsynopsisinfo>#include &lt;systemd/sd-bus.h&gt;</funcsynopsisinfo>
57+
58+
<funcprototype>
59+
<funcdef>int <function>sd_bus_get_n_queued_read</function></funcdef>
60+
<paramdef>sd_bus *<parameter>bus</parameter></paramdef>
61+
<paramdef>uint64_t *<parameter>ret</parameter></paramdef>
62+
</funcprototype>
63+
64+
<funcprototype>
65+
<funcdef>int <function>sd_bus_get_n_queued_write</function></funcdef>
66+
<paramdef>sd_bus *<parameter>bus</parameter></paramdef>
67+
<paramdef>uint64_t *<parameter>ret</parameter></paramdef>
68+
</funcprototype>
69+
</funcsynopsis>
70+
</refsynopsisdiv>
71+
72+
<refsect1>
73+
<title>Description</title>
74+
75+
<para>
76+
<function>sd_bus_get_n_queued_read()</function> may be used to query the number of bus messages in the read queue
77+
of a bus connection object. The read queue contains all messages read from the transport medium (e.g. network
78+
socket) but not yet processed locally. The function expects two arguments: the bus object to query the number of
79+
queued messages of, and a pointer to a 64bit counter variable to write the current queue size to. Use
80+
<function>sd_bus_process()</function> in order to process queued messages, i.e. to reduce the size of the read
81+
queue (as well as, in fact, the write queue, see below) when it is non-zero.
82+
</para>
83+
84+
<para>
85+
Similar, <function>sd_bus_get_n_queued_write()</function> may be used to query the number of currently pending
86+
bus messages in the write queue of a bus connection object. The write queue contains all messages enqueued into
87+
the connection with a call such as <function>sd_bus_send()</function> but not yet written to the transport
88+
medium. The expected arguments are similar to the ones of <function>sd_bus_get_n_queued_read()</function>. Here
89+
too, use <function>sd_bus_process()</function> to reduce the size of the write queue. Alternatively, use
90+
<function>sd_bus_flush()</function> to synchronously write out any pending bus messages until the write queue is
91+
empty.
92+
</para>
93+
</refsect1>
94+
95+
<refsect1>
96+
<title>Return Value</title>
97+
98+
<para>On success, these functions return 0 or a positive integer. On failure, they return a negative errno-style
99+
error code.</para>
100+
</refsect1>
101+
102+
<refsect1>
103+
<title>Errors</title>
104+
105+
<para>Returned errors may indicate the following problems:</para>
106+
107+
<variablelist>
108+
<varlistentry>
109+
<term><constant>-ECHILD</constant></term>
110+
111+
<listitem><para>The bus connection has been created in a different process.</para></listitem>
112+
</varlistentry>
113+
114+
</variablelist>
115+
</refsect1>
116+
117+
<refsect1>
118+
<title>See Also</title>
119+
120+
<para>
121+
<citerefentry><refentrytitle>systemd</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
122+
<citerefentry><refentrytitle>sd-bus</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
123+
<citerefentry><refentrytitle>sd_bus_process</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
124+
<citerefentry><refentrytitle>sd_bus_send</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
125+
<citerefentry><refentrytitle>sd_bus_flush</refentrytitle><manvolnum>3</manvolnum></citerefentry>
126+
</para>
127+
</refsect1>
128+
129+
</refentry>

‎src/core/dbus.c‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,6 +1197,11 @@ int bus_foreach_bus(
11971197

11981198
/* Send to all direct buses, unconditionally */
11991199
SET_FOREACH(b, m->private_buses, i) {
1200+
1201+
/* Don't bother with enqueing these messages to clients that haven't started yet */
1202+
if (sd_bus_is_ready(b) <= 0)
1203+
continue;
1204+
12001205
r = send_message(b, userdata);
12011206
if (r < 0)
12021207
ret = r;
@@ -1274,3 +1279,34 @@ int bus_verify_reload_daemon_async(Manager *m, sd_bus_message *call, sd_bus_erro
12741279
int bus_verify_set_environment_async(Manager *m, sd_bus_message *call, sd_bus_error *error) {
12751280
return bus_verify_polkit_async(call, CAP_SYS_ADMIN, "org.freedesktop.systemd1.set-environment", NULL, false, UID_INVALID, &m->polkit_registry, error);
12761281
}
1282+
1283+
uint64_t manager_bus_n_queued_write(Manager *m) {
1284+
uint64_t c = 0;
1285+
Iterator i;
1286+
sd_bus *b;
1287+
int r;
1288+
1289+
/* Returns the total number of messages queued for writing on all our direct and API busses. */
1290+
1291+
SET_FOREACH(b, m->private_buses, i) {
1292+
uint64_t k;
1293+
1294+
r = sd_bus_get_n_queued_write(b, &k);
1295+
if (r < 0)
1296+
log_debug_errno(r, "Failed to query queued messages for private bus: %m");
1297+
else
1298+
c += k;
1299+
}
1300+
1301+
if (m->api_bus) {
1302+
uint64_t k;
1303+
1304+
r = sd_bus_get_n_queued_write(m->api_bus, &k);
1305+
if (r < 0)
1306+
log_debug_errno(r, "Failed to query queued messages for API bus: %m");
1307+
else
1308+
c += k;
1309+
}
1310+
1311+
return c;
1312+
}

‎src/core/dbus.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,5 @@ int bus_verify_reload_daemon_async(Manager *m, sd_bus_message *call, sd_bus_erro
4848
int bus_verify_set_environment_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
4949

5050
int bus_forward_agent_released(Manager *m, const char *path);
51+
52+
uint64_t manager_bus_n_queued_write(Manager *m);

‎src/core/manager.c‎

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,13 @@
102102
#define JOBS_IN_PROGRESS_PERIOD_USEC (USEC_PER_SEC / 3)
103103
#define JOBS_IN_PROGRESS_PERIOD_DIVISOR 3
104104

105+
/* If there are more than 1K bus messages queue across our API and direct busses, then let's not add more on top until
106+
* the queue gets more empty. */
107+
#define MANAGER_BUS_BUSY_THRESHOLD 1024LU
108+
109+
/* How many units and jobs to process of the bus queue before returning to the event loop. */
110+
#define MANAGER_BUS_MESSAGE_BUDGET 100U
111+
105112
static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata);
106113
static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata);
107114
static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata);
@@ -1886,41 +1893,65 @@ static int manager_dispatch_run_queue(sd_event_source *source, void *userdata) {
18861893
}
18871894

18881895
static unsigned manager_dispatch_dbus_queue(Manager *m) {
1889-
Job *j;
1896+
unsigned n = 0, budget;
18901897
Unit *u;
1891-
unsigned n = 0;
1898+
Job *j;
18921899

18931900
assert(m);
18941901

18951902
if (m->dispatching_dbus_queue)
18961903
return 0;
18971904

1905+
/* Anything to do at all? */
1906+
if (!m->dbus_unit_queue && !m->dbus_job_queue && !m->send_reloading_done && !m->queued_message)
1907+
return 0;
1908+
1909+
/* Do we have overly many messages queued at the moment? If so, let's not enqueue more on top, let's sit this
1910+
* cycle out, and process things in a later cycle when the queues got a bit emptier. */
1911+
if (manager_bus_n_queued_write(m) > MANAGER_BUS_BUSY_THRESHOLD)
1912+
return 0;
1913+
1914+
/* Only process a certain number of units/jobs per event loop iteration. Even if the bus queue wasn't overly
1915+
* full before this call we shouldn't increase it in size too wildly in one step, and we shouldn't monopolize
1916+
* CPU time with generating these messages. Note the difference in counting of this "budget" and the
1917+
* "threshold" above: the "budget" is decreased only once per generated message, regardless how many
1918+
* busses/direct connections it is enqueued on, while the "threshold" is applied to each queued instance of bus
1919+
* message, i.e. if the same message is enqueued to five busses/direct connections it will be counted five
1920+
* times. This difference in counting ("references" vs. "instances") is primarily a result of the fact that
1921+
* it's easier to implement it this way, however it also reflects the thinking that the "threshold" should put
1922+
* a limit on used queue memory, i.e. space, while the "budget" should put a limit on time. Also note that
1923+
* the "threshold" is currently chosen much higher than the "budget". */
1924+
budget = MANAGER_BUS_MESSAGE_BUDGET;
1925+
18981926
m->dispatching_dbus_queue = true;
18991927

1900-
while ((u = m->dbus_unit_queue)) {
1928+
while (budget > 0 && (u = m->dbus_unit_queue)) {
1929+
19011930
assert(u->in_dbus_queue);
19021931

19031932
bus_unit_send_change_signal(u);
1904-
n++;
1933+
n++, budget--;
19051934
}
19061935

1907-
while ((j = m->dbus_job_queue)) {
1936+
while (budget > 0 && (j = m->dbus_job_queue)) {
19081937
assert(j->in_dbus_queue);
19091938

19101939
bus_job_send_change_signal(j);
1911-
n++;
1940+
n++, budget--;
19121941
}
19131942

19141943
m->dispatching_dbus_queue = false;
19151944

1916-
if (m->send_reloading_done) {
1945+
if (budget > 0 && m->send_reloading_done) {
19171946
m->send_reloading_done = false;
1918-
19191947
bus_manager_send_reloading(m, false);
1948+
n++, budget--;
19201949
}
19211950

1922-
if (m->queued_message)
1951+
if (budget > 0 && m->queued_message) {
19231952
bus_send_queued_message(m);
1953+
n++;
1954+
}
19241955

19251956
return n;
19261957
}

‎src/libsystemd/libsystemd.sym‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,3 +549,9 @@ global:
549549
sd_event_source_get_io_fd_own;
550550
sd_event_source_set_io_fd_own;
551551
} LIBSYSTEMD_236;
552+
553+
LIBSYSTEMD_238 {
554+
global:
555+
sd_bus_get_n_queued_read;
556+
sd_bus_get_n_queued_write;
557+
} LIBSYSTEMD_237;

‎src/libsystemd/sd-bus/sd-bus.c‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4091,3 +4091,23 @@ _public_ int sd_bus_get_sender(sd_bus *bus, const char **ret) {
40914091
*ret = bus->patch_sender;
40924092
return 0;
40934093
}
4094+
4095+
_public_ int sd_bus_get_n_queued_read(sd_bus *bus, uint64_t *ret) {
4096+
assert_return(bus, -EINVAL);
4097+
assert_return(bus = bus_resolve(bus), -ENOPKG);
4098+
assert_return(!bus_pid_changed(bus), -ECHILD);
4099+
assert_return(ret, -EINVAL);
4100+
4101+
*ret = bus->rqueue_size;
4102+
return 0;
4103+
}
4104+
4105+
_public_ int sd_bus_get_n_queued_write(sd_bus *bus, uint64_t *ret) {
4106+
assert_return(bus, -EINVAL);
4107+
assert_return(bus = bus_resolve(bus), -ENOPKG);
4108+
assert_return(!bus_pid_changed(bus), -ECHILD);
4109+
assert_return(ret, -EINVAL);
4110+
4111+
*ret = bus->wqueue_size;
4112+
return 0;
4113+
}

‎src/systemd/sd-bus.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,9 @@ int sd_bus_attach_event(sd_bus *bus, sd_event *e, int priority);
202202
int sd_bus_detach_event(sd_bus *bus);
203203
sd_event *sd_bus_get_event(sd_bus *bus);
204204

205+
int sd_bus_get_n_queued_read(sd_bus *bus, uint64_t *ret);
206+
int sd_bus_get_n_queued_write(sd_bus *bus, uint64_t *ret);
207+
205208
int sd_bus_add_filter(sd_bus *bus, sd_bus_slot **slot, sd_bus_message_handler_t callback, void *userdata);
206209
int sd_bus_add_match(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, void *userdata);
207210
int sd_bus_add_match_async(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, sd_bus_message_handler_t install_callback, void *userdata);

0 commit comments

Comments
 (0)