Re: [systemd-devel] Complex supervision structures/delegating watchdog?

2015-02-16 Thread Holger Hans Peter Freyther 
On Mon, Feb 16, 2015 at 11:21:53AM +0100, Lennart Poettering wrote:
> 
> Is your pppd daemon itself also a systemd service?
> 
> What precisely does "monitor" consist of for this case?

Yes, the pppd is invoked by systemd as a service. What I am
doing right now is:

link.service:
ExecStart=pppd call uplink ... nodetach

/etc/ppp/ip-up.d/NN-linkmon:
linkmon -i $PPP_IFACE -d 8.8.8.8 -p `cat /run/${PPP_IFACE}.pid` &


The monitoring right now involves simple ICMP request and
depending on the outcome I change the metric of one of the
default routes. So in case some amount of packet loss is
reached the linkmon will SIGKILL the pid provided.

> We have watchdog support already, with sd_notify(0, "WATCHDOG=1"), and
> WatchdogSec=. But that requires you to run your pppd as a service of
> its own, to be useful.

I thought the sd_notify is only possible by the "main"
application that got started? E.g. in the above case the
linkmon would be a child of pppd. My application wouldn't
run until pppd has setup the link. This means I would need
to configure a high enough timeout to cope with a potential
bigger set-up time.


One nice thing for an external watchdog started by the
NN-linkmon script would be that it would be under pid1
control (e.g. if it is crashing, the other service would
be taken down by systemd), I could have different privileges
for the monitoring system (currently at least one part of
it must be able to send a kill to a parent process).

Another neat feature would be if applications could
communicate some extra (custom) status to systemd. E.g.
in the case of pppd to indicate the state of the link-setup,
for something like our BTS process to indicate if it is
currently broadcasting or muted.

Thanks for listening
holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] make: Automake is complaining about .PRECIOUS being redefined

2013-12-26 Thread Holger Hans Peter Freyther 
On Sat, Jul 20, 2013 at 08:37:14AM +0200, Holger Hans Peter Freyther wrote:

> > But that looks like a plain automake bug. It should know that targets
> > starting with a dot are not real targets. Could you file a bug
> > report with automake, so this gets fixed properly?
> 
> I sent an email to the automake list and will report back.

automake 1.15 will have a fix for that.

holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-12-17 Thread Holger Hans Peter Freyther 
On Sun, Jul 07, 2013 at 09:18:01AM +0200, Holger Hans Peter Freyther wrote:

Good Morning,

> > So unless there is an issue with my recording/replay I think that
> > besides my opinion that mapping a < 4MB file 65 times is ugly, it
> > also appears to be slower for the above workload in journald.
> 
> ping? any comments? what workload should benefit from the mmap cache?

I saw that you made some changes to journald that could benefit the
performance (cache cgroup root path, allocate larger blocks) and wonder
if you want or have re-visited your mmap cache.

In the trace/record of the append only case I showed that all allocated
Window.size were smaller WINDOW_SIZE (bloating the address space, making
finding real leaks more difficult and was a slow down in the benchmark).

Now with your "allocate larger blocks" how many windows do you end up
with for the minimal journal? What is the hit/miss ratio on these Windows?


cheers
holger

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] test: Make testing work on systems without?or old systemd

2013-08-21 Thread Holger Hans Peter Freyther 
On Thu, Aug 22, 2013 at 07:07:07AM +0200, Zbigniew Jędrzejewski-Szmek wrote:

> Those tests everywhere are not very pretty, but I guess we don't have
> much choice if we want those tests to be runnable before booting with systemd.

> Applied.

thanks! do you have a better idea on how to skip them?

holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

2013-08-14 Thread Holger Hans Peter Freyther 
On Wed, Aug 14, 2013 at 11:15:35PM +0200, Kay Sievers wrote:

> I temporarily switched it off now.

thanks for the heads up. 

> Maybe someone has an idea, or an alternative.

If David starts to execute "make check && make distcheck" there
is probably nothing travis-ci provides that David's setup doesn't.
As an alternative I can play catch-up and provide/build additional
deps.

holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

2013-08-14 Thread Holger Hans Peter Freyther 
On Wed, Aug 14, 2013 at 02:48:07PM -0700, David Strauss wrote:

> The CI I maintain [1] for systemd is certainly one alternative that
> continues to be in production. I'd like to get more test automation
> going, but it's not like TravisCI was running any tests beyond a
> successful build. I plan to continue maintaining the Jenkins CI, if

Please take a look at the .travis.yml file. It executes make check
and make distcheck. Maybe on your jenkins setup you try these two
commands as well and check how far you get?

holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] make: Automake is complaining about .PRECIOUS being redefined

2013-07-20 Thread Holger Hans Peter Freyther 
On Sat, Jul 20, 2013 at 03:20:02PM +0200, Zbigniew Jędrzejewski-Szmek wrote:

> ... that's at least how it looks in my testing. I don't think that
> having %MAKEFILE% in the final Makefile makes any sense, and it
> swhould always be "Makefile". Maybe automake forgets to substitute it
> in your case, or something eelse is hapenning.

okay, thanks for checking. so a human error on my side after all. :)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] make: Automake is complaining about .PRECIOUS being redefined

2013-07-19 Thread Holger Hans Peter Freyther 
On Sat, Jul 20, 2013 at 01:09:35AM +0200, Zbigniew Jędrzejewski-Szmek wrote:

Hi,


> Applied for now.


I picked %MAKEFILE% as this is what was in the generated Makefile before
my patch. I just tried to match the output.

> But that looks like a plain automake bug. It should know that targets
> starting with a dot are not real targets. Could you file a bug
> report with automake, so this gets fixed properly?

I sent an email to the automake list and will report back.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] RFC: journald: Allow to cache the cg_get_root_path

2013-07-18 Thread Holger Hans Peter Freyther 
On Thu, Jul 18, 2013 at 08:32:18PM +0200, Karol Lewandowski wrote:

> Agreed, this is why I have been naively thinking that caching wouldn't
> be that bad... However, sending all the required information in message
> itself is clearly the best solution to this problem.

It is nice to avoid the race for sure but unless I am not using perf
record/top correctly or perf itself is broken on ARM, the bottleneck
is really not the opening of files. For me glibc's _int_malloc remains
at the top. So even if we skip a log of reading of files the journald
will still create strings for KEY=VALUE.

So maybe we could put key/val separtaely into the iovec array and avoid
the extra strdup and then in the journal-file we can avoid the memchr
for '='? Anyway I have not looked at journal-file.c much yet...

holger

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] journal: Leave server_dispatch_message early when Storage is none

2013-07-18 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

When using Storage=none there is no point in collecting all the
information just to throw them away. After this change journald
consumes a lot less CPU time when only forwarding messages.
---
 src/journal/journald-server.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 369ebf4..f3599b7 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -862,6 +862,11 @@ void server_dispatch_message(
 if (LOG_PRI(priority) > s->max_level_store)
 return;
 
+/* Stop early in case the information will not be stored
+ * in a journal. */
+if (s->storage == STORAGE_NONE)
+return;
+
 if (!ucred)
 goto finish;
 
-- 
1.8.3.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] make: Automake is complaining about .PRECIOUS being redefined

2013-07-18 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

Yesterday I added test-suite.log as dependency to the .PRECIOUS
target. Automake is warning about this target being redefined
and from what I see there is no way I can stop the warning but
I can add the %MAKEFILE% as dependency.

automake warning:
Makefile.am:35: warning: user target '.PRECIOUS' defined here ...
/usr/share/automake-1.13/am/configure.am: ... overrides Automake target 
'.PRECIOUS' defined here
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index e598585..c9b0715 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -32,7 +32,7 @@ SUBDIRS = . po
 .SECONDARY:
 
 # Keep the test-suite.log
-.PRECIOUS: $(TEST_SUITE_LOG)
+.PRECIOUS: $(TEST_SUITE_LOG) %MAKEFILE%
 
 LIBUDEV_CURRENT=4
 LIBUDEV_REVISION=6
-- 
1.8.3.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] test: Make testing work on systems without or old systemd

2013-07-17 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

* Introduce a macro to conditionally execute tests. This avoids
  skipping the entire test if some parts require systemd
* Skip the journal tests when no /etc/machine-id is present
* Change test-catalog to load the catalog from the source directory
  of systemd.
* /proc/PID/comm got introduced in v2.6.33 but travis is still
  using v2.6.32.
* Enable make check and make distcheck on the travis build
* Use -D"CATALOG_DIR=STR($(abs_top_srcdir)/catalog)" as a STRINGIY
  would result in the path '/home/ich/source/linux' to be expanded
  to '/home/ich/source/1' as linux is defined to 1.
---
 .travis.yml |  5 +++--
 Makefile.am | 13 +++--
 src/journal/test-catalog.c  | 20 +---
 src/journal/test-journal-interleaving.c |  4 
 src/journal/test-journal-stream.c   |  4 
 src/journal/test-journal-verify.c   |  4 
 src/journal/test-journal.c  |  4 
 src/test/test-cgroup-util.c |  5 +++--
 src/test/test-helper.h  | 31 +++
 src/test/test-id128.c   | 11 +++
 src/test/test-unit-file.c   |  3 ++-
 src/test/test-unit-name.c   |  5 -
 src/test/test-util.c|  9 +++--
 13 files changed, 101 insertions(+), 17 deletions(-)
 create mode 100644 src/test/test-helper.h

diff --git a/.travis.yml b/.travis.yml
index 42433fd..7e5251c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,8 +3,9 @@ compiler:
   - gcc
 before_install:
  - sudo apt-get update -qq
- - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf
-script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && make dist V=1
+ - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf python2.7-dev
+script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && sudo ./systemd-machine-id-setup && make check && make distcheck
+after_failure: cat test-suite.log
 notifications:
   irc:
 channels:
diff --git a/Makefile.am b/Makefile.am
index c4b9b1a..e598585 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1130,6 +1130,9 @@ EXTRA_DIST += \
test/sched_rr_ok.service \
test/sched_rr_change.service
 
+EXTRA_DIST += \
+   src/test/test-helper.h
+
 test_engine_SOURCES = \
src/test/test-engine.c
 
@@ -1328,7 +1331,8 @@ test_cgroup_util_SOURCES = \
 
 test_cgroup_util_LDADD = \
libsystemd-label.la \
-   libsystemd-shared.la
+   libsystemd-shared.la \
+   libsystemd-daemon.la
 
 test_env_replace_SOURCES = \
src/test/test-env-replace.c
@@ -2647,7 +2651,8 @@ test_id128_SOURCES = \
 
 test_id128_LDADD = \
libsystemd-shared.la \
-   libsystemd-id128-internal.la
+   libsystemd-id128-internal.la \
+   libsystemd-daemon.la
 
 tests += \
test-id128
@@ -2810,6 +2815,10 @@ test_mmap_cache_LDADD = \
 test_catalog_SOURCES = \
src/journal/test-catalog.c
 
+test_catalog_CFLAGS = \
+   $(AM_CFLAGS) \
+   -D"STR(s)=\#s" -D"CATALOG_DIR=STR($(abs_top_srcdir)/catalog)"
+
 test_catalog_LDADD = \
libsystemd-shared.la \
libsystemd-label.la \
diff --git a/src/journal/test-catalog.c b/src/journal/test-catalog.c
index 987867f..5db5bed 100644
--- a/src/journal/test-catalog.c
+++ b/src/journal/test-catalog.c
@@ -31,6 +31,16 @@
 #include "sd-messages.h"
 #include "catalog.h"
 
+static const char *catalog_dirs[] = {
+CATALOG_DIR,
+NULL,
+};
+
+static const char *no_catalog_dirs[] = {
+"/bin/hopefully/with/no/catalog",
+NULL
+};
+
 static void test_import(Hashmap *h, struct strbuf *sb,
 const char* contents, ssize_t size, int code) {
 int r;
@@ -100,9 +110,13 @@ static void test_catalog_update(void) {
 r = catalog_update(database, NULL, NULL);
 assert(r >= 0);
 
-/* Note: this might actually not find anything, if systemd was
- * not installed before. That should be fine too. */
-r = catalog_update(database, NULL, catalog_file_dirs);
+/* Test what happens if there are no files in the directory. */
+r = catalog_update(database, NULL, no_catalog_dirs);
+assert(r >= 0);
+
+/* Make sure that we at least have some files loaded or the
+   catalog_list below will fail. *

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

2013-07-17 Thread Holger Hans Peter Freyther 
On Wed, Jul 17, 2013 at 04:02:26PM -0300, Lucas De Marchi wrote:

> Talking about kmod and travis-ci. In kmod we are using it. Pretty
> cool, with an IRC bot integration. I only wish there were more options
> of distributions.  Right now it's only Ubuntu 12.04 32 bits

You could ask the people that operate hydra.nixos.org to build
kmod for you. This will at least give you ia32 and x86-64 builds
and a coverage report for the x86-64. I am not sure if they have
irc notifications and how open they are to non GNU projects.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

2013-07-17 Thread Holger Hans Peter Freyther 
On Wed, Jul 17, 2013 at 05:08:46PM +0200, Kay Sievers wrote:
> 
> I did something like that now. Let's see if it works ...

Thank you, it did[1].

[1] https://travis-ci.org/systemd/systemd
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] test: Make testing work on systems without or old systemd

2013-07-17 Thread Holger Hans Peter Freyther 
On Thu, Jul 18, 2013 at 06:16:48AM +0200, Zbigniew Jędrzejewski-Szmek wrote:

> Can we use the STRINGIFY macro instead?

I can try.

> >  /* Note: this might actually not find anything, if systemd was
> >   * not installed before. That should be fine too. */
> > -r = catalog_update(database, NULL, catalog_file_dirs);
> > +r = catalog_update(database, NULL, catalog_dirs);
> OK, so now the comment either was always inaccurate, or just became 
> inaccurate.
> Does it really fail if the catalog was not installed before?
> I wanted the case of no installed systemd to be a test for a missing catalog,
> so it was on purpose.
> 
> >  assert(r >= 0);

Yes, you will ran into this assert. We can pass a dir with ["/nonexistant"] and
stat that it is non-existant.


> Wouldn't access("/etc/machine-id", F_OK) look better, and avoid the extra
> variable?

okay.

> > -log_info("pid1 comm: '%s'", a);
> > +if (stat("/proc/1/comm", &st) == 0) {
> > +assert_se(get_process_comm(1, &a) >= 0);
> > +log_info("pid1 comm: '%s'", a);
> > +} else {
> > +log_info("/proc/1/comm does not exist.");
> log_warning?

I just made it match the other output level.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] test: Make testing work on systems without or old systemd

2013-07-17 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

* Introduce a macro to conditionally execute tests. This avoids
  skipping the entire test if some parts require systemd
* Skip the journal tests when no /etc/machine-id is present
* Change test-catalog to load the catalog from the source directory
  of systemd.
* /proc/PID/comm got introduced in v2.6.33 but travis is still
  using v2.6.32.
* Enable make check and make distcheck on the travis build
---
 .travis.yml |  5 +++--
 Makefile.am | 13 +++--
 src/journal/test-catalog.c  |  7 ++-
 src/journal/test-journal-interleaving.c |  5 +
 src/journal/test-journal-stream.c   |  5 +
 src/journal/test-journal-verify.c   |  4 
 src/journal/test-journal.c  |  5 +
 src/test/test-cgroup-util.c |  5 +++--
 src/test/test-helper.h  | 31 +++
 src/test/test-id128.c   | 11 +++
 src/test/test-unit-file.c   |  3 ++-
 src/test/test-unit-name.c   |  5 -
 src/test/test-util.c|  9 +++--
 13 files changed, 93 insertions(+), 15 deletions(-)
 create mode 100644 src/test/test-helper.h

diff --git a/.travis.yml b/.travis.yml
index 42433fd..7e5251c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,8 +3,9 @@ compiler:
   - gcc
 before_install:
  - sudo apt-get update -qq
- - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf
-script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && make dist V=1
+ - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf python2.7-dev
+script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && sudo ./systemd-machine-id-setup && make check && make distcheck
+after_failure: cat test-suite.log
 notifications:
   irc:
 channels:
diff --git a/Makefile.am b/Makefile.am
index c4b9b1a..e598585 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1130,6 +1130,9 @@ EXTRA_DIST += \
test/sched_rr_ok.service \
test/sched_rr_change.service
 
+EXTRA_DIST += \
+   src/test/test-helper.h
+
 test_engine_SOURCES = \
src/test/test-engine.c
 
@@ -1328,7 +1331,8 @@ test_cgroup_util_SOURCES = \
 
 test_cgroup_util_LDADD = \
libsystemd-label.la \
-   libsystemd-shared.la
+   libsystemd-shared.la \
+   libsystemd-daemon.la
 
 test_env_replace_SOURCES = \
src/test/test-env-replace.c
@@ -2647,7 +2651,8 @@ test_id128_SOURCES = \
 
 test_id128_LDADD = \
libsystemd-shared.la \
-   libsystemd-id128-internal.la
+   libsystemd-id128-internal.la \
+   libsystemd-daemon.la
 
 tests += \
test-id128
@@ -2810,6 +2815,10 @@ test_mmap_cache_LDADD = \
 test_catalog_SOURCES = \
src/journal/test-catalog.c
 
+test_catalog_CFLAGS = \
+   $(AM_CFLAGS) \
+   -D"STR(s)=\#s" -D"CATALOG_DIR=STR($(abs_top_srcdir)/catalog)"
+
 test_catalog_LDADD = \
libsystemd-shared.la \
libsystemd-label.la \
diff --git a/src/journal/test-catalog.c b/src/journal/test-catalog.c
index 987867f..9620301 100644
--- a/src/journal/test-catalog.c
+++ b/src/journal/test-catalog.c
@@ -31,6 +31,11 @@
 #include "sd-messages.h"
 #include "catalog.h"
 
+static const char *catalog_dirs[] = {
+CATALOG_DIR,
+NULL,
+};
+
 static void test_import(Hashmap *h, struct strbuf *sb,
 const char* contents, ssize_t size, int code) {
 int r;
@@ -102,7 +107,7 @@ static void test_catalog_update(void) {
 
 /* Note: this might actually not find anything, if systemd was
  * not installed before. That should be fine too. */
-r = catalog_update(database, NULL, catalog_file_dirs);
+r = catalog_update(database, NULL, catalog_dirs);
 assert(r >= 0);
 }
 
diff --git a/src/journal/test-journal-interleaving.c 
b/src/journal/test-journal-interleaving.c
index 069d297..bd3cb7f 100644
--- a/src/journal/test-journal-interleaving.c
+++ b/src/journal/test-journal-interleaving.c
@@ -286,8 +286,13 @@ static void test_sequence_numbers(void) {
 }
 
 int main(int argc, char *argv[]) {
+struct stat st;
 log_set_max_level(LOG_DEBUG);
 
+/* journal_file_open requires a valid machine id */
+if (stat("/etc/machine-id", &st) != 0)
+return EXIT_TEST_SKIP;
+
 arg_keep = argc > 1;
 
 test_skip(setup_se

[systemd-devel] [PATCH 1/2] test: Keep the test-suite.log around in case of a test failure

2013-07-17 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

The addition of .DELETE_ON_ERROR will lead to the removal of the
test-suite.log in case of a test failure. Mark the rule as PRECIOUS
to keep that file around.
---
 Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 7e6361c..c4b9b1a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -31,6 +31,9 @@ SUBDIRS = . po
 # keep intermediate files
 .SECONDARY:
 
+# Keep the test-suite.log
+.PRECIOUS: $(TEST_SUITE_LOG)
+
 LIBUDEV_CURRENT=4
 LIBUDEV_REVISION=6
 LIBUDEV_AGE=3
-- 
1.8.3.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

2013-07-17 Thread Holger Hans Peter Freyther 
On Fri, Jul 12, 2013 at 07:51:26PM +0200, Lennart Poettering wrote:

> Commited. I figure this will not be useful right-away, since current git
> of systemd requires a very new version of kmod that is not available at
> Travis. However, it's still a good thing to have and to enable as soon
> as that issue is solved.

I could only check this today. The ubuntu/travis build is currently not
using kmod at all so it could be useful right-away. What is missing is
that any member of the systemd group on github visits the travis-ci.org
website, clicks on 'Sign in with GitHub', go to the account section and
enables travis-ci for the systemd repository. It should be < 1 minute of
work.

cheers
holger

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Broken build and CI strategy

2013-07-14 Thread Holger Hans Peter Freyther 
On Sun, Jul 14, 2013 at 01:52:49PM -0700, David Strauss wrote:

Good Morning,

> I can't enable global "make check" of systemd builds in CI until we
> fix the TTY dependency or I do research on how to run the tests with
> TTY support within Jenkins. Personally, I'd rather see the checks
> improved to not require a TTY or to provide the wrapping TTY for the
> necessary tests. I have agreed in anther email exchange to look into
> enabling specific "make check"s that don't require TTY. I keep the
> dummy "make test" as a reminder that we really ought to be automating
> post-build tests.

Well, I don't know about this depedency but if it is really a matter
of having a TTY, did you consider using the 'Nodes' support of Jenkins
and then SSH into the node?

> 
> I'm also happy to delegate more access to the CI box if others would
> like to improve the test integration. I'd like to throw in more static
> analysis, too.

Maybe do "echo 'Not running tests as they always fail in this setup.'"
is better? There were several fanboys sending me private mail claiming
that the tests were run because of the 'make test'.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

2013-07-12 Thread Holger Hans Peter Freyther 
On Fri, Jul 12, 2013 at 07:51:26PM +0200, Lennart Poettering wrote:
> On Tue, 09.07.13 18:09, Holger Hans Peter Freyther (hol...@freyther.de) wrote:
> I figure a simple 
> 
> if (sd_booted() <= 0)
> return EXIT_TEST_SKIP;

Ah great, I had planned to search if something like sd_booted() exists,
skipping the tests will be trivial now.

thanks
holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

2013-07-09 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

Instruct travis-ci to build systemd and create a tarball. In case
of an error travis-ci will complain on IRC. The systemd testsuite
currently requires the host to have a recent version of systemd
installed and running. This is not the case for the Ubuntu VM of
travis-ci. This means make check and make distcheck will result in
a build failure and to avoid this these commands are not executed.

This requires a one time configuration on travis-ci for the repo
on github by the owner of the repo.
---
 .travis.yml |   13 +
 1 file changed, 13 insertions(+)
 create mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index 000..42433fd
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,13 @@
+language: c
+compiler:
+  - gcc
+before_install:
+ - sudo apt-get update -qq
+ - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf
+script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && make dist V=1
+notifications:
+  irc:
+channels:
+  - "irc.freenode.org#systemd"
+on_success: change
+on_failure: always
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Broken build and CI strategy

2013-07-08 Thread Holger Hans Peter Freyther 
On Fri, Jun 28, 2013 at 10:06:11AM +0200, Holger Hans Peter Freyther wrote:
> On Fri, Jun 28, 2013 at 09:43:08AM +0200, Peter Sztanojev wrote:
> 
> > So this issue is about tweaking how jenkins does its job?
> > I have added David Strauss to the CC, hopefully he won't mind.
> 
> Well, that is one part. "make test" really just checks if the test/
> directory exists, it doesn't really contribute to the quality control.

ping? I just checked the current build log and it is still issuing a
"make test" (which still tests if the test directory exists). I think
executing "make check" and "make distcheck" would be more approriate.

cheers
holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-07-07 Thread Holger Hans Peter Freyther 
On Wed, Jun 26, 2013 at 09:58:36PM +0200, Holger Hans Peter Freyther wrote:

> So unless there is an issue with my recording/replay I think that
> besides my opinion that mapping a < 4MB file 65 times is ugly, it
> also appears to be slower for the above workload in journald.

ping? any comments? what workload should benefit from the mmap cache?

holger

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] build-sys: "link" libsystemd-id128.so with libsystemd-label

2013-06-29 Thread Holger Hans Peter Freyther 
On Sat, Jun 29, 2013 at 06:28:40AM +0200, Michael Biebl wrote:

> > could you check if this fixes the build for you?
> 
> It does indeed.

the same for the travis build on ubuntu (make check still fails
though but that is another issue).

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Broken build and CI strategy

2013-06-28 Thread Holger Hans Peter Freyther 
On Fri, Jun 28, 2013 at 07:05:35PM +0200, Michael Biebl wrote:

> That said, I don't understand, why this commit breaks the build with
> that (older) toolchain.
> Neither bootctl nor libsystemd-id128.so is using cg_create().

at the same time it has an undefined reference...

$ nm -D .libs/libsystemd-id128.so | grep cg_create
 U cg_create

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Broken build and CI strategy

2013-06-28 Thread Holger Hans Peter Freyther 
On Fri, Jun 28, 2013 at 04:01:08PM +0200, Lennart Poettering wrote:
> 
> Maybe this is simply broken automake dependency info somewhere left in
> your tree?

I did the git clean but I still wondered, this is why I created the
.travis.yml. Before I was building a branch that contained some of
my RFC patches, now it is master + the .travis.yml and it is failing
in the same way[1].

cheers
holger

PS: Yes, travis does support IRC notifications[2]

[1] https://travis-ci.org/zecke/systemde/builds/8542339
[2] http://about.travis-ci.org/docs/user/notifications/#IRC-notification
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Broken build and CI strategy

2013-06-28 Thread Holger Hans Peter Freyther 
On Fri, Jun 28, 2013 at 03:44:35PM +0200, Michael Biebl wrote:

> v44, v204 and git master all build and link fine on my pretty standard
> Debian sid system, so I can't reproduce the problem, at least not on
> Debian.

Which linker and which system do you run it on? A clean build on my
Debian Testing (i386) and Ubuntu 12.04 (travis, amd64) with GNU ld.bfd
are both failing. GNU gold can link it on boths targets.

I will not spend time to analyze it...

cheers
holger

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Broken build and CI strategy

2013-06-28 Thread Holger Hans Peter Freyther 
On Fri, Jun 28, 2013 at 09:43:08AM +0200, Peter Sztanojev wrote:

> So this issue is about tweaking how jenkins does its job?
> I have added David Strauss to the CC, hopefully he won't mind.

Well, that is one part. "make test" really just checks if the test/
directory exists, it doesn't really contribute to the quality control.

The other thing with "make check" is that it is failing if the build
system doesn't run systemd[1] or fails if the installed version is not
new enough (debian still ships systemd 44 that doesn't have catalogs
so the catalog test fails).


> >> The output of which can be seen on IRC.
> >> The problems always seem to come from non-standard/broken setups
> >
> > Could you please elaborate on standard vs. non-standard/broken setups.
> > travis is building on a clean VM and installing most of the packages
> > specified in the README file.

systemd currently does not link on default Debian/Ubuntu systems.
Could you please elaborate how this is a non-standard/broken setup?

I can make it link by installing binutils-gold, if systemd now
requires gold, could you please update the configure.ac and README
to reflect this?

holger


[1] https://travis-ci.org/zecke/systemde/builds/8531920
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Broken build and CI strategy

2013-06-28 Thread Holger Hans Peter Freyther 
On Fri, Jun 28, 2013 at 09:05:40AM +0200, Peter Sztanojev wrote:
> On Fri, Jun 28, 2013 at 8:50 AM, Holger Hans Peter Freyther

> there already is a jenkins ci for systemd kindly provided by Pantheon:
> http://systemd.getpantheon.com:8080/jenkins/

The jenkins script is still using "make test" (which is like calling
"make /tmp"), to execute the tests one would need to call "make check"
but we already had this and it wasn't changed...

> The output of which can be seen on IRC.
> The problems always seem to come from non-standard/broken setups

Could you please elaborate on standard vs. non-standard/broken setups.
travis is building on a clean VM and installing most of the packages
specified in the README file.

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Broken build and CI strategy

2013-06-28 Thread Holger Hans Peter Freyther 
Dear Lennart,

the systemd build is currently broken[1] and judging from the commit
history it appears that this happened more than once. Besides fixing
the build you might want to consider adopting another strategy.

In mature software projects the following is quite common:

a.) Create a .travis.yml, sync your repo to github[2] and have
travis-ci build your code. My .travis.yml file for systemd is
here[3]. Travis can send email to the repo owners/travis owners
on failed builds.

b.) Use a combination of gerrit/jenkins. E.g. if you don't want
to have broken builds in master. One would push changes for review to
Gerrit, these would be automatically build on a Jenkins system
and if they compile they can be integrated into the main repository.
The workflow is used in projects like coreboot.


cheers
holger


[1] https://travis-ci.org/zecke/systemde/builds/8530934
[2] github can create official mirrors of repositories but they only
sync a couple of times/once a day.
[3] https://github.com/zecke/systemde/blob/master/.travis.yml
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] RFC: journald: Allow to cache the cg_get_root_path

2013-06-27 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

Allow to cache the cg_get_root_path and introduce a new method
cg_pid_get_path_shifted_with_root that can use the cached version
instead of allocating a new string.
---
 src/journal/journald-server.c |   20 +---
 src/shared/cgroup-util.c  |   19 ++-
 src/shared/cgroup-util.h  |1 +
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 44ba916..b08aa1d 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -89,6 +89,20 @@ static const char* const split_mode_table[] = {
 DEFINE_STRING_TABLE_LOOKUP(split_mode, SplitMode);
 DEFINE_CONFIG_PARSE_ENUM(config_parse_split_mode, split_mode, SplitMode, 
"Failed to parse split mode setting");
 
+static const char *cached_cg_root(void) {
+static char *cached = NULL;
+
+int r;
+
+if (!cached) {
+r = cg_get_root_path(&cached);
+if (r < 0)
+cached = NULL;
+}
+
+return cached;
+}
+
 static uint64_t available_space(Server *s, bool verbose) {
 char ids[33];
 _cleanup_free_ char *p = NULL;
@@ -592,7 +606,7 @@ static void dispatch_message_real(
 }
 #endif
 
-r = cg_pid_get_path_shifted(ucred->pid, NULL, &c);
+r = cg_pid_get_path_shifted_with_root(ucred->pid, NULL, &c, 
cached_cg_root());
 if (r >= 0) {
 char *session = NULL;
 
@@ -701,7 +715,7 @@ static void dispatch_message_real(
 }
 #endif
 
-r = cg_pid_get_path_shifted(object_pid, NULL, &c);
+r = cg_pid_get_path_shifted_with_root(object_pid, NULL, &c, 
cached_cg_root());
 if (r >= 0) {
 x = strappenda("OBJECT_SYSTEMD_CGROUP=", c);
 IOVEC_SET_STRING(iovec[n++], x);
@@ -841,7 +855,7 @@ void server_dispatch_message(
 if (!ucred)
 goto finish;
 
-r = cg_pid_get_path_shifted(ucred->pid, NULL, &path);
+r = cg_pid_get_path_shifted_with_root(ucred->pid, NULL, &path, 
cached_cg_root());
 if (r < 0)
 goto finish;
 
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 5816b7d..1a76794 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1059,14 +1059,12 @@ char **cg_shorten_controllers(char **controllers) {
 return strv_uniq(controllers);
 }
 
-int cg_pid_get_path_shifted(pid_t pid, char **root, char **cgroup) {
-_cleanup_free_ char *cg_root = NULL;
+int cg_pid_get_path_shifted_with_root(pid_t pid, char **root, char **cgroup, 
const char *cg_root) {
 char *cg_process, *p;
 int r;
 
-r = cg_get_root_path(&cg_root);
-if (r < 0)
-return r;
+if (!cg_root)
+return -1;
 
 r = cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, pid, &cg_process);
 if (r < 0)
@@ -1099,6 +1097,17 @@ int cg_pid_get_path_shifted(pid_t pid, char **root, char 
**cgroup) {
 return 0;
 }
 
+int cg_pid_get_path_shifted(pid_t pid, char **root, char **cgroup) {
+_cleanup_free_ char *cg_root = NULL;
+int r;
+
+r = cg_get_root_path(&cg_root);
+if (r < 0)
+return r;
+
+return cg_pid_get_path_shifted_with_root(pid, root, cgroup, cg_root);
+}
+
 int cg_path_decode_unit(const char *cgroup, char **unit){
 char *p, *e, *c, *s, *k;
 
diff --git a/src/shared/cgroup-util.h b/src/shared/cgroup-util.h
index 9883d94..7f6605c 100644
--- a/src/shared/cgroup-util.h
+++ b/src/shared/cgroup-util.h
@@ -104,6 +104,7 @@ int cg_path_get_machine_name(const char *path, char 
**machine);
 int cg_path_get_slice(const char *path, char **slice);
 
 int cg_pid_get_path_shifted(pid_t pid, char **root, char **cgroup);
+int cg_pid_get_path_shifted_with_root(pid_t pid, char **root, char **cgroup, 
const char *cg_root);
 
 int cg_pid_get_session(pid_t pid, char **session);
 int cg_pid_get_owner_uid(pid_t pid, uid_t *uid);
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] RFC: journald: Avoid reading /proc/PID/cgroup more than once per msg

2013-06-27 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

---
 src/journal/journald-server.c |   45 ++---
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index b08aa1d..e651a6d 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -523,7 +523,8 @@ static void dispatch_message_real(
 struct timeval *tv,
 const char *label, size_t label_len,
 const char *unit_id,
-pid_t object_pid) {
+pid_t object_pid,
+const char *cached_path) {
 
 charpid[sizeof("_PID=") + DECIMAL_STR_MAX(pid_t)],
 uid[sizeof("_UID=") + DECIMAL_STR_MAX(uid_t)],
@@ -541,7 +542,7 @@ static void dispatch_message_real(
 char *x;
 sd_id128_t id;
 int r;
-char *t, *c;
+char *t, *path;
 uid_t realuid = 0, owner = 0, journal_uid;
 bool owner_valid = false;
 #ifdef HAVE_AUDIT
@@ -606,31 +607,30 @@ static void dispatch_message_real(
 }
 #endif
 
-r = cg_pid_get_path_shifted_with_root(ucred->pid, NULL, &c, 
cached_cg_root());
-if (r >= 0) {
+if (cached_path) {
 char *session = NULL;
 
-x = strappenda("_SYSTEMD_CGROUP=", c);
+x = strappenda("_SYSTEMD_CGROUP=", cached_path);
 IOVEC_SET_STRING(iovec[n++], x);
 
-r = cg_path_get_session(c, &t);
+r = cg_path_get_session(cached_path, &t);
 if (r >= 0) {
 session = strappenda("_SYSTEMD_SESSION=", t);
 free(t);
 IOVEC_SET_STRING(iovec[n++], session);
 }
 
-if (cg_path_get_owner_uid(c, &owner) >= 0) {
+if (cg_path_get_owner_uid(cached_path, &owner) >= 0) {
 owner_valid = true;
 
 sprintf(owner_uid, "_SYSTEMD_OWNER_UID=%lu", 
(unsigned long) owner);
 IOVEC_SET_STRING(iovec[n++], owner_uid);
 }
 
-if (cg_path_get_unit(c, &t) >= 0) {
+if (cg_path_get_unit(cached_path, &t) >= 0) {
 x = strappenda("_SYSTEMD_UNIT=", t);
 free(t);
-} else if (cg_path_get_user_unit(c, &t) >= 0) {
+} else if (cg_path_get_user_unit(cached_path, &t) >= 
0) {
 x = strappenda("_SYSTEMD_USER_UNIT=", t);
 free(t);
 } else if (unit_id) {
@@ -643,8 +643,6 @@ static void dispatch_message_real(
 
 if (x)
 IOVEC_SET_STRING(iovec[n++], x);
-
-free(c);
 }
 
 #ifdef HAVE_SELINUX
@@ -715,27 +713,27 @@ static void dispatch_message_real(
 }
 #endif
 
-r = cg_pid_get_path_shifted_with_root(object_pid, NULL, &c, 
cached_cg_root());
+r = cg_pid_get_path_shifted_with_root(object_pid, NULL, &path, 
cached_cg_root());
 if (r >= 0) {
-x = strappenda("OBJECT_SYSTEMD_CGROUP=", c);
+x = strappenda("OBJECT_SYSTEMD_CGROUP=", path);
 IOVEC_SET_STRING(iovec[n++], x);
 
-r = cg_path_get_session(c, &t);
+r = cg_path_get_session(path, &t);
 if (r >= 0) {
 x = strappenda("OBJECT_SYSTEMD_SESSION=", t);
 free(t);
 IOVEC_SET_STRING(iovec[n++], x);
 }
 
-if (cg_path_get_owner_uid(c, &owner) >= 0) {
+if (cg_path_get_owner_uid(path, &owner) >= 0) {
 sprintf(o_owner_uid, 
"OBJECT_SYSTEMD_OWNER_UID=%lu", (unsigned long) owner);
 IOVEC_SET_STRING(iovec[n++], o_owner_uid);
 }
 
-if (cg_path_get_unit(c, &t) >= 0) {
+if (cg_path_get_unit(path, &t) >= 0) {
 x = strappenda("OBJECT_SYSTEMD_UNIT=", t);
 free(t);
-} else if (cg_path_get_user_unit(c, &t) >= 0) {
+} else if

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-06-27 Thread Holger Hans Peter Freyther 
On Fri, Jun 21, 2013 at 06:01:08PM +0200, Holger Hans Peter Freyther wrote:

> Do you have an idea on how this could be done?

Hi,

first of all the MMAP cache is not why journald is slow but that is
for another mail/benchmark. I was just curious if the mmap cache is
premature optimization or if it brings a speed up.

So what I did was to patch journal-file.c, mmap_cache.c and record
the calls made to posix_fallocate, fstat and mmap_cache_get and write
the parameters to a file.

I created a small tool to replay these operations. This way I can
play with the mmap_cache.c and see if my changes make a difference. I
am attaching my hacked together patch as a reference. The journald will
exit when it would normally rotate the log file.


system:
This was tested on a TI Davinci DM644x. The ARM core runs at 405 Mhz
and the system has 256 mib DRAM. It is running Linux 3.2.40, the userspace
is post Poky 9.0.0 (glibc 2.17, gcc 4.7..)


workload/test:
I was using the following to generate log messages as my example
workload. This resulted in a 19MB file with commands.

while true;
do 
  (for i in `seq 1 100`;
   do echo "Log message... $RANDOM";
   done) | logger;
done






Baseline replay with just iterating over the commands:

root@sysmobts-v2:~# time ./replay-mmap-cache-no-work 
DONE

real0m0.275s
user0m0.080s
sys 0m0.180s





Replay with WINDOWS_MIN 64 (just the best, lowest)

root@sysmobts-v2:~# time ./replay-mmap-cache-min-64 
DONE

real0m3.427s
user0m1.200s
sys 0m2.000s





Replay with WINDOWS_MIN 0 (just the slowest)
root@sysmobts-v2:~# time ./replay-mmap-cache-min-0
DONE

real0m2.212s
user0m1.010s
sys 0m1.040s



So unless there is an issue with my recording/replay I think that
besides my opinion that mapping a < 4MB file 65 times is ugly, it
also appears to be slower for the above workload in journald.


kind regards
holger
>From ba1088eb4127206ae3259a7aabd3fc3f4a1943bb Mon Sep 17 00:00:00 2001
From: Holger Hans Peter Freyther 
Date: Wed, 26 Jun 2013 21:12:57 +0200
Subject: [PATCH] hacks.. for creating a mmap test case..

---
 Makefile.am |8 +++
 src/journal/journal-def.h   |   27 ++
 src/journal/journal-file.c  |   28 ++
 src/journal/journald-server.c   |   16 ++
 src/journal/mmap-cache.c|   40 +++
 src/journal/replay-mmap-cache.c |  108 +++
 6 files changed, 227 insertions(+)
 create mode 100644 src/journal/replay-mmap-cache.c

diff --git a/Makefile.am b/Makefile.am
index 3a196a6..33118a6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2697,6 +2697,13 @@ test_mmap_cache_LDADD = \
 	libsystemd-shared.la \
 	libsystemd-journal-internal.la
 
+replay_mmap_cache_SOURCES = \
+	src/journal/replay-mmap-cache.c
+
+replay_mmap_cache_LDADD = \
+	libsystemd-shared.la \
+	libsystemd-journal-internal.la
+
 test_catalog_SOURCES = \
 	src/journal/test-catalog.c
 
@@ -2866,6 +2873,7 @@ tests += \
 	test-journal-stream \
 	test-journal-verify \
 	test-mmap-cache \
+	replay-mmap-cache \
 	test-catalog
 
 pkginclude_HEADERS += \
diff --git a/src/journal/journal-def.h b/src/journal/journal-def.h
index 7e407a4..460741e 100644
--- a/src/journal/journal-def.h
+++ b/src/journal/journal-def.h
@@ -214,3 +214,30 @@ struct FSSHeader {
 le16_t reserved[3];
 le64_t fsprg_state_size;
 } _packed_;
+
+
+struct mmap_cache_data {
+	uint8_t		type;
+#define PERF_CMD_POSIX_ALLOCATE		1
+#define PERF_CMD_FSTAT			2
+#define PERF_CMD_FSTAT_CHECK		3
+#define PERF_CMD_MMAP_CACHE		4
+
+	union {
+		struct cmd_fallocate {
+			uint64_t	old_size;
+			uint64_t	increment;
+		} allocate;
+		struct cmd_fstat {
+			uint64_t	offset;
+			uint64_t	size;
+		} stat;
+		struct cmd_mmap {
+			int prot;
+			unsigned context;
+			bool keep_always;
+			uint64_t offset;
+			size_t size;
+		} mmap;
+	} u;
+} _packed_;
diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c
index 38499a6..f619de9 100644
--- a/src/journal/journal-file.c
+++ b/src/journal/journal-file.c
@@ -359,10 +359,22 @@ static int journal_file_allocate(JournalFile *f, uint64_t offset, uint64_t size)
 /* Note that the glibc fallocate() fallback is very
inefficient, hence we try to minimize the allocation area
as we can. */
+	struct mmap_cache_data data = {
+		.type		= PERF_CMD_POSIX_ALLOCATE,
+		.u.allocate	= {
+			.old_size = old_size,
+			.increment = new_size - old_size,
+		},
+	};
+	write_cache_data(&data);
 r = posix_fallocate(f->fd, old_size, new_size - old_size);
 if (r != 0)
 return -r;
 
+	struct mmap_cache_data data_stat = {
+		.type		= PERF_CMD_FSTAT,
+	};
+	write_cache_data(&data_stat);
 if (fstat(f->fd, &f->last_stat) < 0)
 return -errno;
 
@@ -382,6 +394,14 @@ static int journal_file_move_to(JournalFile *f, int context, bool keep_always, u
 if (offs

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-06-21 Thread Holger Hans Peter Freyther 
On Fri, Jun 21, 2013 at 04:42:41PM +0100, Colin Guthrie wrote:

> Did you read Lennart's reply?

Only after responding. ;)

> I can also assure you that when there was a bug in this cache window
> code about eight months ago it was quite obvious. This was fixed
> (http://cgit.freedesktop.org/systemd/systemd/commit/?id=89de694724f376a6852e879fe987e7e531327654)
> and since that time I've been running several different unattended
> journals and don't have any issues.

thanks
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-06-21 Thread Holger Hans Peter Freyther 
On Fri, Jun 21, 2013 at 02:08:27PM +0200, Lennart Poettering wrote:

> That said, the current map sizes are nothing we tuned particularly. If
> you can show actal performance benefits I am happy to change them.

Yes, I would be interested in having a performance test for this. Do
you have an idea for a setup? In the end it should be something that
is self contained and can be invoked with time in front of an exits
after a certain amount of log messages.

Do you have an idea on how this could be done?

holger

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-06-21 Thread Holger Hans Peter Freyther 
On Fri, Jun 21, 2013 at 05:31:13PM +0200, Holger Hans Peter Freyther wrote:

> I care about whether or not journald will work reliable on an
> unattended system. And from what I see there is no limit in the
> mmap cache. This means that journald can potentially exhaust the
> virtual address space. Will it happen? I don't know, time will
> tell.

I just read Lennart's mail and found the relevant code to limit the
number of windows to WINDOWS_MIN (64, so 65 windows) and
context_attach_window will put the old window in the last_unused.

The only way to allocate more windows is to have more contexts than
WINDOWS_MIN?

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-06-21 Thread Holger Hans Peter Freyther 
On Fri, Jun 21, 2013 at 04:19:30PM +0200, Kay Sievers wrote:

> I have no idea why you care what the journald process does with its
> very own 2+GB of address space, and why it uses 128MB of it.

I care about whether or not journald will work reliable on an
unattended system. And from what I see there is no limit in the
mmap cache. This means that journald can potentially exhaust the
virtual address space. Will it happen? I don't know, time will
tell.

E.g. the test-mmap-cache.c does not test if the freeing of windows
(make_room) in case of a failed MMAP_FAILED. When was the last
time this code got tested/executed?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-06-21 Thread Holger Hans Peter Freyther 
On Fri, Jun 21, 2013 at 01:53:23PM +0200, Kay Sievers wrote:

> Fragmentation, allocation? I don't think we talk about the same thing here.

... you will figure that out.

> Mapping an on-disk file "a symptom of inefficiency", you might need to
> update your idea of how things work.

I didn't say that mmap of a file is bad. But thanks for putting words
in my mouth. I just wonder how a process that manipulates a file that
is stored in /run/log/journal/*/system.journal that can grow up to a
single digit MB needs an address space of 200MB.

Sure the answer is that journald maps the same fd with the same flags
(rwxs) and size multiple times (possible even with the same offset in
the file). Isn't doing more work than needed excessive behavior? When
the journal size is bigger than the available address space this makes
sense but e.g. when using Storage=volatile with a small enough
RuntimeMaxUse one could use mremap with a single X MB mapping?

cheers
holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Excessive (virtual) memory usage of journald

2013-06-21 Thread Holger Hans Peter Freyther 
On Fri, Jun 21, 2013 at 12:29:56PM +0200, Kay Sievers wrote:

> What's the problem with using address *space*? Address space is not
> used memory, file memory mappings are just how things work in general,
> they are cheap and should not really matter.

It is a symptom of inefficiency. If an application maps 130MB of memory
to manage what should be a 648kb ringbuffer it raises some doubts. The
danger comes from the question if the journald will fragment its memory
in a way that a real memory allocation will fail because there is no space
left in the virtual address space.

The above concern would be easier to resolve if used and mapped memory
would be within reasonable bounds when managing just some kb of data.

holger


___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] RFC for avoid dynamic allocation in?journald

2013-04-08 Thread Holger Hans Peter Freyther 
On Mon, Apr 08, 2013 at 07:06:44PM +0200, Lennart Poettering wrote:

> > Is it really needed to determine the cgroup of /proc/1 on every message?
> > Couldn't journald keep the variables for its identity around?

> a) use gcc's __thread support, i.e. have one static cache var per
>thread. Since all stuff in systemd's repo is essentially single
>threaded this should give you the desired effect and should be a
>pretty simple way to have caches without having to think about
>concurrency. Of course, this only really works for small fixed-size
>data that needs no destructors (i.e. no free()).
 
> b) Only provide the caching in the main thread, and be un-cached
>expensive in all others. This is slightly less nice, but effectively
>gives you the same properties for systemd's own code. We have a call
>in util.c to detect if we are running in the main thread:
>is_main_thread().

I understand the constraints. My hardware (like most older ARM cores)
doesn't have a dedicated HW register for TLS. So I would like to avoid
any usage of threading. What is the argument against caching it inside
the application layer? E.g. is it fair to assume that journald will be
launched once systemd has created the cgroup?

I will try to get a perf report of the current journald. Cross-platform
perf report was broken the last time I tried.. maybe it has been fixed
since then. :}


holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] RFC: journald: Do not dynamically allocate _UID/_GID/_PID strings

2013-04-06 Thread Holger Hans Peter Freyther 
Avoid the dynamic allocation for the _UID, _GID, and _PID strings.
The maximum size of the string can be determined at compile time.

The code has only been compile tested.
---
 src/journal/journald-server.c |   22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index b97b9dc..ca07ace 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -535,8 +535,11 @@ static void dispatch_message_real(
 const char *label, size_t label_len,
 const char *unit_id) {
 
-char _cleanup_free_ *pid = NULL, *uid = NULL, *gid = NULL,
-*source_time = NULL, *boot_id = NULL, *machine_id = NULL,
+char pid[sizeof("_PID=")-1 + DECIMAL_STR_MAX(ucred->pid)];
+char uid[sizeof("_UID=")-1 + DECIMAL_STR_MAX(ucred->uid)];
+char gid[sizeof("_GID=")-1 + DECIMAL_STR_MAX(ucred->gid)];
+
+char _cleanup_free_ *source_time = NULL, *boot_id = NULL, *machine_id 
= NULL,
 *comm = NULL, *cmdline = NULL, *hostname = NULL,
 *audit_session = NULL, *audit_loginuid = NULL,
 *exe = NULL, *cgroup = NULL, *session = NULL,
@@ -562,14 +565,17 @@ static void dispatch_message_real(
 
 realuid = ucred->uid;
 
-if (asprintf(&pid, "_PID=%lu", (unsigned long) ucred->pid) >= 
0)
-IOVEC_SET_STRING(iovec[n++], pid);
+snprintf(pid, sizeof(pid) - 1, "_PID=%lu", (unsigned long) 
ucred->pid);
+char_array_0(pid);
+IOVEC_SET_STRING(iovec[n++], pid);
 
-if (asprintf(&uid, "_UID=%lu", (unsigned long) ucred->uid) >= 
0)
-IOVEC_SET_STRING(iovec[n++], uid);
+snprintf(uid, sizeof(uid) - 1, "_UID=%lu", (unsigned long) 
ucred->uid);
+char_array_0(uid);
+IOVEC_SET_STRING(iovec[n++], uid);
 
-if (asprintf(&gid, "_GID=%lu", (unsigned long) ucred->gid) >= 
0)
-IOVEC_SET_STRING(iovec[n++], gid);
+snprintf(gid, sizeof(gid) - 1, "_GID=%lu", (unsigned long) 
ucred->gid);
+char_array_0(gid);
+IOVEC_SET_STRING(iovec[n++], gid);
 
 r = get_process_comm(ucred->pid, &t);
 if (r >= 0) {
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] RFC: journald: Do not always record _AUDIT_SESSION and _AUDIT_LOGINUID

2013-04-06 Thread Holger Hans Peter Freyther 
When systemd was compiled without audit support, do not collect the
audit session and loginuid in the journal. This is saving a couple of
syscalls and memory allocations per log message.
---
 src/journal/journald-server.c |4 
 1 file changed, 4 insertions(+)

diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index a9d7aa1..b97b9dc 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -555,8 +555,10 @@ static void dispatch_message_real(
 assert(n + N_IOVEC_META_FIELDS <= m);
 
 if (ucred) {
+#ifdef HAVE_AUDIT
 uint32_t audit;
 uid_t loginuid;
+#endif
 
 realuid = ucred->uid;
 
@@ -596,6 +598,7 @@ static void dispatch_message_real(
 IOVEC_SET_STRING(iovec[n++], cmdline);
 }
 
+#ifdef HAVE_AUDIT
 r = audit_session_from_pid(ucred->pid, &audit);
 if (r >= 0)
 if (asprintf(&audit_session, "_AUDIT_SESSION=%lu", 
(unsigned long) audit) >= 0)
@@ -605,6 +608,7 @@ static void dispatch_message_real(
 if (r >= 0)
 if (asprintf(&audit_loginuid, "_AUDIT_LOGINUID=%lu", 
(unsigned long) loginuid) >= 0)
 IOVEC_SET_STRING(iovec[n++], audit_loginuid);
+#endif
 
 t = shortened_cgroup_path(ucred->pid);
 if (t) {
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 0/2] RFC for avoid dynamic allocation in journald

2013-04-06 Thread Holger Hans Peter Freyther 
The following two patches are compile tested only. The first one is
to avoid work when systemd is compiled without audit support. The
second is starting to remove dynamic allocations from the
dispatch_message_real method.



Variables that can easily be statically allocated:
_SYSTEMD_OWNER_UID
_SOURCE_REALTIME_TIMESTAMP


Variables that could benefit from a 'head room' (think of skb or
msgb in Osmocom):
_COMM
_EXE
_CMDLINE
_SYSTEMD_CGROUP
_SYSTEMD_UNIT
_SYSTEMD_USER_UNIT
_HOSTNAME

E.g. 

r = get_process_comm(ucred->pid, &t);
if (r >= 0) {
comm = strappend("_COMM=", t);
free(t);

if (comm)
IOVEC_SET_STRING(iovec[n++], comm);
}

could become..

r = get_process_comm_hr(ucred->pid, &t, STATIC_STRLEN("_COMM="));
if (r >= 0) {
PREPEND("_COMM=", t);
IOVEC_SET_STRING(iovec[n++], t);
}

or...

r = get_process_comm_prep(ucred->pid, &t, "_COMM=");
if (r >= 0) 
IOVEC_SET_STRING(iovec[n++], t);

and the get_process_comm would be an inline function to call
it with 0/NULL.



Variables that could be 'static':
_BOOT_ID
_MACHINE_ID

Are these allowed/expected to be changed during the runtime of the
journald?



Holger Hans Peter Freyther (2):
  RFC: journald: Do not always record _AUDIT_SESSION and
_AUDIT_LOGINUID
  RFC: journald: Do not dynamically allocate _UID/_GID/_PID strings

 src/journal/journald-server.c |   26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] RFC: util: Avoid memory allocations for formatting paths

2013-04-06 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

Avoid memory allocations to construct the path for files in the
procfs. The procfs paths are way shorter than the PATH_MAX so we
can use snprintf on a string located on the stack. This shows up
as a win on x86 using the benchmark program below.

$ make libsystemd-shared.la; gcc -O2 -Isrc/systemd/ -Isrc/ \
-o simple-perf-test simple-perf-test.c \
.libs/libsystemd-shared.a  -lrt

 #include "shared/util.h"
void test_once(void) {
pid_t pid = getpid();
char *tmp = NULL;

get_process_comm(pid, &tmp);
free(tmp);
tmp = NULL;
get_process_cmdline(pid, 0, 1, &tmp);
free(tmp);
is_kernel_thread(pid);
tmp = NULL;
get_process_exe(pid, &tmp);
free(tmp);
}

int main(int argc, char **argv)
{
int i;
for (i = 0; i < 5; ++i)
test_once();
}
---
 src/shared/util.c |   54 +
 1 file changed, 25 insertions(+), 29 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index 1bffd84..53ac41d 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -80,6 +80,16 @@ char **saved_argv = NULL;
 static volatile unsigned cached_columns = 0;
 static volatile unsigned cached_lines = 0;
 
+#define PROCFS_PATH_LEN (sizeof("/proc/")-1 + DECIMAL_STR_MAX(pid_t))
+
+#define FORMAT_PROCFS_PATH(buffer, path, pid)  
 \
+do {   
 \
+assert_cc(sizeof(buffer) == (PROCFS_PATH_LEN + 1 + 
sizeof(path)));  \
+snprintf(buffer, sizeof(buffer) - 1, "/proc/%lu/%s", (unsigned 
long) pid, path);\
+char_array_0(buffer);  
 \
+} while(0)
+
+
 size_t page_size(void) {
 static __thread size_t pgsz = 0;
 long r;
@@ -571,12 +581,9 @@ int get_process_comm(pid_t pid, char **name) {
 if (pid == 0)
 r = read_one_line_file("/proc/self/comm", name);
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/comm", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-r = read_one_line_file(p, name);
-free(p);
+char path[PROCFS_PATH_LEN + sizeof("/comm")];
+FORMAT_PROCFS_PATH(path, "comm", pid);
+r = read_one_line_file(path, name);
 }
 
 return r;
@@ -592,12 +599,9 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool 
comm_fallback, char *
 if (pid == 0)
 f = fopen("/proc/self/cmdline", "re");
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/cmdline", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-f = fopen(p, "re");
-free(p);
+char path[PROCFS_PATH_LEN + sizeof("/cmdline")];
+FORMAT_PROCFS_PATH(path, "cmdline", pid);
+f = fopen(path, "re");
 }
 
 if (!f)
@@ -684,7 +688,7 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool 
comm_fallback, char *
 }
 
 int is_kernel_thread(pid_t pid) {
-char *p;
+char path[PROCFS_PATH_LEN + sizeof("/cmdline")];
 size_t count;
 char c;
 bool eof;
@@ -693,11 +697,8 @@ int is_kernel_thread(pid_t pid) {
 if (pid == 0)
 return 0;
 
-if (asprintf(&p, "/proc/%lu/cmdline", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-f = fopen(p, "re");
-free(p);
+FORMAT_PROCFS_PATH(path, "cmdline", pid);
+f = fopen(path, "re");
 
 if (!f)
 return -errno;
@@ -722,12 +723,9 @@ int get_process_exe(pid_t pid, char **name) {
 if (pid == 0)
 r = readlink_malloc("/proc/self/exe", name);
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/exe", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-r = readlink_malloc(p, name);
-free(p);
+char path[PROCFS_PATH_LEN + sizeof("/exe")];
+FORMAT_PROCFS_PATH(path, "exe", pid);
+r = readlink_malloc(path, name);
 }
 
 return r;
@@ -735,7 +733,7 @@ int get_process_exe(pid_t pid, char **name) {
 
 static int get_process_id(pid_t pid, const char *field, uid_t *uid) {
 _cleanup_fclose_ FILE *f = NULL;
-_cleanup_free_ char *p = NULL;
+

Re: [systemd-devel] [PATCH 2/2] RFC: util: Avoid doing work on the string when it is not used

2013-04-05 Thread Holger Hans Peter Freyther 
On Fri, Apr 05, 2013 at 05:44:34AM +0200, Zbigniew Jędrzejewski-Szmek wrote:

Good Morning,

> It seems that this function must be invoked a bizillion times,
> so making it a bit leaner probably makes sense. Your implicit
> assumpition that the field is not prefixed by whitespace is
> almost certainly true in this case. But it would be better to
> mention it in the commit message. The strstrip on the line
> can simply be dropped. But notice that startswith returns
> the pointer to the end of the pattern (a relatively recent addition),
> so this can be simplified further.

True, I should have written about the assumption for the line start.
I will not revise the patch though as the _gid/_uid routines do not
appear to be used by the journald.

I just wanted to point out that for SELinux querying gid/uid at the
same time could make sense and that in general this loop is doing
more more work than is necessary.

cheers
holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] RFC: util: Avoid memory allocations for formatting paths

2013-04-03 Thread Holger Hans Peter Freyther 
On Wed, Apr 03, 2013 at 02:59:18PM +0200, Lennart Poettering wrote:
> On Wed, 03.04.13 08:27, Holger Freyther (hol...@freyther.de) wrote:
> 
> > +#define PROCFS_PATH_LEN (sizeof("/proc/")-1 + DECIMAL_STR_MAX(unsigned 
> > long))
> 
> Even though we use %lu to actually print the PID it's stil in the PID
> range, so I'd prefer using DECIMAL_STR_MAX(pid_t) here...

Well, I disagree but don't care enough to argue for unsigned long.


> Otherwise looks pretty good!

I will send an updated patch soon.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] High CPU usage of journald

2013-02-19 Thread Holger Hans Peter Freyther 
On Tue, Feb 19, 2013 at 03:27:02AM +0100, Lennart Poettering wrote:

> Well, to be fair: if an app just prints a "couple of log messages", then
> the functions you point out should hardly matter... Optimize inner
> loops, not just the stuff that happens to be called a "couple of"
> times...

I agree, with a "couple of log messages" journald shouldn't be anywhere
the CPU usage it is. Now my instruments are primitive, 'cross' perf
appears to be not implemented yet (another thing to put in the backlog) but
with what I have the _int_malloc sounds plausible. I spent years optimizing
WebKit and getting rid off a malloc was always a measurable improvement.


> No, the purpose of journald is not to allocate memory. It's primary
> purpose is actually to collect sarcastic comments by people. We thrive
> on that...


And it is called journald so you can keep track of the comments? ;)



> Does this mean I will now optimize them all away for you? No, not
> really, I don't even have the appropriate hardware to profile this. Does
> this mean I will merge good patches that optimize this? Hell, yes!

It works both ways. If I'm going to spend a significant amount of my time
to make journald usable on embedded hardware (right now my product doesn't
really need it) I want to have some kind of commitment that while I cut back
on dynamic allocations not a ton of new ones get introduced at the same time.

Just to be clear, when I talk about cutting back dynamic allocations I don't
mean introducing char foo[SHOULD_BE_BIG_ENOUGH_BUT_IS_NOT] but to be a bit
more clever about the usage. E.g. have a 'scratch' string, avoid reallocs
in steps of one, etc.

cheers
holger

PS: I thought Nokia hooked you up with a N900 or such?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] High CPU usage of journald

2013-02-18 Thread Holger Hans Peter Freyther 
On Mon, Feb 18, 2013 at 08:47:44AM -0800, David Strauss wrote:

Dear David, all,


> It's possible that a lot of it could also allocate on the stack or use
> stack-style management of a pool in heap. This is pretty
> performance-critical code, and we're seeing similar CPU overhead.

I think upstream doesn't care about the CPU overhead of the journald. I
had a quick look on my notebook with the same perf command and _int_malloc
is noticable in the trace as well.

In OpenBSC/Osmocom we have our struct msgb (a clone of struct skb) and
we can pull and push from the allocated memory. E.g. to replace a LayerX
header or prepend another header.


I browsed through journald-server.c:dispatch_message_real and there are
several things one could do... there are certainly a lot more items.


1.) Several things are done over and over again... e.g. calls to the
have_effective_cap.. it is unlikely that it will change after the journald
has started.


2.) Avoid allocation part one...

r = get_process_cmdline(ucred->pid, 0, false, &t);
if (r >= 0) {
cmdline = strappend("_CMDLINE=", t);
free(t);

if (cmdline)
IOVEC_SET_STRING(iovec[n++], cmdline);
}

So instead of the strappend (which will do strdup..) one could make sure
that 't' has enough space to prepend the _CMDLINE= or the _EXE, or the
other strings.


3.) Looking at code like... get_process_cmdline

  asprintf(&p, "/proc/%lu/cmdline", (unsigned long) pid)


So first of all /proc/%lu/ is created over and over again for exe, comm,
cmdline, sessionid, cgroup... With a different API one could easily avoid
this.

4.) Still at the same method..

if (max_length == 0) {
size_t len = 1;
while ((c = getc(f)) != EOF) {
k = realloc(r, len+1);
if (k == NULL) {

is most likely not the most effective way to handle the allocation. Or
maybe call get_process_cmdline with a max length?


5.) I wonder if some of the information could be sent from the systemd
to avoid the work in the journald...



I will end up commenting out most of dispatch_message_real and check if
the CPU down drops to a low single digit. I would assume it will.


regards
holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] sched: Only setting CPUSchedulingPriority=rr doesn't work

2012-11-01 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

A service that only sets the scheduling policy to round-robin
fails to be started. This is because the cpu_sched_priority is
initialized to 0 and is not adjusted when the policy is changed.

Clamp the cpu_sched_priority when the scheduler policy is set. Use
the current policy to validate the new priority.

Change the manual page to state that the given range only applies
to the real-time scheduling policies.

Add a testcase that verifies this change:

$ make test-sched-prio; ./test-sched-prio
[test/sched_idle_bad.service:6] CPU scheduling priority is out of range, 
ignoring: 1
[test/sched_rr_bad.service:7] CPU scheduling priority is out of range, 
ignoring: 0
[test/sched_rr_bad.service:8] CPU scheduling priority is out of range, 
ignoring: 100
---
 .gitignore   |1 +
 Makefile.am  |   25 +++-
 man/systemd.exec.xml |   14 ---
 src/core/load-fragment.c |   18 +++--
 src/test/test-sched-prio.c   |   86 ++
 test/sched_idle_bad.service  |6 +++
 test/sched_idle_ok.service   |6 +++
 test/sched_rr_bad.service|8 
 test/sched_rr_change.service |9 +
 test/sched_rr_ok.service |6 +++
 10 files changed, 168 insertions(+), 11 deletions(-)
 create mode 100644 src/test/test-sched-prio.c
 create mode 100644 test/sched_idle_bad.service
 create mode 100644 test/sched_idle_ok.service
 create mode 100644 test/sched_rr_bad.service
 create mode 100644 test/sched_rr_change.service
 create mode 100644 test/sched_rr_ok.service

diff --git a/.gitignore b/.gitignore
index 94a8542..00a10d1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -89,6 +89,7 @@
 /systemd
 /test-engine
 /test-job-type
+/test-sched-prio
 /systemctl
 /systemadm
 .dirstamp
diff --git a/Makefile.am b/Makefile.am
index 1c04047..15f5e1c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1180,7 +1180,8 @@ noinst_PROGRAMS += \
test-log \
test-unit-file \
test-date \
-   test-sleep
+   test-sleep \
+   test-sched-prio
 
 TESTS += \
test-job-type \
@@ -1189,7 +1190,15 @@ TESTS += \
test-unit-name \
test-unit-file \
test-date \
-   test-sleep
+   test-sleep \
+   test-sched-prio
+
+EXTRA_DIST += \
+   test/sched_idle_bad.service \
+   test/sched_idle_ok.service \
+   test/sched_rr_bad.service \
+   test/sched_rr_ok.service \
+   test/sched_rr_change.service
 
 test_engine_SOURCES = \
src/test/test-engine.c
@@ -1307,6 +1316,18 @@ test_watchdog_SOURCES = \
 test_watchdog_LDADD = \
libsystemd-shared.la
 
+test_sched_prio_SOURCES = \
+   src/test/test-sched-prio.c
+
+test_sched_prio_CFLAGS = \
+   $(AM_CFLAGS) \
+   $(DBUS_CFLAGS) \
+   -D"STR(s)=\#s" -D"TEST_DIR=STR($(abs_top_srcdir)/test/)"
+
+test_sched_prio_LDADD = \
+   libsystemd-core.la \
+   libsystemd-daemon.la
+
 # 
--
 systemd_initctl_SOURCES = \
src/initctl/initctl.c
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 7b65143..b684bfb 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -214,13 +214,15 @@
 
 Sets the CPU
 scheduling priority for executed
-processes. Takes an integer between 1
-(lowest priority) and 99 (highest
-priority). The available priority
+processes. The available priority
 range depends on the selected CPU
-scheduling policy (see above). See
-
sched_setscheduler2
-for details.
+scheduling policy (see above). For
+real-time scheduling policies an
+integer between 1 (lowest priority)
+and 99 (highest priority) can be used.
+See 
sched_setscheduler2
+for details.
+
 
 
 
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 5803044..411be7b 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -4,6 +4,7 @@
   This file is part of systemd.
 
   Copyright 2010 Lennart Poettering
+  Copyright 2012 Holger Hans Peter Freyther
 
   systemd is free software; you can redistribute it and/or modify it
   under the terms of the GNU Lesser General Public License as published by
@@ -683,6 +684,8 @@ int config_parse_exec_cpu_sched_policy(
 }
 
 c->cpu_sched_policy = x;
+/* Moving to or from real-time policy? We need to adjust the

[systemd-devel] [PATCH 1/2] man: typo fix

2012-11-01 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

---
 man/sd-id128.xml |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/sd-id128.xml b/man/sd-id128.xml
index abd2004..ac2000e 100644
--- a/man/sd-id128.xml
+++ b/man/sd-id128.xml
@@ -112,7 +112,7 @@
 #define SD_MESSAGE_COREDUMP 
SD_ID128_MAKE(fc,2e,22,bc,6e,e6,47,b6,b9,07,29,ab,34,a2,50,b1)
 
 SD_ID128_CONST_STR() may be
-use to convert constant 128bit IDs into constant
+used to convert constant 128bit IDs into constant
 strings for output. The following example code will
 output the string
 "fc2e22bc6ee647b6b90729ab34a250b1":
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] man: /usr/local/lib is searched for files too, mention it.

2012-09-19 Thread Holger Hans Peter Freyther 
On Wed, Sep 19, 2012 at 02:20:42PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> On Wed, Sep 19, 2012 at 12:02:55PM +0200, Holger Hans Peter Freyther wrote:
> Hi,

Hey,

>  Files in /etc/ override files with the same name in /usr/lib/ and
>  /run/. Files in /run/ override files with the same name in /usr/lib/.

is there a mechanism (entities? includes?) to share this kind of information?
E.g. the tmpfiles.xml has a rather comprehensive description of how the override
is working?

holger
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] man: /usr/local/lib is searched for files too, mention it.

2012-09-19 Thread Holger Hans Peter Freyther 
From: Holger Hans Peter Freyther 

---
 man/binfmt.d.xml   |1 +
 man/modules-load.d.xml |1 +
 man/sysctl.d.xml   |1 +
 man/systemd.preset.xml |2 ++
 man/tmpfiles.d.xml |1 +
 5 files changed, 6 insertions(+)

diff --git a/man/binfmt.d.xml b/man/binfmt.d.xml
index 07ae0ac..791a25a 100644
--- a/man/binfmt.d.xml
+++ b/man/binfmt.d.xml
@@ -49,6 +49,7 @@
 
 /etc/binfmt.d/*.conf
 /run/binfmt.d/*.conf
+
/usr/local/lib/binfmt.d/*.conf
 /usr/lib/binfmt.d/*.conf
 
 
diff --git a/man/modules-load.d.xml b/man/modules-load.d.xml
index bcc4d12..b15c986 100644
--- a/man/modules-load.d.xml
+++ b/man/modules-load.d.xml
@@ -48,6 +48,7 @@
 
 /etc/modules-load.d/*.conf
 /run/modules-load.d/*.conf
+
/usr/local/lib/modules-load.d/*.conf
 
/usr/lib/modules-load.d/*.conf
 
 
diff --git a/man/sysctl.d.xml b/man/sysctl.d.xml
index 69aac8c..df22461 100644
--- a/man/sysctl.d.xml
+++ b/man/sysctl.d.xml
@@ -48,6 +48,7 @@
 
 /etc/sysctl.d/*.conf
 /run/sysctl.d/*.conf
+
/usr/local/lib/sysctl.d/*.conf
 /usr/lib/sysctl.d/*.conf
 
 
diff --git a/man/systemd.preset.xml b/man/systemd.preset.xml
index a692053..0297972 100644
--- a/man/systemd.preset.xml
+++ b/man/systemd.preset.xml
@@ -48,9 +48,11 @@
 
 
/etc/systemd/system-preset/*.preset
 
/run/systemd/system-preset/*.preset
+
/usr/local/lib/systemd/system-preset/*.preset
 
/usr/lib/systemd/system-preset/*.preset
 
/etc/systemd/user-preset/*.preset
 
/run/systemd/user-preset/*.preset
+
/usr/local/lib/systemd/user-preset/*.preset
 
/usr/lib/systemd/user-preset/*.preset
 
 
diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index a86ef33..113b786 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -49,6 +49,7 @@
 
 /etc/tmpfiles.d/*.conf
 /run/tmpfiles.d/*.conf
+
/usr/local/lib/tmpfiles.d/*.conf
 /usr/lib/tmpfiles.d/*.conf
 
 
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel