Re: [systemd-devel] [PATCH 1/2] RFC: journald: Allow to cache the cg_get_root_path
On 07/16/2013 03:26 AM, Lennart Poettering wrote: On Mon, 08.07.13 18:39, Karol Lewandowski (k.lewando...@samsung.com) wrote: According to my analysis /proc access is costly and it would be best to cache the result for later use. Difficulty comes from trying to keep cache up to date, though. We can't really cache this. This stuff is already racy, as by the time we read the attributes the process might already be gone. 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. I think the best way to deal with the performance issue here is the stuff discussed here: https://bugzilla.redhat.com/show_bug.cgi?id=963620 i.e. just have the kernel augment our messages with the data we need, unconditionally. That way we fix both the race issue, and the performance issue, since the data is just there and we can make use of it without any further work. Thanks a lot for this link! Having this issue sorted out will allow me to take closer look into eliminating 64-bit divisions in message receive path. ARM doesn't support divide operation so every div[1] is being done purely in software. [1] Every div/% by non-constant value and every div/% by 64-bit constant results in call to (slow) __aeabi_div*. Thanks ___ 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
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
Re: [systemd-devel] [PATCH 1/2] RFC: journald: Allow to cache the cg_get_root_path
On Thu, 27.06.13 18:30, Holger Hans Peter Freyther (hol...@freyther.de) wrote: From: Holger Hans Peter Freyther hol...@moiji-mobile.com 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. Sorry for the delay in reviewing. THe general approach of caching the result of cg_get_root_path() is a good one, but I am not fond of placing this in a static variable. We try to avoid that where possible, especially if we have some kind of context object around anyway where we could place this. +if (!cg_root) +return -1; We use negative errno-style errors everywhere for indicating errors, not -1. i.e. please use return -EINVAL or suchlike, but not literal numeric constants. Thanks, Lennart -- Lennart Poettering - Red Hat, Inc. ___ 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
On Mon, 08.07.13 18:39, Karol Lewandowski (k.lewando...@samsung.com) wrote: On 06/27/2013 06:30 PM, Holger Hans Peter Freyther wrote: From: Holger Hans Peter Freyther hol...@moiji-mobile.com 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. My 2c, I have been thinking about something similar albeit a bit more generic. According to my analysis /proc access is costly and it would be best to cache the result for later use. Difficulty comes from trying to keep cache up to date, though. We can't really cache this. This stuff is already racy, as by the time we read the attributes the process might already be gone. I think the best way to deal with the performance issue here is the stuff discussed here: https://bugzilla.redhat.com/show_bug.cgi?id=963620 i.e. just have the kernel augment our messages with the data we need, unconditionally. That way we fix both the race issue, and the performance issue, since the data is just there and we can make use of it without any further work. I've just started looking into connector's cn_proc which _seem_ to offer all the required functionality (fork, exec, exit notifications). I have to finish my prototype to verify if it's worth complications it brings. cn_proc is asynchronous, so you get even more race problems like this, where processes are already gone by the time you get the message about them. Lennart -- Lennart Poettering - Red Hat, Inc. ___ 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
On 06/27/2013 06:30 PM, Holger Hans Peter Freyther wrote: From: Holger Hans Peter Freyther hol...@moiji-mobile.com 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. My 2c, I have been thinking about something similar albeit a bit more generic. According to my analysis /proc access is costly and it would be best to cache the result for later use. Difficulty comes from trying to keep cache up to date, though. I've just started looking into connector's cn_proc which _seem_ to offer all the required functionality (fork, exec, exit notifications). I have to finish my prototype to verify if it's worth complications it brings. Thanks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
