Re: [systemd-devel] Newbie question - Requires doesn't work properly
Am 22.11.2013 03:04, schrieb salil GK: Thanks a lot David On 22 November 2013 06:44, David Timothy Strauss da...@davidstrauss.net mailto:da...@davidstrauss.net wrote: On Thu, Nov 21, 2013 at 4:57 PM, salil GK gksa...@gmail.com mailto:gksa...@gmail.com wrote: What happens is - my process may be busy with some other activity during which time it will fail to send periodic message to systemd. After a while it will come out of it's loop and ready to serve. But during this time system would have already marked the process as failed. Then you need to either use another thread, refactor to make a tighter event loop, or increase the watchdog time. Drifting in and out of tolerance with watchdog is not a safe strategy. the problem i see with use another thread is that this thread can happily work and send it's keep alive, but that does not mean at the end that the service itself is working OK and responsible because both are running isolated in case of network services it would be pretty cool if systemd watchdog could be configured to connect to the service avery n seconds and if there is no response restart it because this would monitor the real service without need external tools signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Newbie question - Requires doesn't work properly
It is the responsibility of whatever sends the watchdog to ensure everything's healthy, however necessary. It would be silly to spawn a thread and have it blindly report health to watchdog. The point is for that thread to do proper checks but ensure reports go in at the right intervals. On Nov 22, 2013 7:50 PM, Reindl Harald h.rei...@thelounge.net wrote: Am 22.11.2013 03:04, schrieb salil GK: Thanks a lot David On 22 November 2013 06:44, David Timothy Strauss da...@davidstrauss.netmailto: da...@davidstrauss.net wrote: On Thu, Nov 21, 2013 at 4:57 PM, salil GK gksa...@gmail.commailto: gksa...@gmail.com wrote: What happens is - my process may be busy with some other activity during which time it will fail to send periodic message to systemd. After a while it will come out of it's loop and ready to serve. But during this time system would have already marked the process as failed. Then you need to either use another thread, refactor to make a tighter event loop, or increase the watchdog time. Drifting in and out of tolerance with watchdog is not a safe strategy. the problem i see with use another thread is that this thread can happily work and send it's keep alive, but that does not mean at the end that the service itself is working OK and responsible because both are running isolated in case of network services it would be pretty cool if systemd watchdog could be configured to connect to the service avery n seconds and if there is no response restart it because this would monitor the real service without need external tools ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/4] [RFC v1] gdbus: Preliminary kdbus-support patches
On 11/21/2013 08:28 PM, Colin Walters wrote: On Thu, 2013-11-21 at 16:35 +0100, Karol Lewandowski wrote: Truth is that gio guys already merged tizen's glib-kdbus modifications into their own devel branch without us even knowing, not to mention proposing it. If you're referring to https://git.gnome.org/browse/glib/log/?h=tizen/kdbus-dev I think Ryan just wanted to put the patches in a place where he could (more easily) see them and work on them. What I would like to avoid is us and Ryan (and other guys) working on the same piece of code, effectively forking glib multiple times for kdbus. We are quite open about our work - it just took us a while to bring changes to the state where these are more-or-less ready for public consumption. :) The devel branch of glib is master, and anything landing there needs to go through peer review. wip/ and other branches are more freeform. Thanks for this explanation, I wasn't aware of all these details. Cheers, Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Newbie question - Requires doesn't work properly
Am 22.11.2013 12:49, schrieb David Timothy Strauss: It is the responsibility of whatever sends the watchdog to ensure everything's healthy, however necessary. It would be silly to spawn a thread and have it blindly report health to watchdog. The point is for that thread to do proper checks but ensure reports go in at the right intervals. i know that but the *how* is the question you can internally check what not but that does not mean at the end of the day the service responds correctly to a client connection over the network until you do not go through the same stack meaning doing a network connection i spent hundrets of hours in upstream-debugging of dbmail to find spinlocks and what not else only happening in rare situations, one of them took 16 hours stress tests until it happend with debug-log enabled while on the real server it took a few minutes to get triggered by a random client action that's the difference between theory and real workload your internal checks are mostly theory because in case of a bug you have undefined behavior and what you want to achieve with the watchdog is catch this undefined behavior and restart the service - in doubt this will not work in the rare cases the watchdog should restart until you went the complete code-path of a client, in case of a IMAP server you can enter the spin-loop everywhere from accept the connection to folder listing or receive a message and it may depend on a buffer overflow while high concurrency and different threads are touching each other in a unexpected way been there, died nearly in debug it and catch data for upstream On Nov 22, 2013 7:50 PM, Reindl Harald h.rei...@thelounge.net mailto:h.rei...@thelounge.net wrote: Am 22.11.2013 03:04, schrieb salil GK: Thanks a lot David On 22 November 2013 06:44, David Timothy Strauss da...@davidstrauss.net mailto:da...@davidstrauss.net mailto:da...@davidstrauss.net mailto:da...@davidstrauss.net wrote: On Thu, Nov 21, 2013 at 4:57 PM, salil GK gksa...@gmail.com mailto:gksa...@gmail.com mailto:gksa...@gmail.com mailto:gksa...@gmail.com wrote: What happens is - my process may be busy with some other activity during which time it will fail to send periodic message to systemd. After a while it will come out of it's loop and ready to serve. But during this time system would have already marked the process as failed. Then you need to either use another thread, refactor to make a tighter event loop, or increase the watchdog time. Drifting in and out of tolerance with watchdog is not a safe strategy. the problem i see with use another thread is that this thread can happily work and send it's keep alive, but that does not mean at the end that the service itself is working OK and responsible because both are running isolated in case of network services it would be pretty cool if systemd watchdog could be configured to connect to the service avery n seconds and if there is no response restart it because this would monitor the real service without need external tools signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/4] [RFC v1] gdbus: Preliminary kdbus-support patches
On 11/22/2013 01:32 PM, Matthias Clasen wrote: On Fri, Nov 22, 2013 at 7:03 AM, Karol Lewandowski k.lewando...@samsung.com mailto:k.lewando...@samsung.com wrote: On 11/21/2013 08:28 PM, Colin Walters wrote: On Thu, 2013-11-21 at 16:35 +0100, Karol Lewandowski wrote: Truth is that gio guys already merged tizen's glib-kdbus modifications into their own devel branch without us even knowing, not to mention proposing it. If you're referring to https://git.gnome.org/browse/glib/log/?h=tizen/kdbus-dev I think Ryan just wanted to put the patches in a place where he could (more easily) see them and work on them. What I would like to avoid is us and Ryan (and other guys) working on the same piece of code, effectively forking glib multiple times for kdbus. We are quite open about our work - it just took us a while to bring changes to the state where these are more-or-less ready for public consumption. :) I pointed Ryan at the branch when I learned about it, and he just imported it to have a first look at it. I don't think there was any intention to 'work' on this code right away. Sorry if this startled you. No problem here - we are quite happy that upstream is interested in this. And thanks for pushing out this cleaned up version, I appreciate it! Thanks for bringing this into the light and, consequently - motivating us to clean things up. Cheers, Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] include: fix problem with __LINE__ macro expansion
On Thu, 21.11.13 17:26, Lukasz Skalski (l.skal...@partner.samsung.com) wrote: Hi all, Macro __LINE__ in #define assert_cc() (src/shared/macro.h) is not properly expanded. Please find patch in attachment for review. Thanks! Seems David Herrmann has now fixed this thing slightly differently. Thank you, Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [HEADS-UP] PID 1 now ported to libsystemd-bus
On Thu, 21.11.13 10:03, Oleksii Shevchuk (alx...@gmail.com) wrote: Segfaults fixed, but: systemd-logind[2167]: Failed to start session scope session-3.scope: Invalid arguments 'ssa(sv)' to call org.freedesktop.systemd1.Manager:StartTransientUnit, expecting 'ssa(sv)a(sa(sv))'. org.freedesktop.DBus.Error.InvalidArgs lightdm[2594]: pam_systemd(lightdm:session): Failed to create session: Invalid argument lightdm[2168]: ** (process:2594): WARNING **: Failed to open login1 session: GDBus.Error:org.freedesktop.login1.NoSessionForPID: PID 2594 does not belong to any known session That should be fixed now. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] cgroups: Cache controller masks and optimize queues.
On Tue, 19.11.13 17:12, David Timothy Strauss (da...@davidstrauss.net) wrote: I'm pretty confident in the accuracy of the controller mask aggregation, especially given the new unit test. Here are the main review questions. Ahum. Please do not just commit stuff like this that is not obvious and touches the core without review. make distcheck is broken now. I am fine if commiters commit without review if it's in their own submodule, or if it's man pages, or really obvious things. But this commit does not qualify. It touches the core, and it is far from obvious to me. It includes lines like TODO which are a good indication that this isn't even thought out to the end... Please, for stuff like this get a review from Kay, Zbigniew, Michal Schmidt, or me, before you commit. Thanks! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] include: fix problem with __LINE__ macro expansion
Hi On Fri, Nov 22, 2013 at 2:09 PM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 21.11.13 17:26, Lukasz Skalski (l.skal...@partner.samsung.com) wrote: Hi all, Macro __LINE__ in #define assert_cc() (src/shared/macro.h) is not properly expanded. Please find patch in attachment for review. Thanks! Seems David Herrmann has now fixed this thing slightly differently. I just noticed the compile-failure and fixed it up myself. But this patch definitely looks nicer. I now changed the helper-name to XCONCATENATE similar to XSTRINGIFY and added a UNIQUE macro to generate a unique name. Might be useful in other macros, too. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Newbie question - Requires doesn't work properly
On Fri, Nov 22, 2013 at 10:24 PM, Reindl Harald h.rei...@thelounge.net wrote: your internal checks are mostly theory because in case of a bug you have undefined behavior and what you want to achieve with the watchdog is catch this undefined behavior and restart the service - in doubt this will not work in the rare cases the watchdog should restart until you went the complete code-path of a client, in case of a IMAP server you can enter the spin-loop everywhere from accept the connection to folder listing or receive a message and it may depend on a buffer overflow while high concurrency and different threads are touching each other in a unexpected way You're not hearing what I'm saying. Check what's relevant; it's not systemd's concern what you check. I you need to do a transactional check of an IMAP server from a separate process or thread, then do that. Only report back to watchdog if you've verified what you consider to be a healthy state. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] include: fix problem with __LINE__ macro expansion
On 11/22/2013 02:46 PM, David Herrmann wrote: Hi Hi, On Fri, Nov 22, 2013 at 2:09 PM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 21.11.13 17:26, Lukasz Skalski (l.skal...@partner.samsung.com) wrote: Hi all, Macro __LINE__ in #define assert_cc() (src/shared/macro.h) is not properly expanded. Please find patch in attachment for review. Thanks! Seems David Herrmann has now fixed this thing slightly differently. I just noticed the compile-failure and fixed it up myself. But this patch definitely looks nicer. I now changed the helper-name to XCONCATENATE similar to XSTRINGIFY and added a UNIQUE macro to generate a unique name. Might be useful in other macros, too. Thanks David Thanks. Solution with XCONCATENATE seems to be more generic than assert___cc(expr,line) macro and can be re-used. BR, -- Lukasz Skalski Samsung RD Institute Poland Samsung Electronics l.skal...@partner.samsung.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
On Wed, 2013-11-20 at 19:19 -0500, Colin Walters wrote: I care about pkexec. Note we're now carrying a workaround for this: https://bugs.freedesktop.org/show_bug.cgi?id=71894 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] systemctl: add systemctl cat
---
TODO | 2 --
src/shared/fileio.c | 73 -
src/shared/fileio.h | 1 +
src/systemctl/systemctl.c | 91 +++
4 files changed, 149 insertions(+), 18 deletions(-)
diff --git a/TODO b/TODO
index 6ba4b31..7c6003b 100644
--- a/TODO
+++ b/TODO
@@ -125,8 +125,6 @@ Features:
* optimize the cgroup propagation bits, especially unit_get_members_mask(),
cgroup_context_get_mask()
-* systemctl cat or systemctl view command or or so, that cats the backing
unit file of a service, plus its drop-ins and shows them in a pager
-
* rfkill,backlight: we probably should run the load tools inside of the udev
rules so that the state is properly initialized by the time other software sees
it
* tmpfiles: when applying ownership to /run/log/journal, also do this for the
journal fails contained in it
diff --git a/src/shared/fileio.c b/src/shared/fileio.c
index 733b320..ac1b409 100644
--- a/src/shared/fileio.c
+++ b/src/shared/fileio.c
@@ -20,6 +20,7 @@
***/
#include unistd.h
+#include sys/sendfile.h
#include fileio.h
#include util.h
#include strv.h
@@ -117,6 +118,77 @@ int read_one_line_file(const char *fn, char **line) {
return 0;
}
+ssize_t sendfile_full(int out_fd, const char *fn) {
+_cleanup_fclose_ FILE *f;
+struct stat st;
+int r;
+ssize_t s;
+
+size_t n, l;
+_cleanup_free_ char *buf = NULL;
+
+assert(out_fd 0);
+assert(fn);
+
+f = fopen(fn, r);
+if (!f)
+return -errno;
+
+r = fstat(fileno(f), st);
+if (r 0)
+return -errno;
+
+s = sendfile(out_fd, fileno(f), NULL, st.st_size);
+if (s 0)
+if (errno == EINVAL || errno == ENOSYS) {
+/* continue below */
+} else
+return -errno;
+else
+return s;
+
+/* sendfile() failed, fall back to read/write */
+
+/* Safety check */
+if (st.st_size 4*1024*1024)
+return -E2BIG;
+
+n = st.st_size 0 ? st.st_size : LINE_MAX;
+l = 0;
+
+while (true) {
+char *t;
+size_t k;
+
+t = realloc(buf, n);
+if (!t)
+return -ENOMEM;
+
+buf = t;
+k = fread(buf + l, 1, n - l, f);
+
+if (k = 0) {
+if (ferror(f))
+return -errno;
+
+break;
+}
+
+l += k;
+n *= 2;
+
+/* Safety check */
+if (n 4*1024*1024)
+return -E2BIG;
+}
+
+r = write(out_fd, buf, l);
+if (r 0)
+return -errno;
+
+return (ssize_t) l;
+}
+
int read_full_file(const char *fn, char **contents, size_t *size) {
_cleanup_fclose_ FILE *f = NULL;
size_t n, l;
@@ -168,7 +240,6 @@ int read_full_file(const char *fn, char **contents, size_t
*size) {
buf[l] = 0;
*contents = buf;
-buf = NULL;
if (size)
*size = l;
diff --git a/src/shared/fileio.h b/src/shared/fileio.h
index 59e4150..06c2887 100644
--- a/src/shared/fileio.h
+++ b/src/shared/fileio.h
@@ -31,6 +31,7 @@ int write_string_file_atomic(const char *fn, const char
*line);
int read_one_line_file(const char *fn, char **line);
int read_full_file(const char *fn, char **contents, size_t *size);
+ssize_t sendfile_full(int out_fd, const char *fn);
int parse_env_file(const char *fname, const char *separator, ...) _sentinel_;
int load_env_file(const char *fname, const char *separator, char ***l);
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 576396f..18d5e45 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -3665,16 +3665,25 @@ static int show_all(
return 0;
}
-static int get_unit_from_arg(sd_bus *bus, char *arg, char **unitp, bool
interpret_as_job_id) {
-uint32_t id;
+static int cat(sd_bus *bus, char **args) {
int r = 0;
+char **name;
-if (safe_atou32(arg, id) 0) {
-_cleanup_free_ char *n = NULL;
-char *unit;
-/* Interpret as unit name */
+_cleanup_free_ char *unit = NULL, *n = NULL;
+
+assert(bus);
+assert(args);
-n = unit_name_mangle(arg);
+pager_open_if_enabled();
+
+STRV_FOREACH(name, args+1) {
+_cleanup_free_ char *fragment_path = NULL;
+_cleanup_strv_free_ char **dropin_paths = NULL;
+sd_bus_error error;
+FILE *stdout;
+char **path;
+
+n = unit_name_mangle(*name);
if (!n)
[systemd-devel] [PATCH 1/2] systemctl: refactor show()
---
src/systemctl/systemctl.c | 66 ++-
1 file changed, 43 insertions(+), 23 deletions(-)
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 84826a3..576396f 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -3594,16 +3594,13 @@ static int show_one(
return r;
}
-static int show_one_by_pid(
-const char *verb,
+static int get_unit_dbus_path_by_pid(
sd_bus *bus,
uint32_t pid,
-bool *new_line,
-bool *ellipsized) {
+char **unit) {
_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
_cleanup_bus_message_unref_ sd_bus_message *reply = NULL;
-const char *path = NULL;
int r;
r = sd_bus_call_method(
@@ -3620,11 +3617,11 @@ static int show_one_by_pid(
return r;
}
-r = sd_bus_message_read(reply, o, path);
+r = sd_bus_message_read(reply, o, unit);
if (r 0)
return bus_log_parse_error(r);
-return show_one(verb, bus, path, false, new_line, ellipsized);
+return 0;
}
static int show_all(
@@ -3668,6 +3665,36 @@ static int show_all(
return 0;
}
+static int get_unit_from_arg(sd_bus *bus, char *arg, char **unitp, bool
interpret_as_job_id) {
+uint32_t id;
+int r = 0;
+
+if (safe_atou32(arg, id) 0) {
+_cleanup_free_ char *n = NULL;
+char *unit;
+/* Interpret as unit name */
+
+n = unit_name_mangle(arg);
+if (!n)
+return log_oom();
+
+unit = unit_dbus_path_from_name(n);
+if (!unit)
+return log_oom();
+
+*unitp = unit;
+} else if (interpret_as_job_id) {
+/* Interpret as job id */
+if (asprintf(unitp, /org/freedesktop/systemd1/job/%u, id)
0)
+return log_oom();
+} else {
+/* Interpret as PID */
+r = get_unit_by_pid(bus, id, unitp);
+}
+
+return r;
+}
+
static int show(sd_bus *bus, char **args) {
int r, ret = 0;
bool show_properties, show_status, new_line = false;
@@ -3692,41 +3719,34 @@ static int show(sd_bus *bus, char **args) {
ret = show_all(args[0], bus, false, new_line, ellipsized);
else
STRV_FOREACH(name, args+1) {
+_cleanup_free_ char *unit = NULL;
uint32_t id;
if (safe_atou32(*name, id) 0) {
-_cleanup_free_ char *p = NULL, *n = NULL;
+_cleanup_free_ char *n = NULL;
/* Interpret as unit name */
n = unit_name_mangle(*name);
if (!n)
return log_oom();
-p = unit_dbus_path_from_name(n);
-if (!p)
+unit = unit_dbus_path_from_name(n);
+if (!unit)
return log_oom();
-r = show_one(args[0], bus, p, show_properties,
new_line, ellipsized);
-if (r != 0)
-ret = r;
-
} else if (show_properties) {
-_cleanup_free_ char *p = NULL;
-
/* Interpret as job id */
-if (asprintf(p,
/org/freedesktop/systemd1/job/%u, id) 0)
+if (asprintf(unit,
/org/freedesktop/systemd1/job/%u, id) 0)
return log_oom();
-r = show_one(args[0], bus, p, show_properties,
new_line, ellipsized);
-if (r != 0)
-ret = r;
-
} else {
/* Interpret as PID */
-r = show_one_by_pid(args[0], bus, id,
new_line, ellipsized);
-if (r != 0)
+r = get_unit_dbus_path_by_pid(bus, id, unit);
+if (r 0)
ret = r;
}
+
+show_one(args[0], bus, unit, show_properties,
new_line, ellipsized);
}
if (ellipsized !arg_quiet)
--
1.8.4.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
'Twas brillig, and Colin Walters at 22/11/13 17:05 did gyre and gimble: On Wed, 2013-11-20 at 19:19 -0500, Colin Walters wrote: I care about pkexec. Note we're now carrying a workaround for this: https://bugs.freedesktop.org/show_bug.cgi?id=71894 Thanks for that Colin, I'll pick up the patch in our version. However, I really do hope Lennart will accept that there is a problem in pam_systemd too and commit some fixes to stop setting values that can only cause problems. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] cgroups: Cache controller masks and optimize queues.
I'm disappointed in your response here. It's not up to your normally high standards of technical discourse and tact on this mailing list. I realize I've stepped on some feet, but there's quite a bit more to this than me getting trigger-happy with a core commit. On Fri, Nov 22, 2013 at 11:43 PM, Lennart Poettering lenn...@poettering.net wrote: make distcheck is broken now. This is clearly not the key issue here, given the following: (a) make distcheck was already failing in the test-unit-file suite before my commit (and continues to be failing in master). So, it was already broken. (b) My oversight of excluding the Makefile.am lines to bundle the test units the proper way for make distcheck was pretty minor in its effect. The test still passed make check, and I didn't block general development or use of the project. (c) In the last couple weeks, you've broken builds yourself -- not just tests -- for 12 hours [1][2]. We all make mistakes like that, but I feel like you're using mine as a way to pile on criticism here. There's a reason I emailed you privately when you broke the build that day. In any case, I fixed make distcheck [3] shortly after hearing about the issue. I'll be sure to verify lack of any regression in make distcheck before pushing in the future. I'll also add it to CI once test-unit-file is working; I don't want to break CI builds in a misleading way. I am fine if commiters commit without review if it's in their own submodule, or if it's man pages, or really obvious things. But this commit does not qualify. It touches the core, and it is far from obvious to me. First, I'm not intentionally trying to skip process. At the same time, please understand my frustration with a performance issue introduced in v205 that added hours or days to the boot times of our previously working servers. My attempts to discuss options and approaches with you at LinuxCon, on the mailing list, and IRC have only been marginally productive. Before my commit, we only had a TODO logged. Requesting reviews (by you and others) of proposed changes didn't get much response. Zbigniew was the only person I was able to flag down in any way, but he also said he has minimal familiarity with the current cgroups implementations. In an effort to improve the transparency of my changes and reduce the chance of introducing instability, I coupled my changes with improved functional testing in the key area I optimized. I then posted that patch to the mailing list including some specific (but fairly edge-case) follow-up questions, let it sit several days, and heard back nothing. I hadn't even gotten a reply suggesting that a review would be forthcoming. It includes lines like TODO which are a good indication that this isn't even thought out to the end... I also think it's unfair to characterize my work here as not thought out. (a) My changes went through testing by my ops crew to verify that scalability issues were fixed for both stopped and starting units and that cgroups still received proper configuration values and controllers, including for slices. (b) I bundled a test suite covering functionality that lacked coverage before. (c) The TODO relates to a tiny optimization. The potential improvement is so minor that the optimization is more an issue of removing redundancy for tidiness. I used a similar approach in gutting (but leaving in place for now) functions that probably aren't necessary anymore given how they just return struct values. (d) The nature of the TODO isn't massively different from other items in the TODO file. It just happens to be proximal to the related code. (e) My implementation replaced one running O(n^3) on startup for enabled units, slightly better for disabled ones. There's no way the previous implementation qualified as long-term and thought-out in the first place. We're still waiting for some of those machines we started nearly a week ago to finish applying initial cgroups configuration and allow login. :-/ Please, for stuff like this get a review from Kay, Zbigniew, Michal Schmidt, or me, before you commit. Thanks! Indeed, I'd prefer for that to be the case. Let's talk about ways contributors can have more confidence going forward that fixes to regressions like this will get timely reviews. I know you're really busy with the D-Bus and network work, but maintaining a stable, usable master is increasingly important as this project matures. [1] http://cgit.freedesktop.org/systemd/systemd/commit/?id=2b5c5383e48137d748681645ad7176f02b50ba30 [2] http://cgit.freedesktop.org/systemd/systemd/commit/?id=3db0e46b0de5e46922ebbbd77de4d3d1214bcc9a [3] http://cgit.freedesktop.org/systemd/systemd/commit/?id=21acf11d407ea93d1b0c99088f9f64adad6cff0e ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] cgroups: Cache controller masks and optimize queues.
On Sat, Nov 23, 2013 at 7:53 AM, David Timothy Strauss da...@davidstrauss.net wrote: My attempts to discuss options and approaches with you at LinuxCon, on the mailing list, and IRC have only been marginally productive. Before my commit, we only had a TODO logged. Actually, I'm being a bit unfair here. Most of what we discussed at LinuxCon was the daemon-reload issue, which you addressed really nicely soon afterward. I think I only became aware of the cgroups issue after we started to test the path prefix fix at scale. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] send -isolateDevice and -config option to Xserver
Dear systemd Maintainers, I am still struggling to get multiseat working with systemd 204, gdm3.10 and gnome3.10. Last week Debian Testing has released Gnome 3.8 but unfortunately gdm3 is was build without systemd support, so I tried the experimental 3.10 version with systemd. Unfortunately I can't figure out how to pass options to the Xserver, because the nvidia driver needs an -isolateDevice and/ or a -config option to get a working multiseat. I have set the master-of-seat rule for udev and loginctl seat-status seat0 and seat1 shows the right devices for each seat. But without the -isolateDevice option the first seat Xserver grabs the two Nvidia cards leaving the second with an Error no screens found message. I hope someone can explain me how the two Xservers are started. I think that /lib/systemd/systemd-multi-seat-x start the second Xserver with a gdm3 for seat1 But does it also start seat0? And how can I send the -isolateDevice option to the Xservers? thanks, floris ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [HEADS-UP] PID 1 now ported to libsystemd-bus
On Fri, Nov 22, 2013 at 2:33 PM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 21.11.13 10:03, Oleksii Shevchuk (alx...@gmail.com) wrote: Segfaults fixed, but: systemd-logind[2167]: Failed to start session scope session-3.scope: Invalid arguments 'ssa(sv)' to call org.freedesktop.systemd1.Manager:StartTransientUnit, expecting 'ssa(sv)a(sa(sv))'. org.freedesktop.DBus.Error.InvalidArgs lightdm[2594]: pam_systemd(lightdm:session): Failed to create session: Invalid argument lightdm[2168]: ** (process:2594): WARNING **: Failed to open login1 session: GDBus.Error:org.freedesktop.login1.NoSessionForPID: PID 2594 does not belong to any known session That should be fixed now. It now boots for me, or most things seem to work as they should. There seems to be a few issues left though. For instance systemd-analyze critical-chain gives: # systemd-analyze critical-chain The time after the unit is active or started is printed after the @ character. The time the unit takes to start is printed after the + character. graphical.target @909ms Whereas systemd-analyze gives: Startup finished in 3.091s (firmware) + 12ms (loader) + 3.773s (kernel) + 158ms (initrd) + 574ms (userspace) = 7.610s So something is obviously missing from the critical chain, and the timings given to (userspace) seem to be too low... -t ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
With EPOLLONESHOT we are guaranteed to only recieve one event
until we reload with socket_enter_listening(s), otherwise
multiple events can be generated upon receipt of multiple chunks of data.
We also only want wake-ups when external events happen, i.e.
edge-triggered wakeups.
---
src/core/socket.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/core/socket.c b/src/core/socket.c
index 5fa4a5a..2f860a4 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1119,7 +1119,9 @@ static int socket_watch_fds(Socket *s) {
if (p-event_source)
r = sd_event_source_set_enabled(p-event_source,
SD_EVENT_ON);
else
-r = sd_event_add_io(UNIT(s)-manager-event, p-fd,
EPOLLIN, socket_dispatch_io, p, p-event_source);
+r = sd_event_add_io(UNIT(s)-manager-event, p-fd,
+EPOLLIN|EPOLLET|((s-accept ||
s-distribute) ? 0 : EPOLLONESHOT),
+socket_dispatch_io, p,
p-event_source);
if (r 0) {
log_warning_unit(UNIT(s)-id, Failed to watch
listening fds: %s, strerror(-r));
--
1.8.4.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
oh wait, this doesn't build, here is a version that builds
On Fri, Nov 22, 2013 at 6:56 PM, Shawn Landden sh...@churchofgit.com wrote:
With EPOLLONESHOT we are guaranteed to only recieve one event
until we reload with socket_enter_listening(s), otherwise
multiple events can be generated upon receipt of multiple chunks of data.
We also only want wake-ups when external events happen, i.e.
edge-triggered wakeups.
---
src/core/socket.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/core/socket.c b/src/core/socket.c
index 5fa4a5a..2f860a4 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1119,7 +1119,9 @@ static int socket_watch_fds(Socket *s) {
if (p-event_source)
r = sd_event_source_set_enabled(p-event_source,
SD_EVENT_ON);
else
-r = sd_event_add_io(UNIT(s)-manager-event, p-fd,
EPOLLIN, socket_dispatch_io, p, p-event_source);
+r = sd_event_add_io(UNIT(s)-manager-event, p-fd,
+EPOLLIN|EPOLLET|((s-accept ||
s-distribute) ? 0 : EPOLLONESHOT),
+socket_dispatch_io, p,
p-event_source);
if (r 0) {
log_warning_unit(UNIT(s)-id, Failed to watch
listening fds: %s, strerror(-r));
--
1.8.4.4
From 9ee3855eceeab5bf2f0a2de8f0989b8e3fcef8d3 Mon Sep 17 00:00:00 2001
From: Shawn Landden sh...@churchofgit.com
Date: Wed, 20 Nov 2013 00:55:23 -0800
Subject: [PATCH] core/socket: we only want one event on standard sockets
With EPOLLONESHOT we are guaranteed to only recieve one event
until we reload with socket_enter_listening(s), otherwise
multiple events can be generated upon receipt of multiple chunks of data.
We also only want wake-ups when external events happen, i.e.
edge-triggered wakeups.
---
src/core/socket.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/core/socket.c b/src/core/socket.c
index 5fa4a5a..666ee39 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1119,7 +1119,9 @@ static int socket_watch_fds(Socket *s) {
if (p-event_source)
r = sd_event_source_set_enabled(p-event_source, SD_EVENT_ON);
else
-r = sd_event_add_io(UNIT(s)-manager-event, p-fd, EPOLLIN, socket_dispatch_io, p, p-event_source);
+r = sd_event_add_io(UNIT(s)-manager-event, p-fd,
+EPOLLIN|EPOLLET|(s-accept ? 0 : EPOLLONESHOT),
+socket_dispatch_io, p, p-event_source);
if (r 0) {
log_warning_unit(UNIT(s)-id, Failed to watch listening fds: %s, strerror(-r));
--
1.8.4.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
