Re: [systemd-devel] Fwd: [Pkg-systemd-maintainers] Bug#732157: Want SIGSTOP-style daemon/service readiness notification
If we supported PIDFile= for Type=simple, daemons could drop a PID file to indicate startup completion without having to be full-on Type=forking. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] libsystemd-bus: make sure buf transfered into kenrel is 8 aligned
From: Yin Kangkai kangkai@intel.com
Otherwise, for example hello arg passed to KDBUS_CMD_HELLO might not be 8 bytes
aligned and kernel returns -EFAULT.
319 int bus_kernel_take_fd(sd_bus *b) {
320 struct kdbus_cmd_hello hello;
(gdb) p hello
$8 = (struct kdbus_cmd_hello *) 0xb354
---
src/libsystemd-bus/bus-control.c | 4 ++--
src/libsystemd-bus/bus-kernel.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/libsystemd-bus/bus-control.c b/src/libsystemd-bus/bus-control.c
index 0072c37..6b2790d 100644
--- a/src/libsystemd-bus/bus-control.c
+++ b/src/libsystemd-bus/bus-control.c
@@ -207,7 +207,7 @@ _public_ int sd_bus_release_name(sd_bus *bus, const char
*name) {
}
static int kernel_get_list(sd_bus *bus, uint64_t flags, char ***x) {
-struct kdbus_cmd_name_list cmd = {};
+struct kdbus_cmd_name_list __attribute__ ((__aligned__(8))) cmd = {};
struct kdbus_name_list *name_list;
struct kdbus_cmd_name *name;
uint64_t previous_id = 0;
@@ -1088,7 +1088,7 @@ static int bus_remove_match_internal_kernel(
const char *match,
uint64_t cookie) {
-struct kdbus_cmd_match m;
+struct kdbus_cmd_match __attribute__ ((__aligned__(8))) m;
int r;
assert(bus);
diff --git a/src/libsystemd-bus/bus-kernel.c b/src/libsystemd-bus/bus-kernel.c
index 0e47308..4947d39 100644
--- a/src/libsystemd-bus/bus-kernel.c
+++ b/src/libsystemd-bus/bus-kernel.c
@@ -317,7 +317,7 @@ fail:
}
int bus_kernel_take_fd(sd_bus *b) {
-struct kdbus_cmd_hello hello;
+struct kdbus_cmd_hello __attribute__ ((__aligned__(8))) hello;
int r;
assert(b);
--
1.8.2.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Why doe I not see the logging with -u
On 12/14/2013 09:22 AM, Cecil Westerhof wrote: I made a first setup to make a service for the H2 database. I made the folowing service file: [Unit] Description=H2 Database [Service] Type=simple ExecStart=/usr/bin/java -cp /home/cecil/java/h2/bin/h2-1.3.174.jar org.h2.tools.Console -tool -tcp Restart=always User=cecil [Install] WantedBy=multi-user.target After starting and stopping I got with ‘journalctl -u h2’: -- Logs begin at Tue, 2013-11-19 06:24:51 CET, end at Sat, 2013-12-14 09:15:01 CET. -- Dec 13 11:37:40 Equus.Decebal.nl java[20909]: Web Console server running at http://127.0.0.2:8082 (only local connections) Dec 13 11:37:40 Equus.Decebal.nl java[20909]: TCP server running at tcp://127.0.0.2:9092 (only local connections) Dec 13 11:42:21 Equus.Decebal.nl java[20975]: Web Console server running at http://127.0.0.2:8082 (only local connections) Dec 13 11:42:21 Equus.Decebal.nl java[20975]: TCP server running at tcp://127.0.0.2:9092 (only local connections) Dec 13 11:42:59 Equus.Decebal.nl java[21031]: Web Console server running at http://127.0.0.2:8082 (only local connections) Dec 13 11:42:59 Equus.Decebal.nl java[21031]: TCP server running at tcp://127.0.0.2:9092 (only local connections) Dec 13 11:43:06 Equus.Decebal.nl java[21068]: Web Console server running at http://127.0.0.2:8082 (only local connections) Dec 13 11:43:06 Equus.Decebal.nl java[21068]: TCP server running at tcp://127.0.0.2:9092 (only local connections) lines 1-9/9 (END) I made a VM with openSUSE 13.1 (which has version 208) and there I get: Dec 16 11:04:07 linux-r4lo.site systemd[1]: Starting H2 Database... Dec 16 11:04:07 linux-r4lo.site systemd[1]: Started H2 Database. Dec 16 11:04:07 linux-r4lo.site java[5521]: Web Console server running at http://127.0.0.2:8082 (only local connections) Dec 16 11:04:08 linux-r4lo.site java[5521]: TCP server running at tcp://127.0.0.2:9092 (only local connections) Dec 16 11:04:39 linux-r4lo.site systemd[1]: Stopping H2 Database... Dec 16 11:04:39 linux-r4lo.site systemd[1]: h2.service: main process exited, code=exited, status=143/n/a Dec 16 11:04:39 linux-r4lo.site systemd[1]: Stopped H2 Database. Dec 16 11:04:39 linux-r4lo.site systemd[1]: Unit h2.service entered failed state. So it looks like there was/is a problem with the 195 version. Time to upgrade my system. Luckily the holiday season is almost there. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fwd: [Pkg-systemd-maintainers] Bug#732157: Want SIGSTOP-style daemon/service readiness notification
'Twas brillig, and David Timothy Strauss at 16/12/13 08:04 did gyre and gimble: If we supported PIDFile= for Type=simple, daemons could drop a PID file to indicate startup completion without having to be full-on Type=forking. Yeah but pidfile is kinda ugly too and has problem when processes are run as other users (requiring corresponding tmpfiles to create appropriately name directores). The amount of code/hassle for proper pid file handling is surely more complex than sd-notify support? 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
[systemd-devel] Does not start in graphical mode
I made a openSUSE 13.1 VM in virtualbox. I start it in graphical mode. But I do not get a X environment. With journalctl I see that it is reached: Dec 16 12:18:33 linux-r4lo.site systemd[1]: Starting Graphical Interface. Dec 16 12:18:33 linux-r4lo.site systemd[1]: Reached target Graphical Interface. But only after giving: init 5 I get a X environment. With ‘journalctl -b -p err’ I see: Dec 16 12:18:17 linux-r4lo kernel: piix4_smbus :00:07.0: SMBus base address uninitialized - upgrade BIOS or use force_addr=0xaddr Dec 16 12:18:34 linux-r4lo.site killproc[1170]: killproc: Usage: killproc [-v] [-q] [-L] [-g|-G] [-N] [-p pid_file] [-i ingnore_file] \ [-c root] [-tsec] [-SIG] /full/path/to/executable killproc -l Anybody an idea what could be happening here, or how to get information to find out what is happening? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Does not start in graphical mode
'Twas brillig, and Cecil Westerhof at 16/12/13 11:32 did gyre and gimble: I made a openSUSE 13.1 VM in virtualbox. I start it in graphical mode. But I do not get a X environment. With journalctl I see that it is reached: Dec 16 12:18:33 linux-r4lo.site systemd[1]: Starting Graphical Interface. Dec 16 12:18:33 linux-r4lo.site systemd[1]: Reached target Graphical Interface. But only after giving: init 5 I get a X environment. With ‘journalctl -b -p err’ I see: Dec 16 12:18:17 linux-r4lo kernel: piix4_smbus :00:07.0: SMBus base address uninitialized - upgrade BIOS or use force_addr=0xaddr Dec 16 12:18:34 linux-r4lo.site killproc[1170]: killproc: Usage: killproc [-v] [-q] [-L] [-g|-G] [-N] [-p pid_file] [-i ingnore_file] \ [-c root] [-tsec] [-SIG] /full/path/to/executable killproc -l Anybody an idea what could be happening here, or how to get information to find out what is happening? This is really not a systemd issue nor an issue you should seek specific help from the systemd community. As systemd is responsible for starting all tasks, it can sometimes be hard to work out where things are going wrong. But as you are using opensuse for a base, I would start over on an opensuse specific mailing list or forum and only raise it here if you know it's a systemd bug. In this case it's looking very much like a problem at a more local level. Use systemctl to see what services have failed (I don't know if opensuse is still using the generic prefdm or individual units to start the DM), try and track down where the invalid use of killproc is specified (likely in a unit or script somewhere) and go on from there. 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] Does not start in graphical mode
Le lundi 16 décembre 2013 à 12:32 +0100, Cecil Westerhof a écrit : I made a openSUSE 13.1 VM in virtualbox. I start it in graphical mode. But I do not get a X environment. With journalctl I see that it is reached: Dec 16 12:18:33 linux-r4lo.site systemd[1]: Starting Graphical Interface. Dec 16 12:18:33 linux-r4lo.site systemd[1]: Reached target Graphical Interface. But only after giving: init 5 I get a X environment. With ‘journalctl -b -p err’ I see: Dec 16 12:18:17 linux-r4lo kernel: piix4_smbus :00:07.0: SMBus base address uninitialized - upgrade BIOS or use force_addr=0xaddr Dec 16 12:18:34 linux-r4lo.site killproc[1170]: killproc: Usage: killproc [-v] [-q] [-L] [-g|-G] [-N] [-p pid_file] [-i ingnore_file] \ [-c root] [-tsec] [-SIG] /full/path/to/executable killproc -l Anybody an idea what could be happening here, or how to get information to find out what is happening? Something is using killproc incorrectly.. You should look at the journalctl output more closely to find which service is sending this error. -- Frederic Crozat fcro...@suse.com SUSE ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] getty : how to run getty on every ttyX
On Fri, Dec 13, 2013 at 05:20:19PM +0100, Lennart Poettering wrote: On Fri, 13.12.13 16:15, Lennart Poettering (lenn...@poettering.net) wrote: We had discussed this back at Linux Plumbers last year, and at the time you had suggested that rather than create /dev/ttyN symlinks we should instead do something like /dev/containerttyN instead, and set a 'container_tty' variable containing a list of all those device names so that systemd can discover them sensibly. We never got around to doing this from the libvirt side, and AFAIK systemd hasn't done anything on its side either. So is this still a suitable way forward ? Yeah, I am pretty sure that's what we should do. I figure I should hack that up. I'll work on it now. Committed. systemd-getty-generator will now look for $container_ttys set as an environment variable for PID 1. If that is set it will split the string up on whitespaces and start a getty on all ptys referenced. Note that this only supports ptys, not any other ttys. Example: container_ttys=pts/5 pts/8 pts/15 when pass to PID 1 will spawn three additional gettys on ptys 5, 8 and 15. Note that this *really* only supports ptys, not any other kinds of ttys, sinc for those we require propery device enumeration and notification and we don't have those in containers... I still chose to name this $container_ttys rather than $container_ptys, so that maybe one day we can extend it should devices like this ever get virtualized. This will be in systemd 209. I've tested this with libvirt and it worked except for one small edge case. Say libvirt creates 3 consoles /dev/pts/0, /dev/pts/1 and /dev/pts/2. Now we set container_ttys=pts/0 pts/1 pts/2 Systemd starts up 3 agetty processes - one of each of these. The /dev/console device, however, is also a link to /dev/pts/0 and so systemd starts up a agetty process for that too. Now we have 2 agetty processes fighting over /dev/pts/0 which ends in tears Is this something that systemd should detect cope with, or should we document that the 'container_ttys' env *must exclude* any tty associated with the /dev/console device ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On 12/14/2013 05:12 PM, Zbigniew Jędrzejewski-Szmek wrote: On Fri, Dec 13, 2013 at 10:16:16PM +0100, Karol Lewandowski wrote: One of the problems I see, though, is that no matter how deep I make the queue (`max_dgram_qlen') I still see process sleeping on send() way earlier that configured queue depth would suggest. Are you sure that the sysctl is set early enough, before the listening socket is created? I'm doing most of my testing in nspawn-able container, so it's ok here. I will take special precautions to make sure it's set early enough on our handheld, though. Thanks for the hint! Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] journal: How to limit the file size of runtime system.journal
On Mon, Dec 16, 2013 at 08:31:46AM +0100, Holger Winkelmann [TP] wrote: Hi, Is there any particular reason? I think thresold for runtime journal size can lower much because in initramfs it's not supposed to have much logs. First, there are some data strcutures which are allocated when the file is created, and if the file was very small, relatively more space would wasted. Second, repeated fields are not stored, just referenced, so things become more efficient when the file is not too small. But neither is fundamental reason, and with some tweaking the journal could be made to work much smaller files. I understand. These are really good points when logs are relatively large, ie. the journal is stored on a real disk. However when it's in initramfs context, journal is stored in tmpfs which is using the real memory resource as it's backend. 4 MB seems a little bit overkill especially when memory is quite limited case, like kdump. To be more specific, I think 512 KB or 1 MB is a fairly large enough nubmer when journal is stored to a volatile backend. We totally agree that a minimum size must be below 1MB either on flash or ramfs for embedded devices. otherwise you end up with two solutions for smaller and bigger devices. Is there any reference about the overhead if you use smaller file size? Is there technical limitation for a minimum size? No, there's no real technical limitation. Except some hero should go through src/jounal/journal-file.c and adjust all the constants that they also work with very small files. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Does not start in graphical mode
On 12/16/2013 12:32 PM, Cecil Westerhof wrote: I made a openSUSE 13.1 VM in virtualbox. I start it in graphical mode. But I do not get a X environment. With journalctl I see that it is reached: Dec 16 12:18:33 linux-r4lo.site systemd[1]: Starting Graphical Interface. Dec 16 12:18:33 linux-r4lo.site systemd[1]: Reached target Graphical Interface. But only after giving: init 5 I get a X environment. With ‘journalctl -b -p err’ I see: Dec 16 12:18:17 linux-r4lo kernel: piix4_smbus :00:07.0: SMBus base address uninitialized - upgrade BIOS or use force_addr=0xaddr Dec 16 12:18:34 linux-r4lo.site killproc[1170]: killproc: Usage: killproc [-v] [-q] [-L] [-g|-G] [-N] [-p pid_file] [-i ingnore_file] \ [-c root] [-tsec] [-SIG] /full/path/to/executable killproc -l Anybody an idea what could be happening here, or how to get information to find out what is happening? I have delved a little deeper. It has nothing to do with the killproc. (But I should of-course also look into that. Looks like something from virtualbox.) When looking at the log, I see the following: - Graphical is started and reached - ‘Stop Read-Ahead Data’ is started. - ‘Update UTMP’ is started. - Then the startup is finished. - About half a second later lightdm is closed, before the killproc. I'll ask about it on an openSUSE mailinglist. For people interested in the logging: Dec 16 12:18:33.735477 linux-r4lo.site systemd[1]: Reached target Multi-User System. Dec 16 12:18:33.735525 linux-r4lo.site systemd[1]: Starting Graphical Interface. Dec 16 12:18:33.735548 linux-r4lo.site systemd[1]: Reached target Graphical Interface. Dec 16 12:18:33.735604 linux-r4lo.site systemd[1]: Starting Stop Read-Ahead Data Collection 10s After Completed Startup. Dec 16 12:18:33.735634 linux-r4lo.site systemd[1]: Started Stop Read-Ahead Data Collection 10s After Completed Startup. Dec 16 12:18:33.735683 linux-r4lo.site systemd[1]: Starting Update UTMP about System Runlevel Changes... Dec 16 12:18:33.741268 linux-r4lo.site systemd[1]: Started Update UTMP about System Runlevel Changes. Dec 16 12:18:33.742272 linux-r4lo.site systemd[1]: Startup finished in 2.150s (kernel) + 19.939s (userspace) = 22.090s. Dec 16 12:18:33.801463 linux-r4lo.site lightdm[749]: pam_unix(lightdm-greeter:session): session closed for user lightdm Dec 16 12:18:33.809914 linux-r4lo.site systemd-logind[578]: Removed session 1. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On 12/14/2013 04:47 AM, Lennart Poettering wrote: On Fri, 13.12.13 22:16, Karol Lewandowski (lmc...@gmail.com) wrote: On Fri, Dec 13, 2013 at 03:45:36PM +0100, Lennart Poettering wrote: On Fri, 13.12.13 12:46, Karol Lewandowski (k.lewando...@samsung.com) wrote: One of the problems I see, though, is that no matter how deep I make the queue (`max_dgram_qlen') I still see process sleeping on send() way earlier that configured queue depth would suggest. It would be interesting to find out why this happens. I mean, there are three parameters here I could think of that matter: the qlen, SO_SNDBUF on the sender, and SO_RCVBUF on the receiver (though the latter two might actually change the same value on AF_UNIX? or maybe one of the latter two is a NOP on AF_UNIX?). If any of them reaches the limit then the sender will necessarily have to block. (SO_SNDBUF and SO_RCVBUF can also be controlled via /proc/sys/net/core/rmem* and ../wmem*... For testing purposes it might be easier to play around with these and set them to ludicrously high values...) That's it. While journal code tries to set buffer size via SO_SNDBUF/SO_RCVBUF options to 8MB, kernel limits these to wmem_max/rmem_max. On machines I've tested respective values are quite small - around 150-200kB each. Increasing these did reduce context switches considerably - preliminary tests show that I can now queue thousands of messages (~5k) without problems. I will test this thoroughly in next few days. I do wonder what is the rationale behind such low limits... Thanks a lot! Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fwd: [Pkg-systemd-maintainers] Bug#732157: Want SIGSTOP-style daemon/service readiness notification
On Mon, 16.12.13 00:04, David Timothy Strauss (da...@davidstrauss.net) wrote: If we supported PIDFile= for Type=simple, daemons could drop a PID file to indicate startup completion without having to be full-on Type=forking. There has been a TODO list item for a long time to introduce Type=pid-file which would work like this. I am a bit concerned though that people would just blindly make use of it without actually verifying that their specific daemon really creates the PID file as last part of initialization. But maybe that concern can be dealt with with an explicit warning in the man page. Anyway, I'd be open for such a patch, 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] libsystemd-bus: make sure buf transfered into kenrel is 8 aligned
On Mon, Dec 16, 2013 at 4:01 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Mon, 16.12.13 15:50, Lennart Poettering (lenn...@poettering.net) wrote:
diff --git a/src/libsystemd-bus/bus-control.c
b/src/libsystemd-bus/bus-control.c
index 0072c37..6b2790d 100644
--- a/src/libsystemd-bus/bus-control.c
+++ b/src/libsystemd-bus/bus-control.c
@@ -207,7 +207,7 @@ _public_ int sd_bus_release_name(sd_bus *bus, const
char *name) {
}
static int kernel_get_list(sd_bus *bus, uint64_t flags, char ***x) {
-struct kdbus_cmd_name_list cmd = {};
+struct kdbus_cmd_name_list __attribute__ ((__aligned__(8)))
cmd = {};
Hmm, this feels a bit like this would be better part of the type rather
than the variable. THus, kdbus.h should add this to all is structs,
rather then we decorate the variables...
Kay, would this make sense to you to add to kdbus.h?
Hmm, so thinking about this: the kdbus_cmd_name_list structure contains
64bit values anyway, so should naturally be aligned to 64bit boundaries
anyway... Or am I mistaken there and you are suggesting that on your
32bit architecture (which one is it if I may ask?) 64bit values don't
have to be aligned on an even 8byte boundary, but instead because the
arch is 32bit anyway and thus 64bit values need to be read in two steps
alignment on 4 is done in the abi?
We could try to define size, which is the first member in the struct, as:
__aligned_u64
instead of the current:
u64
That might do the trick. Could you test that if it works on your
platform/setup instead of patching the lib?
Thanks,
Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] prioq: avoid to swap item index
On Mon, 16.12.13 13:15, Yang Chengwei (chengwei.y...@intel.com) wrote: On Mon, Dec 16, 2013 at 04:57:39AM +0100, Lennart Poettering wrote: On Mon, 16.12.13 04:48, Lennart Poettering (lenn...@poettering.net) wrote: On Mon, 16.12.13 11:03, Chengwei Yang (chengwei.y...@intel.com) wrote: the swap() operation of prioq which in fact only swap item's data but keep idx untouched. However, current implement does first swap the idx and then swap back again. Sorry, I do understand? Can you elaborate, please? Is this supposed to wanted to say: I do *not* understand... It's an optimization. A condition of the queue is just like the assert says assert(!q-items[j].idx || *(q-items[j].idx) == j); assert(!q-items[k].idx || *(q-items[k].idx) == k); This is true before and after swap. So in fact no need to swap idx and then swap back again. Just do not touch idx, then item[j].idx is always 0 or j, so as item[k]. The deleted code does 1. first swap the idx regardless it's 0 or not 2. then, if idx isn't 0, swap back. So this patch is a optimization for the case where item.idx isn't 0, which avoid to swap idx and swap idx back. I'm not sure this is clear enough. In simple, the fact is *before* and *after* we need make sure item[j].idx == j or item[j].idx == 0, so why we change it during swap? Because if it changed during swap, we need then assign item[j].idx = j before the swap finish. So the simple way is keep it untouched. Sorry, still not getting what you want to say. Mayb ethere is some confusion regarding what .idx actually is? .idx is supposed to point to some index integer that is stored in the actual structure the user added to the priority queue and that can be used to quickly remove an entry from the priority queue without requiring it to be the first one. In the swap() call we hence first swap the the data and idx pointers themselves, and then in a second step we finally update what the idx pointers actually point to to the new index of the item in our priority queue. I totally don't see how any of that was redundant. We must make sure after all that after the swap: a) For both entries we know that the data pointer has been swapped b) For both entries we know that the pointer to the index value that is part of the user structure has been swapped c) For both entries we know that the index value that is part of the user structure has been swapped 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] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On Mon, 16.12.13 15:36, Karol Lewandowski (k.lewando...@samsung.com) wrote: On 12/14/2013 04:47 AM, Lennart Poettering wrote: On Fri, 13.12.13 22:16, Karol Lewandowski (lmc...@gmail.com) wrote: On Fri, Dec 13, 2013 at 03:45:36PM +0100, Lennart Poettering wrote: On Fri, 13.12.13 12:46, Karol Lewandowski (k.lewando...@samsung.com) wrote: One of the problems I see, though, is that no matter how deep I make the queue (`max_dgram_qlen') I still see process sleeping on send() way earlier that configured queue depth would suggest. It would be interesting to find out why this happens. I mean, there are three parameters here I could think of that matter: the qlen, SO_SNDBUF on the sender, and SO_RCVBUF on the receiver (though the latter two might actually change the same value on AF_UNIX? or maybe one of the latter two is a NOP on AF_UNIX?). If any of them reaches the limit then the sender will necessarily have to block. (SO_SNDBUF and SO_RCVBUF can also be controlled via /proc/sys/net/core/rmem* and ../wmem*... For testing purposes it might be easier to play around with these and set them to ludicrously high values...) That's it. While journal code tries to set buffer size via SO_SNDBUF/SO_RCVBUF options to 8MB, kernel limits these to wmem_max/rmem_max. On machines I've tested respective values are quite small - around 150-200kB each. Hmm, so on the journald's side we actually use SO_RCVBUFFORCE to override that kernel limit. If I understood you correctly though then SO_SNDBUF on the sending side is the issue here, not SO_RCVBUF on the receiving side. We could certainly update src/journal/journal-send.c to also use SO_SNDBUFFORCE on the client side, but that would leave unpriviliged clients and traditional /dev/log clients in the cold, since the SO_SNDBUFFORCE requires privs, and the client side for /dev/log lives in glibc, not in systemd. Increasing these did reduce context switches considerably - preliminary tests show that I can now queue thousands of messages (~5k) without problems. I will test this thoroughly in next few days. I do wonder what is the rationale behind such low limits... Well, usually the logic is to keep things conservative until you notice that this creates issues. To fix this properly, and comprehensively I'd really like to see three changes in the kernel: - Introduce a pair of SO_QLEN and SO_QLENFORCE sockopts to the kernel so that we can set the qlen per-socket - Make the defaults for the rwmem configurable at kernel compile time - Increase the defaults of the kernel 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] systemd + logind possible deadlock
It's not hangin', just pretendin' :) What y'all think about a little patch like this? Probably this is better than nothing, but will it fix anything? That stuff hangs not for 1 minute, but forever ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On Mon, 16.12.13 16:54, Lennart Poettering (lenn...@poettering.net) wrote: That's it. While journal code tries to set buffer size via SO_SNDBUF/SO_RCVBUF options to 8MB, kernel limits these to wmem_max/rmem_max. On machines I've tested respective values are quite small - around 150-200kB each. Hmm, so on the journald's side we actually use SO_RCVBUFFORCE to override that kernel limit. If I understood you correctly though then SO_SNDBUF on the sending side is the issue here, not SO_RCVBUF on the receiving side. We could certainly update src/journal/journal-send.c to also use SO_SNDBUFFORCE on the client side, but that would leave unpriviliged clients and traditional /dev/log clients in the cold, since the SO_SNDBUFFORCE requires privs, and the client side for /dev/log lives in glibc, not in systemd. I made such a change in git now. But again, this is only a very partial solution, as it only covers native clients with priviliges. It does not cover unpriviliged clients or traditional syslog() clients... Also, it cannot influence the qlen. But I fear the rest of this really needs to be fixed in the kernel (and glibc), we cannot do much about this from the systemd side... 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] systemd + logind possible deadlock
On Mon, 16.12.13 18:14, Oleksii Shevchuk (alx...@gmail.com) wrote: It's not hangin', just pretendin' :) What y'all think about a little patch like this? Probably this is better than nothing, but will it fix anything? That stuff hangs not for 1 minute, but forever Are you suggesting O_SNDTIMEO doesn't work for you? Normally it should simply cause the send() to fail if the receiver doesn't take the packet within some time. 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] systemd + logind possible deadlock
Are you suggesting O_SNDTIMEO doesn't work for you? Normally it should simply cause the send() to fail if the receiver doesn't take the packet within some time. I don't take much investigation into this, but looks like systemd-journald dead for some reason, and stay in zombie state, because of systemd locking. Maybe send returned -1, but for some reason this doesn't change things a bit - systemd is inaccessible my systemctl, and it doesn't do any work. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] getty : how to run getty on every ttyX
On Mon, 16.12.13 12:03, Daniel P. Berrange (berra...@redhat.com) wrote: Note that this *really* only supports ptys, not any other kinds of ttys, sinc for those we require propery device enumeration and notification and we don't have those in containers... I still chose to name this $container_ttys rather than $container_ptys, so that maybe one day we can extend it should devices like this ever get virtualized. This will be in systemd 209. I've tested this with libvirt and it worked except for one small edge case. Say libvirt creates 3 consoles /dev/pts/0, /dev/pts/1 and /dev/pts/2. Now we set container_ttys=pts/0 pts/1 pts/2 Systemd starts up 3 agetty processes - one of each of these. The /dev/console device, however, is also a link to /dev/pts/0 and so systemd starts up a agetty process for that too. Now we have 2 agetty processes fighting over /dev/pts/0 which ends in tears Is this something that systemd should detect cope with, or should we document that the 'container_ttys' env *must exclude* any tty associated with the /dev/console device ? I am tempted to say that we should do the latter, it's quite difficult to figure out when they point to the same (for example, because people use a bind mount rather than a symlink), and the roles of the console and the other $container_ttys is quite different during boot if we want to avoid printing logs over the getty and so on... I added this to the wiki text 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] getty : how to run getty on every ttyX
On Mon, Dec 16, 2013 at 05:33:12PM +0100, Lennart Poettering wrote: On Mon, 16.12.13 12:03, Daniel P. Berrange (berra...@redhat.com) wrote: Note that this *really* only supports ptys, not any other kinds of ttys, sinc for those we require propery device enumeration and notification and we don't have those in containers... I still chose to name this $container_ttys rather than $container_ptys, so that maybe one day we can extend it should devices like this ever get virtualized. This will be in systemd 209. I've tested this with libvirt and it worked except for one small edge case. Say libvirt creates 3 consoles /dev/pts/0, /dev/pts/1 and /dev/pts/2. Now we set container_ttys=pts/0 pts/1 pts/2 Systemd starts up 3 agetty processes - one of each of these. The /dev/console device, however, is also a link to /dev/pts/0 and so systemd starts up a agetty process for that too. Now we have 2 agetty processes fighting over /dev/pts/0 which ends in tears Is this something that systemd should detect cope with, or should we document that the 'container_ttys' env *must exclude* any tty associated with the /dev/console device ? I am tempted to say that we should do the latter, it's quite difficult to figure out when they point to the same (for example, because people use a bind mount rather than a symlink), and the roles of the console and the other $container_ttys is quite different during boot if we want to avoid printing logs over the getty and so on... I added this to the wiki text now. Ok, sounds good. I'll update libvirt to take account of this. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libsystemd-bus: make sure buf transfered into kenrel is 8 aligned
On Mon, Dec 16, 2013 at 4:09 PM, Kay Sievers k...@vrfy.org wrote:
On Mon, Dec 16, 2013 at 4:01 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Mon, 16.12.13 15:50, Lennart Poettering (lenn...@poettering.net) wrote:
diff --git a/src/libsystemd-bus/bus-control.c
b/src/libsystemd-bus/bus-control.c
index 0072c37..6b2790d 100644
--- a/src/libsystemd-bus/bus-control.c
+++ b/src/libsystemd-bus/bus-control.c
@@ -207,7 +207,7 @@ _public_ int sd_bus_release_name(sd_bus *bus, const
char *name) {
}
static int kernel_get_list(sd_bus *bus, uint64_t flags, char ***x) {
-struct kdbus_cmd_name_list cmd = {};
+struct kdbus_cmd_name_list __attribute__ ((__aligned__(8)))
cmd = {};
Hmm, this feels a bit like this would be better part of the type rather
than the variable. THus, kdbus.h should add this to all is structs,
rather then we decorate the variables...
Kay, would this make sense to you to add to kdbus.h?
Hmm, so thinking about this: the kdbus_cmd_name_list structure contains
64bit values anyway, so should naturally be aligned to 64bit boundaries
anyway... Or am I mistaken there and you are suggesting that on your
32bit architecture (which one is it if I may ask?) 64bit values don't
have to be aligned on an even 8byte boundary, but instead because the
arch is 32bit anyway and thus 64bit values need to be read in two steps
alignment on 4 is done in the abi?
We could try to define size, which is the first member in the struct, as:
__aligned_u64
instead of the current:
u64
That might do the trick. Could you test that if it works on your
platform/setup instead of patching the lib?
Just added __attribute__ ((__aligned__(8))) to kdbus.h for structures
used in ioctls.
Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] _noreturn_ -- noreturn for C11 compat
also define noreturn w/o stdnoreturn.h
---
src/core/main.c | 2 +-
src/journal/test-journal-interleaving.c | 2 +-
src/shared/log.c| 4 ++--
src/shared/log.h| 4 ++--
src/shared/macro.h | 10 +-
src/shared/pager.c | 2 +-
src/shared/util.c | 2 +-
src/shared/util.h | 2 +-
src/udev/collect/collect.c | 2 +-
9 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/core/main.c b/src/core/main.c
index 6c3d9bf..eb5413e 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -112,7 +112,7 @@ static FILE* serialization = NULL;
static void nop_handler(int sig) {
}
-_noreturn_ static void crash(int sig) {
+noreturn static void crash(int sig) {
if (getpid() != 1)
/* Pass this on immediately, if this is not PID 1 */
diff --git a/src/journal/test-journal-interleaving.c
b/src/journal/test-journal-interleaving.c
index af0d43e..5b74714 100644
--- a/src/journal/test-journal-interleaving.c
+++ b/src/journal/test-journal-interleaving.c
@@ -36,7 +36,7 @@
static bool arg_keep = false;
-_noreturn_ static void log_assert_errno(const char *text, int eno, const char
*file, int line, const char *func) {
+noreturn static void log_assert_errno(const char *text, int eno, const char
*file, int line, const char *func) {
log_meta(LOG_CRIT, file, line, func,
'%s' failed at %s:%u (%s): %s.,
text, file, line, func, strerror(eno));
diff --git a/src/shared/log.c b/src/shared/log.c
index 2404de8..2517f5d 100644
--- a/src/shared/log.c
+++ b/src/shared/log.c
@@ -702,12 +702,12 @@ static void log_assert(int level, const char *text, const
char *file, int line,
}
#pragma GCC diagnostic pop
-_noreturn_ void log_assert_failed(const char *text, const char *file, int
line, const char *func) {
+noreturn void log_assert_failed(const char *text, const char *file, int line,
const char *func) {
log_assert(LOG_CRIT, text, file, line, func, Assertion '%s' failed at
%s:%u, function %s(). Aborting.);
abort();
}
-_noreturn_ void log_assert_failed_unreachable(const char *text, const char
*file, int line, const char *func) {
+noreturn void log_assert_failed_unreachable(const char *text, const char
*file, int line, const char *func) {
log_assert(LOG_CRIT, text, file, line, func, Code should not be
reached '%s' at %s:%u, function %s(). Aborting.);
abort();
}
diff --git a/src/shared/log.h b/src/shared/log.h
index de0e000..3dcfa11 100644
--- a/src/shared/log.h
+++ b/src/shared/log.h
@@ -124,13 +124,13 @@ int log_dump_internal(
const char *func,
char *buffer);
-_noreturn_ void log_assert_failed(
+noreturn void log_assert_failed(
const char *text,
const char *file,
int line,
const char *func);
-_noreturn_ void log_assert_failed_unreachable(
+noreturn void log_assert_failed_unreachable(
const char *text,
const char *file,
int line,
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 362d62b..51af289 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -38,10 +38,18 @@
# endif
#endif
+/* define noreturn without stdnoreturn.h */
+#ifndef noreturn
+# if __STDC_VERSION__ = 201112L
+# define noreturn _Noreturn
+# else
+# define noreturn __attribute__((noreturn))
+# endif
+#endif
+
#define _printf_(a,b) __attribute__ ((format (printf, a, b)))
#define _alloc_(...) __attribute__ ((alloc_size(__VA_ARGS__)))
#define _sentinel_ __attribute__ ((sentinel))
-#define _noreturn_ __attribute__((noreturn))
#define _unused_ __attribute__ ((unused))
#define _destructor_ __attribute__ ((destructor))
#define _pure_ __attribute__ ((pure))
diff --git a/src/shared/pager.c b/src/shared/pager.c
index 9fa6114..72a29f2 100644
--- a/src/shared/pager.c
+++ b/src/shared/pager.c
@@ -32,7 +32,7 @@
static pid_t pager_pid = 0;
-_noreturn_ static void pager_fallback(void) {
+noreturn static void pager_fallback(void) {
ssize_t n;
do {
diff --git a/src/shared/util.c b/src/shared/util.c
index 0ce6f70..e1f92fd 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -3469,7 +3469,7 @@ int wait_for_terminate_and_warn(const char *name, pid_t
pid) {
return -EPROTO;
}
-_noreturn_ void freeze(void) {
+noreturn void freeze(void) {
/* Make sure nobody waits for us on a socket anymore */
close_all_fds(NULL, 0);
diff --git a/src/shared/util.h b/src/shared/util.h
index d5fa81c..d43aeff 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -417,7 +417,7 @@ char *normalize_env_assignment(const char *s);
int wait_for_terminate(pid_t pid, siginfo_t *status);
int wait_for_terminate_and_warn(const char *name, pid_t pid);
-_noreturn_ void freeze(void);
[systemd-devel] [PATCH] journal: fix assert against (theoretical) undefined behavior
While all the libc implementations I know return NULL when memchr's size parameter is 0: C11 7.24.1p2: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters. see http://llvm.org/bugs/show_bug.cgi?id=18247 --- src/journal/journal-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 4009b29..59b2829 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -956,7 +956,7 @@ static int journal_file_append_data( const void *eq; assert(f); -assert(data || size == 0); +assert(data); hash = hash64(data, size); -- 1.8.5.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] journal: fix assert against (theoretical) undefined behavior
On Mon, 16.12.13 09:20, Shawn Landden (sh...@churchofgit.com) wrote: While all the libc implementations I know return NULL when memchr's size parameter is 0: C11 7.24.1p2: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters. see http://llvm.org/bugs/show_bug.cgi?id=18247 Hmm? But what does that have to do with the requirements we make on our own internal functions? --- src/journal/journal-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 4009b29..59b2829 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -956,7 +956,7 @@ static int journal_file_append_data( const void *eq; assert(f); -assert(data || size == 0); +assert(data); hash = hash64(data, size); 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] journal: fix assert against (theoretical) undefined behavior
On Mon, Dec 16, 2013 at 10:36 AM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 16.12.13 09:20, Shawn Landden (sh...@churchofgit.com) wrote: While all the libc implementations I know return NULL when memchr's size parameter is 0: C11 7.24.1p2: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters. see http://llvm.org/bugs/show_bug.cgi?id=18247 Hmm? But what does that have to do with the requirements we make on our own internal functions? Well, it makes clang's scan-build shut up :), which is how i initially came across it. --- src/journal/journal-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 4009b29..59b2829 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -956,7 +956,7 @@ static int journal_file_append_data( const void *eq; assert(f); -assert(data || size == 0); +assert(data); hash = hash64(data, size); Lennart -- Lennart Poettering, Red Hat ___ 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] journal: fix assert against (theoretical) undefined behavior
On Mon, Dec 16, 2013 at 7:50 PM, Shawn Landden sh...@churchofgit.com wrote: On Mon, Dec 16, 2013 at 10:36 AM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 16.12.13 09:20, Shawn Landden (sh...@churchofgit.com) wrote: While all the libc implementations I know return NULL when memchr's size parameter is 0: C11 7.24.1p2: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters. see http://llvm.org/bugs/show_bug.cgi?id=18247 Hmm? But what does that have to do with the requirements we make on our own internal functions? Well, it makes clang's scan-build shut up :), which is how i initially came across it. Unfortunately there are quite a lot of false positives in scan-build. I have been cleaning it up for a while. I did come across this one earlier. Isn't the issue here that scan-build makes the wrong assumption that data can be null at that point? Generally it gets confused sometimes about functions that return a non-negative value only when setting some data succeeded. At least I think that was the conclusion I came to about the report. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Is it possible to start other service units by a service unit which was killed?
Because I have managed a few processes in systemd, I'd like to just start all processes at the same time when one of the process was failed or dead with kill signal. But I'm curious about fail from your answer, now. What is the fail signal in systemd? I don't know how to make systemd take fail by some signal or method in source code. Thanks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] journal: fix assert against (theoretical) undefined behavior
On Mon, Dec 16, 2013 at 11:06 AM, Thomas H.P. Andersen pho...@gmail.com wrote: On Mon, Dec 16, 2013 at 7:50 PM, Shawn Landden sh...@churchofgit.com wrote: On Mon, Dec 16, 2013 at 10:36 AM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 16.12.13 09:20, Shawn Landden (sh...@churchofgit.com) wrote: While all the libc implementations I know return NULL when memchr's size parameter is 0: C11 7.24.1p2: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters. see http://llvm.org/bugs/show_bug.cgi?id=18247 Hmm? But what does that have to do with the requirements we make on our own internal functions? Well, it makes clang's scan-build shut up :), which is how i initially came across it. Unfortunately there are quite a lot of false positives in scan-build. I have been cleaning it up for a while. I did come across this one earlier. Isn't the issue here that scan-build makes the wrong assumption that data can be null at that point? Generally it gets confused sometimes about functions that return a non-negative value only when setting some data succeeded. At least I think that was the conclusion I came to about the report. While there are certainly those (thinking walking a linked-list is use-after free, _cleanup_ issues: http://llvm.org/bugs/show_bug.cgi?id=3888 ) this was a real bug that scan_build found, but which I don't totally agree is important. If cause that there wasn't assert() against the first argument to memchr() being NULL, instead there was an assert against the first argument not being NULL OR size being 0, which (I was corrected to accept) can invoke undefined behavior. ___ 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] Is it possible to start other service units by a service unit which was killed?
В Tue, 17 Dec 2013 04:13:33 +0900 Tony Seo tonys...@gmail.com пишет: But I'm curious about fail from your answer, now. What is the fail signal in systemd? When process terminated unexpectedly with any exit status that does not mean normal exit. See SuccessExitStatus in man systemd.service. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] journal: fix assert against (theoretical) undefined behavior
On Mon, Dec 16, 2013 at 8:15 PM, Shawn Landden sh...@churchofgit.com wrote: On Mon, Dec 16, 2013 at 11:06 AM, Thomas H.P. Andersen pho...@gmail.com wrote: On Mon, Dec 16, 2013 at 7:50 PM, Shawn Landden sh...@churchofgit.com wrote: On Mon, Dec 16, 2013 at 10:36 AM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 16.12.13 09:20, Shawn Landden (sh...@churchofgit.com) wrote: While all the libc implementations I know return NULL when memchr's size parameter is 0: C11 7.24.1p2: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters. see http://llvm.org/bugs/show_bug.cgi?id=18247 Hmm? But what does that have to do with the requirements we make on our own internal functions? Well, it makes clang's scan-build shut up :), which is how i initially came across it. Unfortunately there are quite a lot of false positives in scan-build. I have been cleaning it up for a while. I did come across this one earlier. Isn't the issue here that scan-build makes the wrong assumption that data can be null at that point? Generally it gets confused sometimes about functions that return a non-negative value only when setting some data succeeded. At least I think that was the conclusion I came to about the report. While there are certainly those (thinking walking a linked-list is use-after free, _cleanup_ issues: http://llvm.org/bugs/show_bug.cgi?id=3888 ) this was a real bug that scan_build found, but which I don't totally agree is important. If cause that there wasn't assert() against the first argument to memchr() being NULL, instead there was an assert against the first argument not being NULL OR size being 0, which (I was corrected to accept) can invoke undefined behavior. After rereading it I see that I was mistaken. I had thought that data would already be set at that point if size == 0. Sorry for the confusion. Would it be enough to just add: if (data == NULL size == 0) eq = false; else eq = memchr(data, '=', size); ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] journal: fix assert against (theoretical) undefined behavior
On Mon, Dec 16, 2013 at 12:19 PM, Thomas H.P. Andersen pho...@gmail.com wrote: On Mon, Dec 16, 2013 at 8:15 PM, Shawn Landden sh...@churchofgit.com wrote: On Mon, Dec 16, 2013 at 11:06 AM, Thomas H.P. Andersen pho...@gmail.com wrote: On Mon, Dec 16, 2013 at 7:50 PM, Shawn Landden sh...@churchofgit.com wrote: On Mon, Dec 16, 2013 at 10:36 AM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 16.12.13 09:20, Shawn Landden (sh...@churchofgit.com) wrote: While all the libc implementations I know return NULL when memchr's size parameter is 0: C11 7.24.1p2: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters. see http://llvm.org/bugs/show_bug.cgi?id=18247 Hmm? But what does that have to do with the requirements we make on our own internal functions? Well, it makes clang's scan-build shut up :), which is how i initially came across it. Unfortunately there are quite a lot of false positives in scan-build. I have been cleaning it up for a while. I did come across this one earlier. Isn't the issue here that scan-build makes the wrong assumption that data can be null at that point? Generally it gets confused sometimes about functions that return a non-negative value only when setting some data succeeded. At least I think that was the conclusion I came to about the report. While there are certainly those (thinking walking a linked-list is use-after free, _cleanup_ issues: http://llvm.org/bugs/show_bug.cgi?id=3888 ) this was a real bug that scan_build found, but which I don't totally agree is important. If cause that there wasn't assert() against the first argument to memchr() being NULL, instead there was an assert against the first argument not being NULL OR size being 0, which (I was corrected to accept) can invoke undefined behavior. After rereading it I see that I was mistaken. I had thought that data would already be set at that point if size == 0. Sorry for the confusion. Would it be enough to just add: if (data == NULL size == 0) || eq = false; NULL else eq = memchr(data, '=', size); or keep the existing assert() and add if (size == 0) eq = NULL; else eq = memchr(data, '=', size); ___ 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
[systemd-devel] dmesg: Vacuuming done, freed xxx bytes
is it really needed to pass this messages to dmesg instead syslog or supress them? it is disturbing in cases you distribute dmesg over a infrastructure with more than 20 machines [941542.003467] systemd-journald[22511]: Vacuuming done, freed 2879488 bytes [944310.095412] systemd-journald[22511]: Vacuuming done, freed 2875392 bytes [947130.481168] systemd-journald[22511]: Vacuuming done, freed 2883584 bytes [949921.017800] systemd-journald[22511]: Vacuuming done, freed 2875392 bytes [952696.026187] systemd-journald[22511]: Vacuuming done, freed 2805760 bytes [955908.163206] systemd-journald[22511]: Vacuuming done, freed 2826240 bytes signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] journal: fix against (theoretical) undefined behavior
While all the libc implementations I know return NULL when memchr's size
parameter is 0, without accessing any memory, passing NULL to memchr is
still invalid:
C11 7.24.1p2: Where an argument declared as size_t n specifies the length
of the array for a function, n can have the value zero on a call to that
function. Unless explicitly stated otherwise in the description of a
particular function in this subclause, pointer arguments on such a call
shall still have valid values, as described in 7.1.4. On such a call, a
function that locates a character finds no occurrence, a function that
compares two character sequences returns zero, and a function that copies
characters copies zero characters.
see http://llvm.org/bugs/show_bug.cgi?id=18247
---
src/journal/journal-file.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c
index 4009b29..c6c9f5d 100644
--- a/src/journal/journal-file.c
+++ b/src/journal/journal-file.c
@@ -1010,7 +1010,10 @@ static int journal_file_append_data(
if (r 0)
return r;
-eq = memchr(data, '=', size);
+if (!data)
+eq = NULL;
+else
+eq = memchr(data, '=', size);
if (eq eq data) {
uint64_t fp;
Object *fo;
--
1.8.5.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libsystemd-bus: make sure buf transfered into kenrel is 8 aligned
On 2013-12-16, 16:01 +0100, Lennart Poettering wrote:
On Mon, 16.12.13 15:50, Lennart Poettering (lenn...@poettering.net) wrote:
diff --git a/src/libsystemd-bus/bus-control.c
b/src/libsystemd-bus/bus-control.c
index 0072c37..6b2790d 100644
--- a/src/libsystemd-bus/bus-control.c
+++ b/src/libsystemd-bus/bus-control.c
@@ -207,7 +207,7 @@ _public_ int sd_bus_release_name(sd_bus *bus, const
char *name) {
}
static int kernel_get_list(sd_bus *bus, uint64_t flags, char ***x) {
-struct kdbus_cmd_name_list cmd = {};
+struct kdbus_cmd_name_list __attribute__ ((__aligned__(8)))
cmd = {};
Hmm, this feels a bit like this would be better part of the type rather
than the variable. THus, kdbus.h should add this to all is structs,
rather then we decorate the variables...
Kay, would this make sense to you to add to kdbus.h?
Hmm, so thinking about this: the kdbus_cmd_name_list structure contains
64bit values anyway, so should naturally be aligned to 64bit boundaries
anyway... Or am I mistaken there and you are suggesting that on your
32bit architecture (which one is it if I may ask?) 64bit values don't
I am doing tests in 32bit system.
have to be aligned on an even 8byte boundary, but instead because the
arch is 32bit anyway and thus 64bit values need to be read in two steps
alignment on 4 is done in the abi?
I am getting a little more clearer about how gcc tries to make stack
boundary aligned..
I am using this example test code:
8
#include stdio.h
#include linux/types.h
struct kdbus_cmd_hello {
__u64 size;
__u64 conn_flags;
__u64 attach_flags;
__u64 bus_flags;
__u64 id;
__u64 bloom_size;
__u64 pool_size;
__u8 id128[16];
};
int main(int argc, char **argv)
{
struct kdbus_cmd_hello hello;
int r;
printf(hello addr: %08x\n, hello);
r = 1;
return 0;
}
8
By default, gcc will *try* to make stack boundary 16 bytes aligned
(pls refer to -mpreferred-stack-boundary=num in gcc mannual).
struct kdbus_cmd_hello itself is 64bit aligned, yes. However, if you
have a int r after the kdbus_cmd_hello, address of hello might be
16 bytes boundary + 4 (so that after pushing r, stack boundary
will be 16 bytes aligned).
$ ./test
hello addr: bfe8d334
$ ./test
hello addr: bf9aa9f4
$ ./test
hello addr: bfe443c4
$ ./test
hello addr: bfe8d334
$ ./test
hello addr: bf9fdf24
Regards,
Kangkai
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libsystemd-bus: make sure buf transfered into kenrel is 8 aligned
On 2013-12-16, 17:49 +0100, Kay Sievers wrote: On Mon, Dec 16, 2013 at 4:09 PM, Kay Sievers k...@vrfy.org wrote: Just added __attribute__ ((__aligned__(8))) to kdbus.h for structures used in ioctls. Yes, your patch is much more cleaner, thanks Kay. Regards, Kangkai ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] journal: fix against (theoretical) undefined behavior
On Mon, Dec 16, 2013 at 03:41:00PM -0800, Shawn Landden wrote: While all the libc implementations I know return NULL when memchr's size parameter is 0, without accessing any memory, passing NULL to memchr is still invalid: C11 7.24.1p2: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters. This analysis seems correct. Applied. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] rtnl: fix for 32bits
Commit 0a0dc69b655cfb10cab39133f5d521e7b35ce3d5 broke tests for 32 bits --- src/libsystemd-rtnl/rtnl-message.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd-rtnl/rtnl-message.c b/src/libsystemd-rtnl/rtnl-message.c index 264cca0..c62eca9 100644 --- a/src/libsystemd-rtnl/rtnl-message.c +++ b/src/libsystemd-rtnl/rtnl-message.c @@ -601,7 +601,7 @@ int sd_rtnl_message_append_ether_addr(sd_rtnl_message *m, unsigned short type, c return -ENOTSUP; } -r = add_rtattr(m, type, data, sizeof(data)); +r = add_rtattr(m, type, data, ETH_ALEN); if (r 0) return r; -- 1.8.5.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] rtnl: fix for 32bits
On Tue, Dec 17, 2013 at 02:13:57PM +0900, Marc-Antoine Perennou wrote: Commit 0a0dc69b655cfb10cab39133f5d521e7b35ce3d5 broke tests for 32 bits --- src/libsystemd-rtnl/rtnl-message.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd-rtnl/rtnl-message.c b/src/libsystemd-rtnl/rtnl-message.c index 264cca0..c62eca9 100644 --- a/src/libsystemd-rtnl/rtnl-message.c +++ b/src/libsystemd-rtnl/rtnl-message.c @@ -601,7 +601,7 @@ int sd_rtnl_message_append_ether_addr(sd_rtnl_message *m, unsigned short type, c return -ENOTSUP; } -r = add_rtattr(m, type, data, sizeof(data)); +r = add_rtattr(m, type, data, ETH_ALEN); Applied. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] udev rules environment variable
Hello,
got a small question about creating a rule, like this:
ACTION==add, , ENV{test_device}=1
ACTION==remove, , ENV{test_device}==1,
RUN+=/path/to/some/script
Does udev save test_device variable someplace and then it can be used
later on, when have ACTION==remove ?
--
Robert Milasan
L3 Support Engineer
SUSE Linux (http://www.suse.com)
email: rmila...@suse.com
GPG fingerprint: B6FE F4A8 0FA3 3040 3402 6FE7 2F64 167C 1909 6D1A
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] udev rules environment variable
On 12/17/2013 08:52 AM, Robert Milasan wrote:
Hello,
got a small question about creating a rule, like this:
ACTION==add, , ENV{test_device}=1
ACTION==remove, , ENV{test_device}==1,
RUN+=/path/to/some/script
Does udev save test_device variable someplace and then it can be used
later on, when have ACTION==remove ?
Typically not.
I would be using external tools for that, like 'collect'
Might be biased, though, what with me having written it ...
And the alternative of querying the udev database from within an
udev event is just plain ugly and prone to races.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
