starlingx/integ
Code Issues Proposed changes
872dd513fc
integ/base/systemd/centos/patches/907-sd-event-split-out-enable-and-disable-codepaths-from.patch
Li Zhou ccfeeef59d systemd: Prevent excessive /proc/1/mountinfo reparsing 
Backport the patches for this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=1819868

We met such an issue:
When testing a large number of pods (> 230), occasionally observed a
number of issues related to systemd process:
    systemd ran continually 90-100% cpu usage
    systemd memory usage started increasing rapidly (20GB/hour)
    systemctl commands would always timeout (Failed to get properties:
        Connection timed out)
    sm services failed and can't recover: open-ldap,
        registry-token-server, docker-distribution, etcd
    new pods can't start, and got stuck in state ContainerCreating

Those patches work to prevent excessive /proc/1/mountinfo reparsing.
It has been verified that those patches can improve this performance
greatly.

16 commits are listed in sequence (from [1] to [16]) at below link
for the issue:
https://github.com/systemd-rhel/rhel-8/pull/154/commits

[16](10)core: prevent excessive /proc/self/mountinfo parsing
[15][Dropped-6]test: add ratelimiting test
[14](9)sd-event: add ability to ratelimit event sources
[13](8)sd-event: increase n_enabled_child_sources just once
[12](7)sd-event: update state at the end in event_source_enable
[11](6)sd-event: remove earliest_index/latest_index into common part of
event source objects
[10][Dropped-5]sd-event: follow coding style with naming return
parameter
[9] [Dropped-4]sd-event: ref event loop while in sd_event_prepare() ot
sd_event_run()
[8] (5)sd-event: refuse running default event loops in any other thread
than the one they are default for
[7] [Dropped-3]sd-event: let's suffix last_run/last_log with "_usec"
[6] [Dropped-2]sd-event: fix delays assert brain-o (#17790)
[5] (4)sd-event: split out code to add/remove timer event sources to
earliest/latest prioq
[4] (3)sd-event: split clock data allocation out of sd_event_add_time()
[3] [Dropped-1]sd-event: mention that two debug logged events are
ignored
[2] (2)sd-event: split out enable and disable codepaths from
sd_event_source_set_enabled()
[1] (1)sd-event: split out helper functions for reshuffling prioqs

I ported 10 of them back (from (1) to (10)) to fix this issue
and dropped the other 6 (from [Dropped-1] to [Dropped-6]) for those
reasons:
[Dropped-1]Only changes error log.
[Dropped-2]Fixes a bug introduced in a commit which doesn't exist in
this version.
[Dropped-3]Only changes vars' names and there is no functional change.
[Dropped-4]More commits are needed for merging it, while I don't see
any help on adding the rate-limiting ability.
[Dropped-5]Change coding style for a function which isn't really used
by anyone.
[Dropped-6]Add test cases.

Closes-Bug: #1924686
Signed-off-by: Li Zhou <li.zhou@windriver.com>
Change-Id: Ia4c8f162cb1a47b40d1b26cf4d604976b97e92d6
2021-04-22 22:09:33 -04:00

316 lines
11 KiB
Diff
Raw Blame History

From 477bbfd4f5012613144c5ba5517aa8de1f300da6 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 23 Oct 2020 21:21:58 +0200
Subject: [PATCH 07/20] sd-event: split out enable and disable codepaths from
sd_event_source_set_enabled()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
So far half of sd_event_source_set_enabled() was doing enabling, the
other half was doing disabling. Let's split that into two separate
calls.
(This also adds a new shortcut to sd_event_source_set_enabled(): if the
caller toggles between "ON" and "ONESHOT" we'll now shortcut this, since
the event source is already enabled in that case and shall remain
enabled.)
This heavily borrows and is inspired from Michal Sekletár's #17284
refactoring.
(cherry picked from commit ddfde737b546c17e54182028153aa7f7e78804e3)
Related: #1819868
[commit d7ad6ad123200f562081ff09f7bed3c6d969ac0a from
https://github.com/systemd-rhel/rhel-8/
LZ: Dropped SOURCE_INOTIFY related parts because it hasn't been added
in this systemd version.]
Signed-off-by: Li Zhou <li.zhou@windriver.com>
---
src/libsystemd/sd-event/sd-event.c | 224 +++++++++++++++--------------
1 file changed, 118 insertions(+), 106 deletions(-)
diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index 2f5ff23..2e07478 100644
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -1606,153 +1606,165 @@ _public_ int sd_event_source_get_enabled(sd_event_source *s, int *m) {
return 0;
}
-_public_ int sd_event_source_set_enabled(sd_event_source *s, int m) {
+static int event_source_disable(sd_event_source *s) {
int r;
- assert_return(s, -EINVAL);
- assert_return(m == SD_EVENT_OFF || m == SD_EVENT_ON || m == SD_EVENT_ONESHOT, -EINVAL);
- assert_return(!event_pid_changed(s->event), -ECHILD);
+ assert(s);
+ assert(s->enabled != SD_EVENT_OFF);
- /* If we are dead anyway, we are fine with turning off
- * sources, but everything else needs to fail. */
- if (s->event->state == SD_EVENT_FINISHED)
- return m == SD_EVENT_OFF ? 0 : -ESTALE;
+ /* Unset the pending flag when this event source is disabled */
+ if (!IN_SET(s->type, SOURCE_DEFER, SOURCE_EXIT)) {
+ r = source_set_pending(s, false);
+ if (r < 0)
+ return r;
+ }
- if (s->enabled == m)
- return 0;
+ s->enabled = SD_EVENT_OFF;
- if (m == SD_EVENT_OFF) {
+ switch (s->type) {
- /* Unset the pending flag when this event source is disabled */
- if (!IN_SET(s->type, SOURCE_DEFER, SOURCE_EXIT)) {
- r = source_set_pending(s, false);
- if (r < 0)
- return r;
- }
+ case SOURCE_IO:
+ source_io_unregister(s);
+ break;
- switch (s->type) {
+ case SOURCE_TIME_REALTIME:
+ case SOURCE_TIME_BOOTTIME:
+ case SOURCE_TIME_MONOTONIC:
+ case SOURCE_TIME_REALTIME_ALARM:
+ case SOURCE_TIME_BOOTTIME_ALARM:
+ event_source_time_prioq_reshuffle(s);
+ break;
- case SOURCE_IO:
- r = source_io_unregister(s);
- if (r < 0)
- return r;
+ case SOURCE_SIGNAL:
+ event_gc_signal_data(s->event, &s->priority, s->signal.sig);
+ break;
- s->enabled = m;
- break;
+ case SOURCE_CHILD:
+ assert(s->event->n_enabled_child_sources > 0);
+ s->event->n_enabled_child_sources--;
- case SOURCE_TIME_REALTIME:
- case SOURCE_TIME_BOOTTIME:
- case SOURCE_TIME_MONOTONIC:
- case SOURCE_TIME_REALTIME_ALARM:
- case SOURCE_TIME_BOOTTIME_ALARM:
- s->enabled = m;
- event_source_time_prioq_reshuffle(s);
- break;
+ event_gc_signal_data(s->event, &s->priority, SIGCHLD);
+ break;
- case SOURCE_SIGNAL:
- s->enabled = m;
+ case SOURCE_EXIT:
+ prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
+ break;
- event_gc_signal_data(s->event, &s->priority, s->signal.sig);
- break;
+ case SOURCE_DEFER:
+ case SOURCE_POST:
+ break;
- case SOURCE_CHILD:
- s->enabled = m;
+ default:
+ assert_not_reached("Wut? I shouldn't exist.");
+ }
- assert(s->event->n_enabled_child_sources > 0);
- s->event->n_enabled_child_sources--;
+ return 0;
+}
- event_gc_signal_data(s->event, &s->priority, SIGCHLD);
- break;
+static int event_source_enable(sd_event_source *s, int m) {
+ int r;
- case SOURCE_EXIT:
- s->enabled = m;
- prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
- break;
+ assert(s);
+ assert(IN_SET(m, SD_EVENT_ON, SD_EVENT_ONESHOT));
+ assert(s->enabled == SD_EVENT_OFF);
- case SOURCE_DEFER:
- case SOURCE_POST:
- s->enabled = m;
- break;
+ /* Unset the pending flag when this event source is enabled */
+ if (!IN_SET(s->type, SOURCE_DEFER, SOURCE_EXIT)) {
+ r = source_set_pending(s, false);
+ if (r < 0)
+ return r;
+ }
- default:
- assert_not_reached("Wut? I shouldn't exist.");
- }
+ s->enabled = m;
- } else {
+ switch (s->type) {
- /* Unset the pending flag when this event source is enabled */
- if (s->enabled == SD_EVENT_OFF && !IN_SET(s->type, SOURCE_DEFER, SOURCE_EXIT)) {
- r = source_set_pending(s, false);
- if (r < 0)
- return r;
+ case SOURCE_IO:
+ r = source_io_register(s, m, s->io.events);
+ if (r < 0) {
+ s->enabled = SD_EVENT_OFF;
+ return r;
}
- switch (s->type) {
+ break;
- case SOURCE_IO:
- r = source_io_register(s, m, s->io.events);
- if (r < 0)
- return r;
+ case SOURCE_TIME_REALTIME:
+ case SOURCE_TIME_BOOTTIME:
+ case SOURCE_TIME_MONOTONIC:
+ case SOURCE_TIME_REALTIME_ALARM:
+ case SOURCE_TIME_BOOTTIME_ALARM:
+ event_source_time_prioq_reshuffle(s);
+ break;
- s->enabled = m;
- break;
+ case SOURCE_SIGNAL:
+ r = event_make_signal_data(s->event, s->signal.sig, NULL);
+ if (r < 0) {
+ s->enabled = SD_EVENT_OFF;
+ event_gc_signal_data(s->event, &s->priority, s->signal.sig);
+ return r;
+ }
- case SOURCE_TIME_REALTIME:
- case SOURCE_TIME_BOOTTIME:
- case SOURCE_TIME_MONOTONIC:
- case SOURCE_TIME_REALTIME_ALARM:
- case SOURCE_TIME_BOOTTIME_ALARM:
- s->enabled = m;
- event_source_time_prioq_reshuffle(s);
- break;
+ break;
- case SOURCE_SIGNAL:
+ case SOURCE_CHILD:
+ s->event->n_enabled_child_sources++;
- s->enabled = m;
+ r = event_make_signal_data(s->event, SIGCHLD, NULL);
+ if (r < 0) {
+ s->enabled = SD_EVENT_OFF;
+ s->event->n_enabled_child_sources--;
+ event_gc_signal_data(s->event, &s->priority, SIGCHLD);
+ return r;
+ }
- r = event_make_signal_data(s->event, s->signal.sig, NULL);
- if (r < 0) {
- s->enabled = SD_EVENT_OFF;
- event_gc_signal_data(s->event, &s->priority, s->signal.sig);
- return r;
- }
- break;
+ break;
- case SOURCE_CHILD:
+ case SOURCE_EXIT:
+ prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
+ break;
- if (s->enabled == SD_EVENT_OFF)
- s->event->n_enabled_child_sources++;
+ case SOURCE_DEFER:
+ case SOURCE_POST:
+ break;
- s->enabled = m;
+ default:
+ assert_not_reached("Wut? I shouldn't exist.");
+ }
- r = event_make_signal_data(s->event, SIGCHLD, NULL);
- if (r < 0) {
- s->enabled = SD_EVENT_OFF;
- s->event->n_enabled_child_sources--;
- event_gc_signal_data(s->event, &s->priority, SIGCHLD);
- return r;
- }
+ return 0;
+}
- break;
+_public_ int sd_event_source_set_enabled(sd_event_source *s, int m) {
+ int r;
- case SOURCE_EXIT:
- s->enabled = m;
- prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
- break;
+ assert_return(s, -EINVAL);
+ assert_return(IN_SET(m, SD_EVENT_OFF, SD_EVENT_ON, SD_EVENT_ONESHOT), -EINVAL);
+ assert_return(!event_pid_changed(s->event), -ECHILD);
- case SOURCE_DEFER:
- case SOURCE_POST:
- s->enabled = m;
- break;
+ /* If we are dead anyway, we are fine with turning off sources, but everything else needs to fail. */
+ if (s->event->state == SD_EVENT_FINISHED)
+ return m == SD_EVENT_OFF ? 0 : -ESTALE;
- default:
- assert_not_reached("Wut? I shouldn't exist.");
+ if (s->enabled == m) /* No change? */
+ return 0;
+
+ if (m == SD_EVENT_OFF)
+ r = event_source_disable(s);
+ else {
+ if (s->enabled != SD_EVENT_OFF) {
+ /* Switching from "on" to "oneshot" or back? If that's the case, we can take a shortcut, the
+ * event source is already enabled after all. */
+ s->enabled = m;
+ return 0;
}
+
+ r = event_source_enable(s, m);
}
+ if (r < 0)
+ return r;
event_source_pp_prioq_reshuffle(s);
-
return 0;
}
--
2.17.1
View Git Blame Copy Permalink