Re: [systemd-devel] [systemd-commits] src/journal
On 12/19/2014 04:49 PM, Filipe Brandenburger wrote: This seems to have fixed test-journal-stream but test-journal-interleaving is still broken for me, it fails with: NUMBER=1 NUMBER=2 Assertion 'r == 1' failed at src/journal/test-journal-interleaving.c:101, function test_check_numbers_down(). Aborting. Aborted (core dumped) Can you please double check? Hello Filipe, thanks for pinging me about this. This should be fixed now with 668c965af4e journal: skipping of exhausted journal files is bad if direction changed. Can you please make sure that make check passes before pushing the commits? I usually do, but sometimes I forget :) Sorry! Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 4 commits - src/core test/TEST-03-JOBS
On 11/27/2014 03:42 AM, Zbigniew Jędrzejewski-Szmek wrote: On Wed, Nov 26, 2014 at 07:35:30AM -0800, Michal Schmidt wrote: src/core/job.c |7 +-- src/core/job.h | 14 ++ src/core/transaction.c |2 +- test/TEST-03-JOBS/test-jobs.sh |9 + 4 files changed, 25 insertions(+), 7 deletions(-) Hi, test-engine does not pass anymore. I didn't check exactly, but one of those commits is the most likely culprit. Yeah, this one: New commits: commit e0312f4db08c7100bd00299614e87bedc759b366 Author: Michal Schmidt mschm...@redhat.com Date: Wed Nov 26 16:33:46 2014 +0100 core: fix check for transaction destructiveness When checking if the transaction is destructive, we need to check if the previously installed job is a superset of the new job (and hence the new job will fold into the installed one without changing it), not the other way around. diff --git a/src/core/transaction.c b/src/core/transaction.c index bbaa6da..b992edd 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -511,7 +511,7 @@ static int transaction_is_destructive(Transaction *tr, JobMode mode, sd_bus_erro assert(!j-transaction_next); if (j-unit-job (mode == JOB_FAIL || j-unit-job-irreversible) -!job_type_is_superset(j-type, j-unit-job-type)) +!job_type_is_superset(j-unit-job-type, j-type)) return sd_bus_error_setf(e, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE, Transaction is destructive.); } Fixed now in c21b92ffe7 core: fix transaction destructiveness check once more Thanks, Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Cannot use systemctl after heavy swapping
On 11/14/2014 03:20 PM, Jan Janssen wrote: I think there might be something wrong with how the rate limiting works in manager.c. Just recently, firefox went nuts and got the whole system swapping like crazy. After manual OOM killing, the system is back to normal, but I can't seem to do any service management with systemctl afterwards. A simple sudo systemctl start systemd-timedated.service will hang forever. While the journal keeps getting this message about every second: systemd[1]: Looping too fast. Throttling execution a little. while other systemctl actions tend to time out (status, for example). Hi, are you able to trigger the problem again at will? I'd love to have a reproducer for this. There've been occasional reports of seeing the Looping too fast message before. Interestingly, if I don't use sudo (and instead rely on polkit), everything seems to work as expected and I can get things started. This suggests that PID1's confusion is affecting the private DBus socket (/run/systemd/private), but its connection to the system bus is still working. This is all on systemd 217 on up-to-date Arch. Regards, Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 17/26] unit: place reservations before merging other's dependencies
With the hashmap implementation that uses chaining the reservations
merely ensure that the merging won't result in long bucket chains.
With a future alternative implementation it will additionally reserve
memory to make sure the merging won't fail.
---
src/core/unit.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/src/core/unit.c b/src/core/unit.c
index 0389e6e..41b9ba4 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -585,6 +585,27 @@ static void merge_names(Unit *u, Unit *other) {
assert_se(hashmap_replace(u-manager-units, t, u) == 0);
}
+static int reserve_dependencies(Unit *u, Unit *other, UnitDependency d) {
+unsigned n_reserve;
+
+assert(u);
+assert(other);
+assert(d _UNIT_DEPENDENCY_MAX);
+
+/*
+ * If u does not have this dependency set allocated, there is no need
+ * to reserve anything. In that case other's set will be transfered
+ * as a whole to u by complete_move().
+ */
+if (!u-dependencies[d])
+return 0;
+
+/* merge_dependencies() will skip a u-on-u dependency */
+n_reserve = set_size(other-dependencies[d]) -
!!set_get(other-dependencies[d], u);
+
+return set_reserve(u-dependencies[d], n_reserve);
+}
+
static void merge_dependencies(Unit *u, Unit *other, const char *other_id,
UnitDependency d) {
Iterator i;
Unit *back;
@@ -627,6 +648,7 @@ static void merge_dependencies(Unit *u, Unit *other, const
char *other_id, UnitD
int unit_merge(Unit *u, Unit *other) {
UnitDependency d;
const char *other_id = NULL;
+int r;
assert(u);
assert(other);
@@ -660,6 +682,17 @@ int unit_merge(Unit *u, Unit *other) {
if (other-id)
other_id = strdupa(other-id);
+/* Make reservations to ensure merge_dependencies() won't fail */
+for (d = 0; d _UNIT_DEPENDENCY_MAX; d++) {
+r = reserve_dependencies(u, other, d);
+/*
+ * We don't rollback reservations if we fail. We don't have
+ * a way to undo reservations. A reservation is not a leak.
+ */
+if (r 0)
+return r;
+}
+
/* Merge names */
merge_names(u, other);
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 21/26] hashmap: rewrite the implementation
On 10/20/2014 08:42 PM, Lennart Poettering wrote:
On Thu, 16.10.14 09:50, Michal Schmidt (mschm...@redhat.com) wrote:
+/* Fields that all hashmap/set types must have */
+struct HashmapBase {
+const struct hash_ops *hash_ops; /* hash and compare ops to use */
+
+union _packed_ {
+struct indirect_storage indirect; /* if has_indirect */
+struct direct_storage direct; /* if !has_indirect */
+};
+
+uint8_t type:2; /* HASHMAP_TYPE_* */
Should probably be a named enum (see above)
+uint8_t has_indirect:1; /* whether indirect storage is
used */
Should really be a bool, no?
Interestingly, changing this confuses pahole greatly.
With all the three bitfields declared as uint8_t pahole showed the struct
layout sanely:
struct HashmapBase {
const struct hash_ops * hash_ops; /* 0 8 */
union {
struct indirect_storage indirect;/* 39 */
struct direct_storage direct;/* 39 */
}; /* 839 */
uint8_ttype:2; /*47: 6 1 */
uint8_thas_indirect:1; /*47: 5 1 */
uint8_tn_direct_entries:3; /*47: 2 1 */
/* size: 48, cachelines: 1, members: 5 */
/* bit_padding: 2 bits */
/* last cacheline: 48 bytes */
};
With type changed to a named enum, has_indirect to bool,
and n_direct_entries to unsigned, pahole says:
base_type__name_to_size: base_type __unknown__
base_type__name_to_size: base_type __unknown__
die__process_inline_expansion: tag not supported (INVALID)!
base_type__name_to_size: base_type __unknown__
...
struct HashmapBase {
const struct hash_ops * hash_ops; /* 0 8 */
union {
struct indirect_storage indirect;/* 39 */
struct direct_storage direct;/* 39 */
}; /* 839 */
/* Bitfield combined with next fields */
enum HashmapType type:2; /*44: 6 4 */
/* XXX 22 bits hole, try to pack */
/* Bitfield combined with next fields */
_Bool has_indirect:1; /*47: 5 1 */
/* Bitfield combined with previous fields */
__unknown__n_direct_entries:3; /*44: 2 0 */
/* size: 48, cachelines: 1, members: 5 */
/* bit holes: 1, sum bit holes: 22 bits */
/* padding: 4 */
/* bit_padding: 252 bits */
/* last cacheline: 48 bytes */
/* BRAIN FART ALERT! 48 != 48 + 0(holes), diff = 0 */
};
But as far as I can tell, generated code looks OK and it still runs
fine, so I won't worry about it.
Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 00/26] hashmap rewrite
On 10/20/2014 08:23 PM, Lennart Poettering wrote: On Thu, 16.10.14 09:50, Michal Schmidt (mschm...@redhat.com) wrote: Key changes that affect other code: - Sets and Hashmaps do not remember the insertion order anymore. They can still be iterated with *_FOREACH* or *_first*, but the order of entries is undefined. Hmm, this means the iteration logic will have to iteratively skip over empty buckets to be able to enumerate? Yes, to find occupied buckets, it does a sequential scan over the array of DIB values. That should be pretty fast. I figure that isn't too bad if we scale the number of buckets by the size of the hashmap anyway. Yes, though we never scale back down when many entries get removed from a hashmap (unless ALL of them get removed). It does not seem to be needed often. - There is a new type LinkedHashmap to use in the few cases where insertion order matters. The cases that I believe need it are converted in this patch series. Sounds good. Don't like the name though. I think OrderedHashmap would be better, since for the user of the data structure the different is all about the order, right? I saw the name LinkedHashmap used for this thing in Java/Android, so I went with that. I can rename it, no problem. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 21/26] hashmap: rewrite the implementation
On 10/20/2014 08:42 PM, Lennart Poettering wrote:
On Thu, 16.10.14 09:50, Michal Schmidt (mschm...@redhat.com) wrote:
+enum {
+HASHMAP_TYPE_PLAIN,
+HASHMAP_TYPE_LINKED,
+HASHMAP_TYPE_SET,
+__HASHMAP_TYPE_COUNT
+};
Why is this enum anonymous? Wouldn't it be nicer to give it a name? We
only use it internally only anyway...
So far we called the final counting entry of such enums _FOOBAR_MAX,
not __FOOBAR_COUNT, please stick to this naming scheme... (also iirc
the C standard reserves the double _ prefix for the language
itself. The kernel ignores that C standard issue so far, but we should
probably honour it.
OK, I'll improve these.
+uint8_t type:2; /* HASHMAP_TYPE_* */
Should probably be a named enum (see above)
[...]
+uint8_t has_indirect:1; /* whether indirect storage is
used */
Should really be a bool, no?
OK.
+static struct HashmapBase *__hashmap_new(const struct hash_ops *hash_ops,
uint8_t type HASHMAP_DEBUG_PARAMS) {
+HashmapBase *h;
+
+h = malloc0(all_hashmap_sizes[type]);
+if (!h)
+return NULL;
+
Hmm, a malloc() for each hashmap we allocate? Isn#t that prohibitively
expensive given how slow malloc() is? We have soo many hashmaps!
Is it really that slow? A small test program that does malloc(48) ten
million times (no freeing) runs for about 0.3 seconds on my laptop.
systemd allocates about 2000 hashmaps here. So these would take about
60 microseconds to malloc. Maybe the test program has it too easy due
to having to deal with no heap fragmentation. But then I'd expect the
allocations this small to be helped by glibc's fastbins...
I guess I'll take some measurements under qemu with no KVM :-)
+Set *_set_new(const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS) {
+return (Set*) __hashmap_new(hash_ops, HASHMAP_TYPE_SET
HASHMAP_DEBUG_PASS_ARGS);
+}
Not liking the double underscores... I know this is kernel style, but it's
still in conflict with ISO C...
Can we maybe name this internal_xyz or so?
Will do so.
Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 05/26] hashmap: drop assert(h) from linked_hashmap_next
It's handled just fine by returning NULL.
---
src/shared/hashmap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/shared/hashmap.c b/src/shared/hashmap.c
index c4fde89..8225b8e 100644
--- a/src/shared/hashmap.c
+++ b/src/shared/hashmap.c
@@ -945,7 +945,6 @@ void *hashmap_next(Hashmap *h, const void *key) {
unsigned hash;
struct hashmap_entry *e;
-assert(h);
assert(key);
if (!h)
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 09/26] journal: make sd_journal::files a LinkedHashmap
Anything that uses hashmap_next() almost certainly cares about the order
and needs to be a LinkedHashmap.
---
src/journal/journal-internal.h | 2 +-
src/journal/journalctl.c | 6 +++---
src/journal/sd-journal.c | 38 +++---
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/src/journal/journal-internal.h b/src/journal/journal-internal.h
index e591fb6..f33d9fd 100644
--- a/src/journal/journal-internal.h
+++ b/src/journal/journal-internal.h
@@ -100,7 +100,7 @@ struct sd_journal {
char *path;
char *prefix;
-Hashmap *files;
+LinkedHashmap *files;
MMapCache *mmap;
Location current_location;
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 328e40b..5c065f9 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -1488,7 +1488,7 @@ static int verify(sd_journal *j) {
log_show_color(true);
-HASHMAP_FOREACH(f, j-files, i) {
+LINKED_HASHMAP_FOREACH(f, j-files, i) {
int k;
usec_t first, validated, last;
@@ -1583,7 +1583,7 @@ static int access_check(sd_journal *j) {
assert(j);
if (set_isempty(j-errors)) {
-if (hashmap_isempty(j-files))
+if (linked_hashmap_isempty(j-files))
log_notice(No journal files were found.);
return 0;
}
@@ -1615,7 +1615,7 @@ static int access_check(sd_journal *j) {
}
#endif
-if (hashmap_isempty(j-files)) {
+if (linked_hashmap_isempty(j-files)) {
log_error(No journal files were opened due to
insufficient permissions.);
r = -EACCES;
}
diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
index 479444c..cf4ac56 100644
--- a/src/journal/sd-journal.c
+++ b/src/journal/sd-journal.c
@@ -86,7 +86,7 @@ static void detach_location(sd_journal *j) {
j-current_file = NULL;
j-current_field = 0;
-HASHMAP_FOREACH(f, j-files, i)
+LINKED_HASHMAP_FOREACH(f, j-files, i)
f-current_offset = 0;
}
@@ -881,7 +881,7 @@ static int real_journal_next(sd_journal *j, direction_t
direction) {
assert_return(j, -EINVAL);
assert_return(!journal_pid_changed(j), -ECHILD);
-HASHMAP_FOREACH(f, j-files, i) {
+LINKED_HASHMAP_FOREACH(f, j-files, i) {
bool found;
r = next_beyond_location(j, f, direction, o, p);
@@ -1290,10 +1290,10 @@ static int add_any_file(sd_journal *j, const char
*path) {
assert(j);
assert(path);
-if (hashmap_get(j-files, path))
+if (linked_hashmap_get(j-files, path))
return 0;
-if (hashmap_size(j-files) = JOURNAL_FILES_MAX) {
+if (linked_hashmap_size(j-files) = JOURNAL_FILES_MAX) {
log_warning(Too many open journal files, not adding %s.,
path);
return set_put_error(j, -ETOOMANYREFS);
}
@@ -1304,7 +1304,7 @@ static int add_any_file(sd_journal *j, const char *path) {
/* journal_file_dump(f); */
-r = hashmap_put(j-files, f-path, f);
+r = linked_hashmap_put(j-files, f-path, f);
if (r 0) {
journal_file_close(f);
return r;
@@ -1353,7 +1353,7 @@ static int remove_file(sd_journal *j, const char *prefix,
const char *filename)
if (!path)
return -ENOMEM;
-f = hashmap_get(j-files, path);
+f = linked_hashmap_get(j-files, path);
if (!f)
return 0;
@@ -1365,7 +1365,7 @@ static void remove_file_real(sd_journal *j, JournalFile
*f) {
assert(j);
assert(f);
-hashmap_remove(j-files, f-path);
+linked_hashmap_remove(j-files, f-path);
log_debug(File %s removed., f-path);
@@ -1376,7 +1376,7 @@ static void remove_file_real(sd_journal *j, JournalFile
*f) {
if (j-unique_file == f) {
/* Jump to the next unique_file or NULL if that one was last */
-j-unique_file = hashmap_next(j-files, j-unique_file-path);
+j-unique_file = linked_hashmap_next(j-files,
j-unique_file-path);
j-unique_offset = 0;
if (!j-unique_file)
j-unique_file_lost = true;
@@ -1636,7 +1636,7 @@ static int add_current_paths(sd_journal *j) {
* root directories. We don't expect errors here, so we
* treat them as fatal. */
-HASHMAP_FOREACH(f, j-files, i) {
+LINKED_HASHMAP_FOREACH(f, j-files, i) {
_cleanup_free_ char *dir;
int r;
@@ -1691,7 +1691,7 @@ static sd_journal *journal_new(int flags, const char
*path) {
goto fail;
}
-j-files = hashmap_new(string_hash_ops);
+
[systemd-devel] [PATCH 11/26] resolve: make DnsScope::conflict_queue a LinkedHashmap
on_conflict_dispatch() uses hashmap_steal_first() and then does
something non-trivial with it. It may care about the order.
---
src/resolve/resolved-dns-scope.c | 10 +-
src/resolve/resolved-dns-scope.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 40d5992..68091fa 100644
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -82,10 +82,10 @@ DnsScope* dns_scope_free(DnsScope *s) {
dns_transaction_free(t);
}
-while ((rr = hashmap_steal_first(s-conflict_queue)))
+while ((rr = linked_hashmap_steal_first(s-conflict_queue)))
dns_resource_record_unref(rr);
-hashmap_free(s-conflict_queue);
+linked_hashmap_free(s-conflict_queue);
sd_event_source_unref(s-conflict_event_source);
dns_cache_flush(s-cache);
@@ -682,7 +682,7 @@ static int on_conflict_dispatch(sd_event_source *es, usec_t
usec, void *userdata
_cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr =
NULL;
_cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
-rr = hashmap_steal_first(scope-conflict_queue);
+rr = linked_hashmap_steal_first(scope-conflict_queue);
if (!rr)
break;
@@ -709,7 +709,7 @@ int dns_scope_notify_conflict(DnsScope *scope,
DnsResourceRecord *rr) {
/* We don't send these queries immediately. Instead, we queue
* them, and send them after some jitter delay. */
-r = hashmap_ensure_allocated(scope-conflict_queue,
dns_resource_key_hash_ops);
+r = linked_hashmap_ensure_allocated(scope-conflict_queue,
dns_resource_key_hash_ops);
if (r 0) {
log_oom();
return r;
@@ -718,7 +718,7 @@ int dns_scope_notify_conflict(DnsScope *scope,
DnsResourceRecord *rr) {
/* We only place one RR per key in the conflict
* messages, not all of them. That should be enough to
* indicate where there might be a conflict */
-r = hashmap_put(scope-conflict_queue, rr-key, rr);
+r = linked_hashmap_put(scope-conflict_queue, rr-key, rr);
if (r == -EEXIST || r == 0)
return 0;
if (r 0) {
diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h
index a022309..5104011 100644
--- a/src/resolve/resolved-dns-scope.h
+++ b/src/resolve/resolved-dns-scope.h
@@ -55,7 +55,7 @@ struct DnsScope {
DnsCache cache;
DnsZone zone;
-Hashmap *conflict_queue;
+LinkedHashmap *conflict_queue;
sd_event_source *conflict_event_source;
RateLimit ratelimit;
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 03/26] test: add and improve hashmap tests
Test more corner cases and error states in several tests.
Add new tests for:
hashmap_move
hashmap_remove
hashmap_remove2
hashmap_remove_value
hashmap_remove_and_replace
hashmap_get2
hashmap_first
In test_hashmap_many additionally test with an intentionally bad hash
function.
---
src/test/test-hashmap-plain.c | 331 +++---
1 file changed, 308 insertions(+), 23 deletions(-)
diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c
index e98b3ff..6b93ed6 100644
--- a/src/test/test-hashmap-plain.c
+++ b/src/test/test-hashmap-plain.c
@@ -154,8 +154,10 @@ static void test_hashmap_move_one(void) {
hashmap_put(m, key 3, val3);
hashmap_put(m, key 4, val4);
-hashmap_move_one(n, m, key 3);
-hashmap_move_one(n, m, key 4);
+assert_se(hashmap_move_one(n, NULL, key 3) == -ENOENT);
+assert_se(hashmap_move_one(n, m, key 5) == -ENOENT);
+assert_se(hashmap_move_one(n, m, key 3) == 0);
+assert_se(hashmap_move_one(n, m, key 4) == 0);
r = hashmap_get(n, key 3);
assert_se(r streq(r, val3));
@@ -164,6 +166,49 @@ static void test_hashmap_move_one(void) {
r = hashmap_get(m, key 3);
assert_se(!r);
+assert_se(hashmap_move_one(n, m, key 3) == -EEXIST);
+
+hashmap_free_free(m);
+hashmap_free_free(n);
+}
+
+static void test_hashmap_move(void) {
+Hashmap *m, *n;
+char *val1, *val2, *val3, *val4, *r;
+
+val1 = strdup(val1);
+assert_se(val1);
+val2 = strdup(val2);
+assert_se(val2);
+val3 = strdup(val3);
+assert_se(val3);
+val4 = strdup(val4);
+assert_se(val4);
+
+m = hashmap_new(string_hash_ops);
+n = hashmap_new(string_hash_ops);
+
+hashmap_put(n, key 1, strdup(val1));
+hashmap_put(m, key 1, val1);
+hashmap_put(m, key 2, val2);
+hashmap_put(m, key 3, val3);
+hashmap_put(m, key 4, val4);
+
+hashmap_move(n, NULL);
+hashmap_move(n, m);
+
+assert_se(hashmap_size(m) == 1);
+r = hashmap_get(m, key 1);
+assert_se(r streq(r, val1));
+
+r = hashmap_get(n, key 1);
+assert_se(r streq(r, val1));
+r = hashmap_get(n, key 2);
+assert_se(r streq(r, val2));
+r = hashmap_get(n, key 3);
+assert_se(r streq(r, val3));
+r = hashmap_get(n, key 4);
+assert_se(r streq(r, val4));
hashmap_free_free(m);
hashmap_free_free(n);
@@ -183,7 +228,11 @@ static void test_hashmap_update(void) {
r = hashmap_get(m, key 1);
assert_se(streq(r, old_value));
-hashmap_update(m, key 1, val2);
+assert_se(hashmap_update(m, key 2, val2) == -ENOENT);
+r = hashmap_get(m, key 1);
+assert_se(streq(r, old_value));
+
+assert_se(hashmap_update(m, key 1, val2) == 0);
r = hashmap_get(m, key 1);
assert_se(streq(r, new_value));
@@ -193,18 +242,107 @@ static void test_hashmap_update(void) {
}
static void test_hashmap_put(void) {
-Hashmap *m;
+Hashmap *m = NULL;
int valid_hashmap_put;
+void *val1 = (void*) val 1;
-m = hashmap_new(string_hash_ops);
+hashmap_ensure_allocated(m, string_hash_ops);
+assert_se(m);
-valid_hashmap_put = hashmap_put(m, key 1, (void*) (const char *)
val 1);
+valid_hashmap_put = hashmap_put(m, key 1, val1);
assert_se(valid_hashmap_put == 1);
+assert_se(hashmap_put(m, key 1, val1) == 0);
+assert_se(hashmap_put(m, key 1, (void *)val 2) == -EEXIST);
-assert_se(m);
hashmap_free(m);
}
+static void test_hashmap_remove(void) {
+_cleanup_hashmap_free_ Hashmap *m = NULL;
+char *r;
+
+r = hashmap_remove(NULL, key 1);
+assert_se(r == NULL);
+
+m = hashmap_new(string_hash_ops);
+assert_se(m);
+
+r = hashmap_remove(m, no such key);
+assert_se(r == NULL);
+
+hashmap_put(m, key 1, (void*) val 1);
+hashmap_put(m, key 2, (void*) val 2);
+
+r = hashmap_remove(m, key 1);
+assert_se(streq(r, val 1));
+
+r = hashmap_get(m, key 2);
+assert_se(streq(r, val 2));
+assert_se(!hashmap_get(m, key 1));
+}
+
+static void test_hashmap_remove2(void) {
+_cleanup_hashmap_free_free_free_ Hashmap *m = NULL;
+char key1[] = key 1;
+char key2[] = key 2;
+char val1[] = val 1;
+char val2[] = val 2;
+void *r, *r2;
+
+r = hashmap_remove2(NULL, key 1, r2);
+assert_se(r == NULL);
+
+m = hashmap_new(string_hash_ops);
+assert_se(m);
+
+r = hashmap_remove2(m, no such key, r2);
+assert_se(r == NULL);
+
+hashmap_put(m, strdup(key1), strdup(val1));
+hashmap_put(m, strdup(key2), strdup(val2));
+
+r = hashmap_remove2(m, key1,
[systemd-devel] [PATCH 07/26] journal: make JournalFile::chain_cache a LinkedHashmap
The order of entries may matter here. Oldest entries are evicted first
when the cache is full.
(Though I don't see anything to rejuvenate entries on cache hits.)
---
src/journal/journal-file.c | 16
src/journal/journal-file.h | 2 +-
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c
index 038b437..e99483d 100644
--- a/src/journal/journal-file.c
+++ b/src/journal/journal-file.c
@@ -136,7 +136,7 @@ void journal_file_close(JournalFile *f) {
if (f-mmap)
mmap_cache_unref(f-mmap);
-hashmap_free_free(f-chain_cache);
+linked_hashmap_free_free(f-chain_cache);
#if defined(HAVE_XZ) || defined(HAVE_LZ4)
free(f-compress_buffer);
@@ -1366,7 +1366,7 @@ typedef struct ChainCacheItem {
} ChainCacheItem;
static void chain_cache_put(
-Hashmap *h,
+LinkedHashmap *h,
ChainCacheItem *ci,
uint64_t first,
uint64_t array,
@@ -1380,8 +1380,8 @@ static void chain_cache_put(
if (array == first)
return;
-if (hashmap_size(h) = CHAIN_CACHE_MAX)
-ci = hashmap_steal_first(h);
+if (linked_hashmap_size(h) = CHAIN_CACHE_MAX)
+ci = linked_hashmap_steal_first(h);
else {
ci = new(ChainCacheItem, 1);
if (!ci)
@@ -1390,7 +1390,7 @@ static void chain_cache_put(
ci-first = first;
-if (hashmap_put(h, ci-first, ci) 0) {
+if (linked_hashmap_put(h, ci-first, ci) 0) {
free(ci);
return;
}
@@ -1419,7 +1419,7 @@ static int generic_array_get(
a = first;
/* Try the chain cache first */
-ci = hashmap_get(f-chain_cache, first);
+ci = linked_hashmap_get(f-chain_cache, first);
if (ci i ci-total) {
a = ci-array;
i -= ci-total;
@@ -1522,7 +1522,7 @@ static int generic_array_bisect(
/* Start with the first array in the chain */
a = first;
-ci = hashmap_get(f-chain_cache, first);
+ci = linked_hashmap_get(f-chain_cache, first);
if (ci n ci-total) {
/* Ah, we have iterated this bisection array chain
* previously! Let's see if we can skip ahead in the
@@ -2498,7 +2498,7 @@ int journal_file_open(
goto fail;
}
-f-chain_cache = hashmap_new(uint64_hash_ops);
+f-chain_cache = linked_hashmap_new(uint64_hash_ops);
if (!f-chain_cache) {
r = -ENOMEM;
goto fail;
diff --git a/src/journal/journal-file.h b/src/journal/journal-file.h
index fa5b943..a9300e5 100644
--- a/src/journal/journal-file.h
+++ b/src/journal/journal-file.h
@@ -76,7 +76,7 @@ typedef struct JournalFile {
JournalMetrics metrics;
MMapCache *mmap;
-Hashmap *chain_cache;
+LinkedHashmap *chain_cache;
#if defined(HAVE_XZ) || defined(HAVE_LZ4)
void *compress_buffer;
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 25/26] test: test a corner case in hashmap_remove_and_replace
---
src/test/test-hashmap-plain.c | 20
1 file changed, 20 insertions(+)
diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c
index 8ea793d..f9553c9 100644
--- a/src/test/test-hashmap-plain.c
+++ b/src/test/test-hashmap-plain.c
@@ -380,6 +380,7 @@ static void test_hashmap_remove_and_replace(void) {
void *key2 = UINT_TO_PTR(2);
void *key3 = UINT_TO_PTR(3);
void *r;
+int i, j;
m = hashmap_new(trivial_hash_ops);
assert_se(m);
@@ -407,6 +408,25 @@ static void test_hashmap_remove_and_replace(void) {
r = hashmap_get(m, key2);
assert_se(r == key2);
assert_se(!hashmap_get(m, key3));
+
+/* Repeat this test several times to increase the chance of hitting
+ * the less likely case in hashmap_remove_and_replace where it
+ * compensates for the backward shift. */
+for (i = 0; i 20; i++) {
+hashmap_clear(m);
+
+for (j = 1; j 7; j++)
+hashmap_put(m, UINT_TO_PTR(10*i + j), UINT_TO_PTR(10*i
+ j));
+valid = hashmap_remove_and_replace(m, UINT_TO_PTR(10*i + 1),
+ UINT_TO_PTR(10*i + 2),
+ UINT_TO_PTR(10*i + 2));
+assert_se(valid == 0);
+assert_se(!hashmap_get(m, UINT_TO_PTR(10*i + 1)));
+for (j = 2; j 7; j++) {
+r = hashmap_get(m, UINT_TO_PTR(10*i + j));
+assert_se(r == UINT_TO_PTR(10*i + j));
+}
+}
}
static void test_hashmap_ensure_allocated(void) {
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 26/26] shared: drop mempool, now unused
---
Makefile.am | 2 --
src/shared/mempool.c | 94
src/shared/mempool.h | 55 --
3 files changed, 151 deletions(-)
delete mode 100644 src/shared/mempool.c
delete mode 100644 src/shared/mempool.h
diff --git a/Makefile.am b/Makefile.am
index 3763228..714c050 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -769,8 +769,6 @@ libsystemd_shared_la_SOURCES = \
src/shared/time-util.h \
src/shared/locale-util.c \
src/shared/locale-util.h \
- src/shared/mempool.c \
- src/shared/mempool.h \
src/shared/hashmap.c \
src/shared/hashmap.h \
src/shared/siphash24.c \
diff --git a/src/shared/mempool.c b/src/shared/mempool.c
deleted file mode 100644
index b1655ea..000
--- a/src/shared/mempool.c
+++ /dev/null
@@ -1,94 +0,0 @@
-/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
-
-/***
- This file is part of systemd.
-
- Copyright 2010-2014 Lennart Poettering
- Copyright 2014 Michal Schmidt
-
- 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
- the Free Software Foundation; either version 2.1 of the License, or
- (at your option) any later version.
-
- systemd is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public License
- along with systemd; If not, see http://www.gnu.org/licenses/.
-***/
-
-#include mempool.h
-#include macro.h
-#include util.h
-
-struct pool {
-struct pool *next;
-unsigned n_tiles;
-unsigned n_used;
-};
-
-void* __mempool_alloc_tile(struct mempool *mp) {
-unsigned i;
-
-/* When a tile is released we add it to the list and simply
- * place the next pointer at its offset 0. */
-
-assert(mp-tile_size = sizeof(void*));
-assert(mp-at_least 0);
-
-if (mp-freelist) {
-void *r;
-
-r = mp-freelist;
-mp-freelist = * (void**) mp-freelist;
-return r;
-}
-
-if (_unlikely_(!mp-first_pool) ||
-_unlikely_(mp-first_pool-n_used = mp-first_pool-n_tiles)) {
-unsigned n;
-size_t size;
-struct pool *p;
-
-n = mp-first_pool ? mp-first_pool-n_tiles : 0;
-n = MAX(mp-at_least, n * 2);
-size = PAGE_ALIGN(ALIGN(sizeof(struct pool)) +
n*mp-tile_size);
-n = (size - ALIGN(sizeof(struct pool))) / mp-tile_size;
-
-p = malloc(size);
-if (!p)
-return NULL;
-
-p-next = mp-first_pool;
-p-n_tiles = n;
-p-n_used = 0;
-
-mp-first_pool = p;
-}
-
-i = mp-first_pool-n_used++;
-
-return ((uint8_t*) mp-first_pool) + ALIGN(sizeof(struct pool)) +
i*mp-tile_size;
-}
-
-void __mempool_free_tile(struct mempool *mp, void *p) {
-* (void**) p = mp-freelist;
-mp-freelist = p;
-}
-
-#ifdef VALGRIND
-
-void mempool_drop(struct mempool *mp) {
-struct pool *p = mp-first_pool;
-while (p) {
-struct pool *n;
-n = p-next;
-free(p);
-p = n;
-}
-}
-
-#endif
diff --git a/src/shared/mempool.h b/src/shared/mempool.h
deleted file mode 100644
index 7f8bb98..000
--- a/src/shared/mempool.h
+++ /dev/null
@@ -1,55 +0,0 @@
-/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
-
-#pragma once
-
-/***
- This file is part of systemd.
-
- Copyright 2011-2014 Lennart Poettering
- Copyright 2014 Michal Schmidt
-
- 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
- the Free Software Foundation; either version 2.1 of the License, or
- (at your option) any later version.
-
- systemd is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public License
- along with systemd; If not, see http://www.gnu.org/licenses/.
-***/
-
-#include stddef.h
-
-struct pool;
-
-struct mempool {
-struct pool *first_pool;
-void *freelist;
-size_t tile_size;
-unsigned at_least;
-};
-
-void* __mempool_alloc_tile(struct mempool *mp);
-void __mempool_free_tile(struct mempool *mp, void *p);
-
-#define DEFINE_MEMPOOL(pool_name, tile_type, alloc_at_least) \
-struct mempool pool_name
[systemd-devel] [PATCH 10/26] sd-bus: make sd_bus::reply_callbacks a LinkedHashmap
The way process_closing() picks the first entry from reply_callbacks
and works with it makes it likely that it cares about the order.
---
src/libsystemd/sd-bus/bus-internal.h | 2 +-
src/libsystemd/sd-bus/bus-slot.c | 2 +-
src/libsystemd/sd-bus/sd-bus.c | 14 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/libsystemd/sd-bus/bus-internal.h
b/src/libsystemd/sd-bus/bus-internal.h
index 601ea5a..de52a58 100644
--- a/src/libsystemd/sd-bus/bus-internal.h
+++ b/src/libsystemd/sd-bus/bus-internal.h
@@ -230,7 +230,7 @@ struct sd_bus {
struct bus_match_node match_callbacks;
Prioq *reply_callbacks_prioq;
-Hashmap *reply_callbacks;
+LinkedHashmap *reply_callbacks;
LIST_HEAD(struct filter_callback, filter_callbacks);
Hashmap *nodes;
diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c
index d6793c2..a10c011 100644
--- a/src/libsystemd/sd-bus/bus-slot.c
+++ b/src/libsystemd/sd-bus/bus-slot.c
@@ -75,7 +75,7 @@ void bus_slot_disconnect(sd_bus_slot *slot) {
case BUS_REPLY_CALLBACK:
if (slot-reply_callback.cookie != 0)
-hashmap_remove(slot-bus-reply_callbacks,
slot-reply_callback.cookie);
+linked_hashmap_remove(slot-bus-reply_callbacks,
slot-reply_callback.cookie);
if (slot-reply_callback.timeout != 0)
prioq_remove(slot-bus-reply_callbacks_prioq,
slot-reply_callback, slot-reply_callback.prioq_idx);
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index e34a6bb..3303355 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -139,7 +139,7 @@ static void bus_free(sd_bus *b) {
bus_reset_queues(b);
-hashmap_free_free(b-reply_callbacks);
+linked_hashmap_free_free(b-reply_callbacks);
prioq_free(b-reply_callbacks_prioq);
assert(b-match_callbacks.type == BUS_MATCH_ROOT);
@@ -1759,7 +1759,7 @@ _public_ int sd_bus_call_async(
if (!BUS_IS_OPEN(bus-state))
return -ENOTCONN;
-r = hashmap_ensure_allocated(bus-reply_callbacks, uint64_hash_ops);
+r = linked_hashmap_ensure_allocated(bus-reply_callbacks,
uint64_hash_ops);
if (r 0)
return r;
@@ -1782,7 +1782,7 @@ _public_ int sd_bus_call_async(
s-reply_callback.callback = callback;
s-reply_callback.cookie = BUS_MESSAGE_COOKIE(m);
-r = hashmap_put(bus-reply_callbacks, s-reply_callback.cookie,
s-reply_callback);
+r = linked_hashmap_put(bus-reply_callbacks,
s-reply_callback.cookie, s-reply_callback);
if (r 0) {
s-reply_callback.cookie = 0;
return r;
@@ -2096,7 +2096,7 @@ static int process_timeout(sd_bus *bus) {
assert_se(prioq_pop(bus-reply_callbacks_prioq) == c);
c-timeout = 0;
-hashmap_remove(bus-reply_callbacks, c-cookie);
+linked_hashmap_remove(bus-reply_callbacks, c-cookie);
c-cookie = 0;
slot = container_of(c, sd_bus_slot, reply_callback);
@@ -2165,7 +2165,7 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) {
if (m-destination bus-unique_name !streq_ptr(m-destination,
bus-unique_name))
return 0;
-c = hashmap_remove(bus-reply_callbacks, m-reply_cookie);
+c = linked_hashmap_remove(bus-reply_callbacks, m-reply_cookie);
if (!c)
return 0;
@@ -2499,7 +2499,7 @@ static int process_closing(sd_bus *bus, sd_bus_message
**ret) {
assert(bus);
assert(bus-state == BUS_CLOSING);
-c = hashmap_first(bus-reply_callbacks);
+c = linked_hashmap_first(bus-reply_callbacks);
if (c) {
_cleanup_bus_error_free_ sd_bus_error error_buffer =
SD_BUS_ERROR_NULL;
sd_bus_slot *slot;
@@ -2522,7 +2522,7 @@ static int process_closing(sd_bus *bus, sd_bus_message
**ret) {
c-timeout = 0;
}
-hashmap_remove(bus-reply_callbacks, c-cookie);
+linked_hashmap_remove(bus-reply_callbacks, c-cookie);
c-cookie = 0;
slot = container_of(c, sd_bus_slot, reply_callback);
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 08/26] journal: make Server::user_journals a LinkedHashmap
Order matters here. It replaces oldest entries first when
USER_JOURNALS_MAX is reached.
---
src/journal/journald-server.c | 24
src/journal/journald-server.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 52111f7..275931b 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -266,7 +266,7 @@ static JournalFile* find_journal(Server *s, uid_t uid) {
if (r 0)
return s-system_journal;
-f = hashmap_get(s-user_journals, UINT32_TO_PTR(uid));
+f = linked_hashmap_get(s-user_journals, UINT32_TO_PTR(uid));
if (f)
return f;
@@ -274,9 +274,9 @@ static JournalFile* find_journal(Server *s, uid_t uid) {
SD_ID128_FORMAT_VAL(machine), uid) 0)
return s-system_journal;
-while (hashmap_size(s-user_journals) = USER_JOURNALS_MAX) {
+while (linked_hashmap_size(s-user_journals) = USER_JOURNALS_MAX) {
/* Too many open? Then let's close one */
-f = hashmap_steal_first(s-user_journals);
+f = linked_hashmap_steal_first(s-user_journals);
assert(f);
journal_file_close(f);
}
@@ -287,7 +287,7 @@ static JournalFile* find_journal(Server *s, uid_t uid) {
server_fix_perms(s, f, uid);
-r = hashmap_put(s-user_journals, UINT32_TO_PTR(uid), f);
+r = linked_hashmap_put(s-user_journals, UINT32_TO_PTR(uid), f);
if (r 0) {
journal_file_close(f);
return s-system_journal;
@@ -328,13 +328,13 @@ void server_rotate(Server *s) {
do_rotate(s, s-runtime_journal, runtime, false, 0);
do_rotate(s, s-system_journal, system, s-seal, 0);
-HASHMAP_FOREACH_KEY(f, k, s-user_journals, i) {
+LINKED_HASHMAP_FOREACH_KEY(f, k, s-user_journals, i) {
r = do_rotate(s, f, user, s-seal, PTR_TO_UINT32(k));
if (r = 0)
-hashmap_replace(s-user_journals, k, f);
+linked_hashmap_replace(s-user_journals, k, f);
else if (!f)
/* Old file has been closed and deallocated */
-hashmap_remove(s-user_journals, k);
+linked_hashmap_remove(s-user_journals, k);
}
}
@@ -350,7 +350,7 @@ void server_sync(Server *s) {
log_error(Failed to sync system journal: %s,
strerror(-r));
}
-HASHMAP_FOREACH_KEY(f, k, s-user_journals, i) {
+LINKED_HASHMAP_FOREACH_KEY(f, k, s-user_journals, i) {
r = journal_file_set_offline(f);
if (r 0)
log_error(Failed to sync user journal: %s,
strerror(-r));
@@ -1482,7 +1482,7 @@ int server_init(Server *s) {
mkdir_p(/run/systemd/journal, 0755);
-s-user_journals = hashmap_new(NULL);
+s-user_journals = linked_hashmap_new(NULL);
if (!s-user_journals)
return log_oom();
@@ -1602,7 +1602,7 @@ void server_maybe_append_tags(Server *s) {
if (s-system_journal)
journal_file_maybe_append_tag(s-system_journal, n);
-HASHMAP_FOREACH(f, s-user_journals, i)
+LINKED_HASHMAP_FOREACH(f, s-user_journals, i)
journal_file_maybe_append_tag(f, n);
#endif
}
@@ -1620,10 +1620,10 @@ void server_done(Server *s) {
if (s-runtime_journal)
journal_file_close(s-runtime_journal);
-while ((f = hashmap_steal_first(s-user_journals)))
+while ((f = linked_hashmap_steal_first(s-user_journals)))
journal_file_close(f);
-hashmap_free(s-user_journals);
+linked_hashmap_free(s-user_journals);
sd_event_source_unref(s-syslog_event_source);
sd_event_source_unref(s-native_event_source);
diff --git a/src/journal/journald-server.h b/src/journal/journald-server.h
index 42a2235..d3c227d 100644
--- a/src/journal/journald-server.h
+++ b/src/journal/journald-server.h
@@ -76,7 +76,7 @@ typedef struct Server {
JournalFile *runtime_journal;
JournalFile *system_journal;
-Hashmap *user_journals;
+LinkedHashmap *user_journals;
uint64_t seqnum;
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 02/26] test: generate tests for LinkedHashmap from Hashmap tests
test-hashmap-linked.c is generated from test-hashmap-plain.c simply by
substituting linked_hashmap for hashmap etc.
In the cases where tests rely on the order of entries, a distinction
between plain and linked hashmaps is made using the LINKED macro, which
is defined only for test-hashmap-linked.c.
---
Makefile.am | 21 +-
src/test/.gitignore | 1 +
src/test/test-hashmap-plain.c | 541 +
src/test/test-hashmap.c | 542 ++
4 files changed, 584 insertions(+), 521 deletions(-)
create mode 100644 src/test/.gitignore
create mode 100644 src/test/test-hashmap-plain.c
diff --git a/Makefile.am b/Makefile.am
index e52db17..275e793 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1570,8 +1570,27 @@ test_namespace_SOURCES = \
test_namespace_LDADD = \
libsystemd-core.la
+CLEANFILES += \
+ src/test/test-hashmap-linked.c
+
+BUILT_SOURCES += \
+ src/test/test-hashmap-linked.c
+
+src/test/test-hashmap-linked.c: src/test/test-hashmap-plain.c
+ $(AM_V_at)$(MKDIR_P) $(dir $@)
+ $(AM_V_GEN)$(AWK) 'BEGIN { print /* GENERATED FILE */\n#define LINKED
} \
+ { if (!match($$0, ^#include)) \
+gsub(/hashmap/, linked_hashmap); \
+gsub(/HASHMAP/, LINKED_HASHMAP); \
+gsub(/Hashmap/, LinkedHashmap); \
+print }' $ $@
+
+nodist_test_hashmap_SOURCES = \
+ src/test/test-hashmap-linked.c
+
test_hashmap_SOURCES = \
- src/test/test-hashmap.c
+ src/test/test-hashmap.c \
+ src/test/test-hashmap-plain.c
test_hashmap_LDADD = \
libsystemd-core.la
diff --git a/src/test/.gitignore b/src/test/.gitignore
new file mode 100644
index 000..b4c7f49
--- /dev/null
+++ b/src/test/.gitignore
@@ -0,0 +1 @@
+test-hashmap-linked.c
diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c
new file mode 100644
index 000..e98b3ff
--- /dev/null
+++ b/src/test/test-hashmap-plain.c
@@ -0,0 +1,541 @@
+/***
+ This file is part of systemd
+
+ Copyright 2013 Daniel Buch
+
+ 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
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include strv.h
+#include util.h
+#include hashmap.h
+
+void test_hashmap_funcs(void);
+
+static void test_hashmap_replace(void) {
+Hashmap *m;
+char *val1, *val2, *val3, *val4, *val5, *r;
+
+m = hashmap_new(string_hash_ops);
+
+val1 = strdup(val1);
+assert_se(val1);
+val2 = strdup(val2);
+assert_se(val2);
+val3 = strdup(val3);
+assert_se(val3);
+val4 = strdup(val4);
+assert_se(val4);
+val5 = strdup(val5);
+assert_se(val5);
+
+hashmap_put(m, key 1, val1);
+hashmap_put(m, key 2, val2);
+hashmap_put(m, key 3, val3);
+hashmap_put(m, key 4, val4);
+
+hashmap_replace(m, key 3, val1);
+r = hashmap_get(m, key 3);
+assert_se(streq(r, val1));
+
+hashmap_replace(m, key 5, val5);
+r = hashmap_get(m, key 5);
+assert_se(streq(r, val5));
+
+free(val1);
+free(val2);
+free(val3);
+free(val4);
+free(val5);
+hashmap_free(m);
+}
+
+static void test_hashmap_copy(void) {
+Hashmap *m, *copy;
+char *val1, *val2, *val3, *val4, *r;
+
+val1 = strdup(val1);
+assert_se(val1);
+val2 = strdup(val2);
+assert_se(val2);
+val3 = strdup(val3);
+assert_se(val3);
+val4 = strdup(val4);
+assert_se(val4);
+
+m = hashmap_new(string_hash_ops);
+
+hashmap_put(m, key 1, val1);
+hashmap_put(m, key 2, val2);
+hashmap_put(m, key 3, val3);
+hashmap_put(m, key 4, val4);
+
+copy = hashmap_copy(m);
+
+r = hashmap_get(copy, key 1);
+assert_se(streq(r, val1));
+r = hashmap_get(copy, key 2);
+assert_se(streq(r, val2));
+r = hashmap_get(copy, key 3);
+assert_se(streq(r, val3));
+r = hashmap_get(copy, key 4);
+assert_se(streq(r, val4));
+
+hashmap_free_free(copy);
+hashmap_free(m);
+}
+
+static void test_hashmap_get_strv(void) {
+Hashmap *m;
+char **strv;
+char *val1, *val2, *val3, *val4;
+
+
[systemd-devel] [PATCH 04/26] hashmap: hashmap_move_one should return -ENOENT when 'other' is NULL
-ENOENT is the same return value as if 'other' were an allocated hashmap
that does not contain the key. A NULL hashmap is a possible way of
expressing a hashmap that contains no key.
---
src/shared/hashmap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/shared/hashmap.c b/src/shared/hashmap.c
index 4c51705..c4fde89 100644
--- a/src/shared/hashmap.c
+++ b/src/shared/hashmap.c
@@ -886,15 +886,15 @@ int hashmap_move_one(Hashmap *h, Hashmap *other, const
void *key) {
unsigned h_hash, other_hash;
struct hashmap_entry *e;
-if (!other)
-return 0;
-
assert(h);
h_hash = bucket_hash(h, key);
if (hash_scan(h, h_hash, key))
return -EEXIST;
+if (!other)
+return -ENOENT;
+
other_hash = bucket_hash(other, key);
e = hash_scan(other, other_hash, key);
if (!e)
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 13/26] hashmap: return more information from resize_buckets
Return 0 if no resize was needed, 1 if successfully resized and
negative on error.
---
src/shared/hashmap.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/shared/hashmap.c b/src/shared/hashmap.c
index 1ae3602..2a152ab 100644
--- a/src/shared/hashmap.c
+++ b/src/shared/hashmap.c
@@ -369,7 +369,7 @@ static struct hashmap_entry *hash_scan(Hashmap *h, unsigned
hash, const void *ke
return NULL;
}
-static bool resize_buckets(Hashmap *h) {
+static int resize_buckets(Hashmap *h) {
struct hashmap_entry **n, *i;
unsigned m;
uint8_t nkey[HASH_KEY_SIZE];
@@ -377,7 +377,7 @@ static bool resize_buckets(Hashmap *h) {
assert(h);
if (_likely_(h-n_entries*4 h-n_buckets*3))
-return false;
+return 0;
/* Increase by four */
m = (h-n_entries+1)*4-1;
@@ -385,7 +385,7 @@ static bool resize_buckets(Hashmap *h) {
/* If we hit OOM we simply risk packed hashmaps... */
n = new0(struct hashmap_entry*, m);
if (!n)
-return false;
+return -ENOMEM;
/* Let's use a different randomized hash key for the
* extension, so that people cannot guess what we are using
@@ -424,7 +424,7 @@ static bool resize_buckets(Hashmap *h) {
memcpy(h-hash_key, nkey, HASH_KEY_SIZE);
-return true;
+return 1;
}
static int __hashmap_put(Hashmap *h, const void *key, void *value, unsigned
hash) {
@@ -432,7 +432,7 @@ static int __hashmap_put(Hashmap *h, const void *key, void
*value, unsigned hash
struct hashmap_entry *e;
-if (resize_buckets(h))
+if (resize_buckets(h) 0)
hash = bucket_hash(h, key);
if (h-from_pool)
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 06/26] install: make InstallContext::{will_install, have_installed} LinkedHashmaps
It appears order may matter here. Use LinkedHashmaps to be safe.
---
src/shared/install.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/src/shared/install.c b/src/shared/install.c
index ff5dcba..4b9fb7a 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -41,8 +41,8 @@
#include special.h
typedef struct {
-Hashmap *will_install;
-Hashmap *have_installed;
+LinkedHashmap *will_install;
+LinkedHashmap *have_installed;
} InstallContext;
static int in_search_path(const char *path, char **search) {
@@ -844,16 +844,16 @@ static void install_info_free(InstallInfo *i) {
free(i);
}
-static void install_info_hashmap_free(Hashmap *m) {
+static void install_info_hashmap_free(LinkedHashmap *m) {
InstallInfo *i;
if (!m)
return;
-while ((i = hashmap_steal_first(m)))
+while ((i = linked_hashmap_steal_first(m)))
install_info_free(i);
-hashmap_free(m);
+linked_hashmap_free(m);
}
static void install_context_done(InstallContext *c) {
@@ -881,11 +881,11 @@ static int install_info_add(
if (!unit_name_is_valid(name, TEMPLATE_VALID))
return -EINVAL;
-if (hashmap_get(c-have_installed, name) ||
-hashmap_get(c-will_install, name))
+if (linked_hashmap_get(c-have_installed, name) ||
+linked_hashmap_get(c-will_install, name))
return 0;
-r = hashmap_ensure_allocated(c-will_install, string_hash_ops);
+r = linked_hashmap_ensure_allocated(c-will_install,
string_hash_ops);
if (r 0)
return r;
@@ -907,7 +907,7 @@ static int install_info_add(
}
}
-r = hashmap_put(c-will_install, i-name, i);
+r = linked_hashmap_put(c-will_install, i-name, i);
if (r 0)
goto fail;
@@ -1180,7 +1180,7 @@ static int unit_file_can_install(
if (r 0)
return r;
-assert_se(i = hashmap_first(c.will_install));
+assert_se(i = linked_hashmap_first(c.will_install));
r = unit_file_search(c, i, paths, root_dir, allow_symlink, true);
@@ -1401,13 +1401,13 @@ static int install_context_apply(
assert(paths);
assert(config_path);
-while ((i = hashmap_first(c-will_install))) {
+while ((i = linked_hashmap_first(c-will_install))) {
-q = hashmap_ensure_allocated(c-have_installed,
string_hash_ops);
+q = linked_hashmap_ensure_allocated(c-have_installed,
string_hash_ops);
if (q 0)
return q;
-assert_se(hashmap_move_one(c-have_installed, c-will_install,
i-name) == 0);
+assert_se(linked_hashmap_move_one(c-have_installed,
c-will_install, i-name) == 0);
q = unit_file_search(c, i, paths, root_dir, false, true);
if (q 0) {
@@ -1442,13 +1442,13 @@ static int install_context_mark_for_removal(
/* Marks all items for removal */
-while ((i = hashmap_first(c-will_install))) {
+while ((i = linked_hashmap_first(c-will_install))) {
-q = hashmap_ensure_allocated(c-have_installed,
string_hash_ops);
+q = linked_hashmap_ensure_allocated(c-have_installed,
string_hash_ops);
if (q 0)
return q;
-assert_se(hashmap_move_one(c-have_installed, c-will_install,
i-name) == 0);
+assert_se(linked_hashmap_move_one(c-have_installed,
c-will_install, i-name) == 0);
q = unit_file_search(c, i, paths, root_dir, false, true);
if (q == -ENOENT) {
@@ -1544,12 +1544,12 @@ int unit_file_add_dependency(
return r;
}
-while ((info = hashmap_first(c.will_install))) {
-r = hashmap_ensure_allocated(c.have_installed,
string_hash_ops);
+while ((info = linked_hashmap_first(c.will_install))) {
+r = linked_hashmap_ensure_allocated(c.have_installed,
string_hash_ops);
if (r 0)
return r;
-assert_se(hashmap_move_one(c.have_installed, c.will_install,
info-name) == 0);
+assert_se(linked_hashmap_move_one(c.have_installed,
c.will_install, info-name) == 0);
r = unit_file_search(c, info, paths, root_dir, false, false);
if (r 0)
@@ -1720,7 +1720,7 @@ int unit_file_set_default(
if (r 0)
return r;
-assert_se(i = hashmap_first(c.will_install));
+assert_se(i = linked_hashmap_first(c.will_install));
r = unit_file_search(c, i, paths, root_dir, false, true);
if (r 0)
--
2.1.0
___
systemd-devel mailing list
[systemd-devel] [PATCH 12/26] shared: split mempool implementation from hashmaps
= b;
@@ -339,7 +278,7 @@ static void remove_entry(Hashmap *h, struct hashmap_entry
*e) {
unlink_entry(h, e, hash);
if (h-from_pool)
-deallocate_tile(first_entry_tile, e);
+hashmap_entry_pool_free_tile(e);
else
free(e);
}
@@ -357,7 +296,7 @@ void hashmap_free(Hashmap*h) {
free(h-buckets);
if (h-from_pool)
-deallocate_tile(first_hashmap_tile, h);
+hashmap_pool_free_tile(container_of(h, struct hashmap_tile,
h));
else
free(h);
}
@@ -497,7 +436,7 @@ static int __hashmap_put(Hashmap *h, const void *key, void
*value, unsigned hash
hash = bucket_hash(h, key);
if (h-from_pool)
-e = allocate_tile(first_entry_pool, first_entry_tile,
sizeof(struct hashmap_entry), 64U);
+e = hashmap_entry_pool_alloc_tile();
else
e = new(struct hashmap_entry, 1);
diff --git a/src/shared/mempool.c b/src/shared/mempool.c
new file mode 100644
index 000..b1655ea
--- /dev/null
+++ b/src/shared/mempool.c
@@ -0,0 +1,94 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2010-2014 Lennart Poettering
+ Copyright 2014 Michal Schmidt
+
+ 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
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include mempool.h
+#include macro.h
+#include util.h
+
+struct pool {
+struct pool *next;
+unsigned n_tiles;
+unsigned n_used;
+};
+
+void* __mempool_alloc_tile(struct mempool *mp) {
+unsigned i;
+
+/* When a tile is released we add it to the list and simply
+ * place the next pointer at its offset 0. */
+
+assert(mp-tile_size = sizeof(void*));
+assert(mp-at_least 0);
+
+if (mp-freelist) {
+void *r;
+
+r = mp-freelist;
+mp-freelist = * (void**) mp-freelist;
+return r;
+}
+
+if (_unlikely_(!mp-first_pool) ||
+_unlikely_(mp-first_pool-n_used = mp-first_pool-n_tiles)) {
+unsigned n;
+size_t size;
+struct pool *p;
+
+n = mp-first_pool ? mp-first_pool-n_tiles : 0;
+n = MAX(mp-at_least, n * 2);
+size = PAGE_ALIGN(ALIGN(sizeof(struct pool)) +
n*mp-tile_size);
+n = (size - ALIGN(sizeof(struct pool))) / mp-tile_size;
+
+p = malloc(size);
+if (!p)
+return NULL;
+
+p-next = mp-first_pool;
+p-n_tiles = n;
+p-n_used = 0;
+
+mp-first_pool = p;
+}
+
+i = mp-first_pool-n_used++;
+
+return ((uint8_t*) mp-first_pool) + ALIGN(sizeof(struct pool)) +
i*mp-tile_size;
+}
+
+void __mempool_free_tile(struct mempool *mp, void *p) {
+* (void**) p = mp-freelist;
+mp-freelist = p;
+}
+
+#ifdef VALGRIND
+
+void mempool_drop(struct mempool *mp) {
+struct pool *p = mp-first_pool;
+while (p) {
+struct pool *n;
+n = p-next;
+free(p);
+p = n;
+}
+}
+
+#endif
diff --git a/src/shared/mempool.h b/src/shared/mempool.h
new file mode 100644
index 000..7f8bb98
--- /dev/null
+++ b/src/shared/mempool.h
@@ -0,0 +1,55 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+ This file is part of systemd.
+
+ Copyright 2011-2014 Lennart Poettering
+ Copyright 2014 Michal Schmidt
+
+ 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
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include stddef.h
+
+struct pool;
+
+struct mempool {
+struct pool *first_pool;
+void
[systemd-devel] [PATCH 24/26] configure.ac: add --enable-hashmap-debug option
The option simply enables hashmap debugging by defining ENABLE_HASHMAP_DEBUG. I suggest developing new code with it enabled, to have the iterator checks. --- configure.ac | 7 +++ 1 file changed, 7 insertions(+) diff --git a/configure.ac b/configure.ac index 945adfc..02a1923 100644 --- a/configure.ac +++ b/configure.ac @@ -1307,6 +1307,13 @@ AC_ARG_ENABLE(tests, enable_tests=$enableval, enable_tests=yes) AM_CONDITIONAL(ENABLE_TESTS, [test x$enable_tests = xyes]) +AC_ARG_ENABLE(hashmap-debug, + [AC_HELP_STRING([--enable-hashmap-debug], [enable hashmap debugging])], + enable_hashmap_debug=$enableval, enable_hashmap_debug=no) +AS_IF([test x$enable_hashmap_debug = xyes], [ +AC_DEFINE(ENABLE_HASHMAP_DEBUG, 1, [Define if hashmap debugging is to be enabled]) +]) + AC_SUBST([dbuspolicydir], [$with_dbuspolicydir]) AC_SUBST([dbussessionservicedir], [$with_dbussessionservicedir]) AC_SUBST([dbussystemservicedir], [$with_dbussystemservicedir]) -- 2.1.0 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 18/26] hashmap: allow hashmap_move to fail
It cannot fail in the current hashmap implementation, but it may fail in
alternative implementations (unless a sufficiently large reservation has
been placed beforehand).
---
src/shared/hashmap.c | 8 +---
src/shared/hashmap.h | 6 +++---
src/shared/set.c | 2 +-
src/shared/set.h | 2 +-
src/test/test-hashmap-plain.c | 4 ++--
5 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/src/shared/hashmap.c b/src/shared/hashmap.c
index a11ef45..a5740df 100644
--- a/src/shared/hashmap.c
+++ b/src/shared/hashmap.c
@@ -815,16 +815,16 @@ int hashmap_reserve(Hashmap *h, unsigned entries_add) {
return 0;
}
-void hashmap_move(Hashmap *h, Hashmap *other) {
+int hashmap_move(Hashmap *h, Hashmap *other) {
struct hashmap_entry *e, *n;
assert(h);
/* The same as hashmap_merge(), but every new item from other
- * is moved to h. This function is guaranteed to succeed. */
+ * is moved to h. */
if (!other)
-return;
+return 0;
for (e = other-iterate_list_head; e; e = n) {
unsigned h_hash, other_hash;
@@ -839,6 +839,8 @@ void hashmap_move(Hashmap *h, Hashmap *other) {
unlink_entry(other, e, other_hash);
link_entry(h, e, h_hash);
}
+
+return 0;
}
int hashmap_move_one(Hashmap *h, Hashmap *other, const void *key) {
diff --git a/src/shared/hashmap.h b/src/shared/hashmap.h
index 9a16eaf..df8d311 100644
--- a/src/shared/hashmap.h
+++ b/src/shared/hashmap.h
@@ -160,9 +160,9 @@ int hashmap_reserve(Hashmap *h, unsigned entries_add);
static inline int linked_hashmap_reserve(LinkedHashmap *h, unsigned
entries_add) {
return hashmap_reserve((Hashmap*) h, entries_add);
}
-void hashmap_move(Hashmap *h, Hashmap *other);
-static inline void linked_hashmap_move(LinkedHashmap *h, LinkedHashmap *other)
{
-hashmap_move((Hashmap*) h, (Hashmap*) other);
+int hashmap_move(Hashmap *h, Hashmap *other);
+static inline int linked_hashmap_move(LinkedHashmap *h, LinkedHashmap *other) {
+return hashmap_move((Hashmap*) h, (Hashmap*) other);
}
int hashmap_move_one(Hashmap *h, Hashmap *other, const void *key);
static inline int linked_hashmap_move_one(LinkedHashmap *h, LinkedHashmap
*other, const void *key) {
diff --git a/src/shared/set.c b/src/shared/set.c
index 1a3465c..84ab82a 100644
--- a/src/shared/set.c
+++ b/src/shared/set.c
@@ -141,7 +141,7 @@ int set_reserve(Set *s, unsigned entries_add) {
return hashmap_reserve(MAKE_HASHMAP(s), entries_add);
}
-void set_move(Set *s, Set *other) {
+int set_move(Set *s, Set *other) {
return hashmap_move(MAKE_HASHMAP(s), MAKE_HASHMAP(other));
}
diff --git a/src/shared/set.h b/src/shared/set.h
index 8fe071a..d2622d1 100644
--- a/src/shared/set.h
+++ b/src/shared/set.h
@@ -51,7 +51,7 @@ int set_remove_and_put(Set *s, void *old_value, void
*new_value);
int set_merge(Set *s, Set *other);
int set_reserve(Set *s, unsigned entries_add);
-void set_move(Set *s, Set *other);
+int set_move(Set *s, Set *other);
int set_move_one(Set *s, Set *other, void *value);
unsigned set_size(Set *s);
diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c
index 1c2c556..17b72a7 100644
--- a/src/test/test-hashmap-plain.c
+++ b/src/test/test-hashmap-plain.c
@@ -194,8 +194,8 @@ static void test_hashmap_move(void) {
hashmap_put(m, key 3, val3);
hashmap_put(m, key 4, val4);
-hashmap_move(n, NULL);
-hashmap_move(n, m);
+assert(hashmap_move(n, NULL) == 0);
+assert(hashmap_move(n, m) == 0);
assert_se(hashmap_size(m) == 1);
r = hashmap_get(m, key 1);
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 23/26] tools: add gdb command to dump hashmap information
$ sudo gdb -p 1
...
(gdb) source gdb-sd_dump_hashmaps.py
(gdb) sd_dump_hashmaps
... lists allocated hashmaps ...
(gdb) sd_dump_hashmaps 1
... lists allocated hashmaps, their DIB histograms and contiguous
blocks statistics ...
---
Makefile.am | 3 ++
tools/gdb-sd_dump_hashmaps.py | 94 +++
2 files changed, 97 insertions(+)
create mode 100644 tools/gdb-sd_dump_hashmaps.py
diff --git a/Makefile.am b/Makefile.am
index 58e5aef..3763228 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -6262,3 +6262,6 @@ print-%:
git-contrib:
@git shortlog -s `git describe --abbrev=0`.. | cut -c8- | awk '{ print
$$0 , }' | sort -u
+
+EXTRA_DIST += \
+tools/gdb-sd_dump_hashmaps.py
diff --git a/tools/gdb-sd_dump_hashmaps.py b/tools/gdb-sd_dump_hashmaps.py
new file mode 100644
index 000..d9fc421
--- /dev/null
+++ b/tools/gdb-sd_dump_hashmaps.py
@@ -0,0 +1,94 @@
+# -*- Mode: python; coding: utf-8; indent-tabs-mode: nil -*- */
+#
+# This file is part of systemd.
+#
+# Copyright 2014 Michal Schmidt
+#
+# 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
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+#
+# systemd is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with systemd; If not, see http://www.gnu.org/licenses/.
+
+import gdb
+
+class sd_dump_hashmaps(gdb.Command):
+dump systemd's hashmaps
+
+def __init__(self):
+super(sd_dump_hashmaps, self).__init__(sd_dump_hashmaps,
gdb.COMMAND_DATA, gdb.COMPLETE_NONE)
+
+def invoke(self, arg, from_tty):
+d = gdb.parse_and_eval(hashmap_debug_list)
+all_entry_sizes = gdb.parse_and_eval(all_entry_sizes)
+all_direct_buckets = gdb.parse_and_eval(all_direct_buckets)
+hashmap_base_t = gdb.lookup_type(HashmapBase)
+uchar_t = gdb.lookup_type(unsigned char)
+ulong_t = gdb.lookup_type(unsigned long)
+debug_offset = gdb.parse_and_eval((unsigned
long)((HashmapBase*)0)-debug)
+
+print type, hash, indirect, entries, max_entries, buckets,
creator
+while d:
+h = gdb.parse_and_eval((HashmapBase*)((char*)%d -
%d) % (int(d.cast(ulong_t)), debug_offset))
+
+if h[has_indirect]:
+storage_ptr =
h[indirect][storage].cast(uchar_t.pointer())
+n_entries = h[indirect][n_entries]
+n_buckets = h[indirect][n_buckets]
+else:
+storage_ptr =
h[direct][storage].cast(uchar_t.pointer())
+n_entries = h[n_direct_entries]
+n_buckets = all_direct_buckets[int(h[type])];
+
+t = [plain, linked, set][int(h[type])]
+
+print %s, %s, %s, %d, %d, %d, %s (%s:%d) % (t,
h[hash_ops], bool(h[has_indirect]), n_entries, d[max_entries], n_buckets,
d[func], d[file], d[line])
+
+if arg != and n_entries 0:
+dib_raw_addr = storage_ptr +
(all_entry_sizes[h[type]] * n_buckets)
+
+histogram = {}
+for i in xrange(0, n_buckets):
+dib = int(dib_raw_addr[i])
+histogram[dib] = histogram.get(dib, 0)
+ 1
+
+for dib in sorted(iter(histogram)):
+if dib != 255:
+print %3d %8d %f%% of
entries % (dib, histogram[dib], 100.0*histogram[dib]/n_entries)
+else:
+print %3d %8d %f%% of slots
% (dib, histogram[dib], 100.0*histogram[dib]/n_buckets)
+print mean DIB of entries: %f %
(sum([dib*histogram[dib] for dib in iter(histogram) if dib !=
255])*1.0/n_entries)
+
+blocks = []
+current_len = 1
+prev = int(dib_raw_addr[0])
+for i in xrange(1, n_buckets):
+dib = int(dib_raw_addr[i])
+if (dib == 255) != (prev == 255):
+if prev != 255
[systemd-devel] [PATCH 01/26] hashmap: add LinkedHashmap as a distinct type
Few Hashmaps/Sets need to remember the insertion order. Most don't care
about the order when iterating. It would be possible to use more compact
hashmap storage in the latter cases.
Add LinkedHashmap as a distinct type from Hashmap, with functions
prefixed with linked_. For now, the functions are nothing more than
inline wrappers for plain Hashmap functions.
---
src/shared/hashmap.h | 116 +++
1 file changed, 116 insertions(+)
diff --git a/src/shared/hashmap.h b/src/shared/hashmap.h
index e25840f..2a4277d 100644
--- a/src/shared/hashmap.h
+++ b/src/shared/hashmap.h
@@ -34,6 +34,7 @@
#define HASH_KEY_SIZE 16
typedef struct Hashmap Hashmap;
+typedef struct LinkedHashmap LinkedHashmap;
typedef struct _IteratorStruct _IteratorStruct;
typedef _IteratorStruct* Iterator;
@@ -82,56 +83,171 @@ extern const struct hash_ops devt_hash_ops = {
#endif
Hashmap *hashmap_new(const struct hash_ops *hash_ops);
+static inline LinkedHashmap *linked_hashmap_new(const struct hash_ops
*hash_ops) {
+return (LinkedHashmap*) hashmap_new(hash_ops);
+}
void hashmap_free(Hashmap *h);
+static inline void linked_hashmap_free(LinkedHashmap *h) {
+hashmap_free((Hashmap*) h);
+}
void hashmap_free_free(Hashmap *h);
+static inline void linked_hashmap_free_free(LinkedHashmap *h) {
+hashmap_free_free((Hashmap*) h);
+}
void hashmap_free_free_free(Hashmap *h);
+static inline void linked_hashmap_free_free_free(LinkedHashmap *h) {
+hashmap_free_free_free((Hashmap*) h);
+}
Hashmap *hashmap_copy(Hashmap *h);
+static inline LinkedHashmap *linked_hashmap_copy(LinkedHashmap *h) {
+return (LinkedHashmap*) hashmap_copy((Hashmap*) h);
+}
int hashmap_ensure_allocated(Hashmap **h, const struct hash_ops *hash_ops);
+static inline int linked_hashmap_ensure_allocated(LinkedHashmap **h, const
struct hash_ops *hash_ops) {
+return hashmap_ensure_allocated((Hashmap**) h, hash_ops);
+}
int hashmap_put(Hashmap *h, const void *key, void *value);
+static inline int linked_hashmap_put(LinkedHashmap *h, const void *key, void
*value) {
+return hashmap_put((Hashmap*) h, key, value);
+}
int hashmap_update(Hashmap *h, const void *key, void *value);
+static inline int linked_hashmap_update(LinkedHashmap *h, const void *key,
void *value) {
+return hashmap_update((Hashmap*) h, key, value);
+}
int hashmap_replace(Hashmap *h, const void *key, void *value);
+static inline int linked_hashmap_replace(LinkedHashmap *h, const void *key,
void *value) {
+return hashmap_replace((Hashmap*) h, key, value);
+}
void *hashmap_get(Hashmap *h, const void *key);
+static inline void *linked_hashmap_get(LinkedHashmap *h, const void *key) {
+return hashmap_get((Hashmap*) h, key);
+}
void *hashmap_get2(Hashmap *h, const void *key, void **rkey);
+static inline void *linked_hashmap_get2(LinkedHashmap *h, const void *key,
void **rkey) {
+return hashmap_get2((Hashmap*) h, key, rkey);
+}
bool hashmap_contains(Hashmap *h, const void *key);
+static inline bool linked_hashmap_contains(LinkedHashmap *h, const void *key) {
+return hashmap_contains((Hashmap*) h, key);
+}
void *hashmap_remove(Hashmap *h, const void *key);
+static inline void *linked_hashmap_remove(LinkedHashmap *h, const void *key) {
+return hashmap_remove((Hashmap*) h, key);
+}
void *hashmap_remove2(Hashmap *h, const void *key, void **rkey);
+static inline void *linked_hashmap_remove2(LinkedHashmap *h, const void *key,
void **rkey) {
+return hashmap_remove2((Hashmap*) h, key, rkey);
+}
void *hashmap_remove_value(Hashmap *h, const void *key, void *value);
+static inline void *linked_hashmap_remove_value(LinkedHashmap *h, const void
*key, void *value) {
+return hashmap_remove_value((Hashmap*) h, key, value);
+}
int hashmap_remove_and_put(Hashmap *h, const void *old_key, const void
*new_key, void *value);
+static inline int linked_hashmap_remove_and_put(LinkedHashmap *h, const void
*old_key, const void *new_key, void *value) {
+return hashmap_remove_and_put((Hashmap*) h, old_key, new_key, value);
+}
int hashmap_remove_and_replace(Hashmap *h, const void *old_key, const void
*new_key, void *value);
+static inline int linked_hashmap_remove_and_replace(LinkedHashmap *h, const
void *old_key, const void *new_key, void *value) {
+return hashmap_remove_and_replace((Hashmap*) h, old_key, new_key,
value);
+}
int hashmap_merge(Hashmap *h, Hashmap *other);
+static inline int linked_hashmap_merge(LinkedHashmap *h, LinkedHashmap *other)
{
+return hashmap_merge((Hashmap*) h, (Hashmap*) other);
+}
void hashmap_move(Hashmap *h, Hashmap *other);
+static inline void linked_hashmap_move(LinkedHashmap *h, LinkedHashmap *other)
{
+hashmap_move((Hashmap*) h, (Hashmap*) other);
+}
int hashmap_move_one(Hashmap *h, Hashmap *other, const void *key);
+static inline int linked_hashmap_move_one(LinkedHashmap *h, LinkedHashmap
[systemd-devel] [PATCH 14/26] hashmap: introduce hashmap_reserve()
With the current hashmap implementation that uses chaining, placing a
reservation can serve two purposes:
- To optimize putting of entries if the number of entries to put is
known. The reservation allocates buckets, so later resizing can be
avoided.
- To avoid having very long bucket chains after using
hashmap_move(_one).
In an alternative hashmap implementation it will serve an additional
purpose:
- To guarantee a subsequent hashmap_move(_one) will not fail with
-ENOMEM (this never happens in the current implementation).
---
src/shared/hashmap.c | 32 ++--
src/shared/hashmap.h | 4
src/shared/set.c | 4
src/shared/set.h | 1 +
4 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/src/shared/hashmap.c b/src/shared/hashmap.c
index 2a152ab..a11ef45 100644
--- a/src/shared/hashmap.c
+++ b/src/shared/hashmap.c
@@ -369,18 +369,26 @@ static struct hashmap_entry *hash_scan(Hashmap *h,
unsigned hash, const void *ke
return NULL;
}
-static int resize_buckets(Hashmap *h) {
+static int resize_buckets(Hashmap *h, unsigned entries_add) {
struct hashmap_entry **n, *i;
-unsigned m;
+unsigned m, new_n_entries, new_n_buckets;
uint8_t nkey[HASH_KEY_SIZE];
assert(h);
-if (_likely_(h-n_entries*4 h-n_buckets*3))
+new_n_entries = h-n_entries + entries_add;
+
+/* overflow? */
+if (_unlikely_(new_n_entries entries_add || new_n_entries UINT_MAX
/ 4))
+return -ENOMEM;
+
+new_n_buckets = new_n_entries * 4 / 3;
+
+if (_likely_(new_n_buckets = h-n_buckets))
return 0;
-/* Increase by four */
-m = (h-n_entries+1)*4-1;
+/* Increase by four at least */
+m = MAX((h-n_entries+1)*4-1, new_n_buckets);
/* If we hit OOM we simply risk packed hashmaps... */
n = new0(struct hashmap_entry*, m);
@@ -432,7 +440,7 @@ static int __hashmap_put(Hashmap *h, const void *key, void
*value, unsigned hash
struct hashmap_entry *e;
-if (resize_buckets(h) 0)
+if (resize_buckets(h, 1) 0)
hash = bucket_hash(h, key);
if (h-from_pool)
@@ -795,6 +803,18 @@ int hashmap_merge(Hashmap *h, Hashmap *other) {
return 0;
}
+int hashmap_reserve(Hashmap *h, unsigned entries_add) {
+int r;
+
+assert(h);
+
+r = resize_buckets(h, entries_add);
+if (r 0)
+return r;
+
+return 0;
+}
+
void hashmap_move(Hashmap *h, Hashmap *other) {
struct hashmap_entry *e, *n;
diff --git a/src/shared/hashmap.h b/src/shared/hashmap.h
index 2a4277d..9a16eaf 100644
--- a/src/shared/hashmap.h
+++ b/src/shared/hashmap.h
@@ -156,6 +156,10 @@ int hashmap_merge(Hashmap *h, Hashmap *other);
static inline int linked_hashmap_merge(LinkedHashmap *h, LinkedHashmap *other)
{
return hashmap_merge((Hashmap*) h, (Hashmap*) other);
}
+int hashmap_reserve(Hashmap *h, unsigned entries_add);
+static inline int linked_hashmap_reserve(LinkedHashmap *h, unsigned
entries_add) {
+return hashmap_reserve((Hashmap*) h, entries_add);
+}
void hashmap_move(Hashmap *h, Hashmap *other);
static inline void linked_hashmap_move(LinkedHashmap *h, LinkedHashmap *other)
{
hashmap_move((Hashmap*) h, (Hashmap*) other);
diff --git a/src/shared/set.c b/src/shared/set.c
index ed16067..1a3465c 100644
--- a/src/shared/set.c
+++ b/src/shared/set.c
@@ -137,6 +137,10 @@ int set_merge(Set *s, Set *other) {
return hashmap_merge(MAKE_HASHMAP(s), MAKE_HASHMAP(other));
}
+int set_reserve(Set *s, unsigned entries_add) {
+return hashmap_reserve(MAKE_HASHMAP(s), entries_add);
+}
+
void set_move(Set *s, Set *other) {
return hashmap_move(MAKE_HASHMAP(s), MAKE_HASHMAP(other));
}
diff --git a/src/shared/set.h b/src/shared/set.h
index 840ee0a..8fe071a 100644
--- a/src/shared/set.h
+++ b/src/shared/set.h
@@ -50,6 +50,7 @@ void *set_remove(Set *s, void *value);
int set_remove_and_put(Set *s, void *old_value, void *new_value);
int set_merge(Set *s, Set *other);
+int set_reserve(Set *s, unsigned entries_add);
void set_move(Set *s, Set *other);
int set_move_one(Set *s, Set *other, void *value);
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 16/26] install, cgtop: adjust hashmap_move_one callers for -ENOMEM possibility
That hashmap_move_one() currently cannot fail with -ENOMEM is an
implementation detail, which is not possible to guarantee in general.
Hashmap implementations based on anything else than chaining of
individual entries may have to allocate.
hashmap_move_one will not fail with -ENOMEM if a proper reservation has
been made beforehand. Use reservations in install.c.
In cgtop.c simply propagate the error instead of asserting.
---
src/cgtop/cgtop.c| 4 +++-
src/shared/install.c | 40 +---
2 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
index ab8c4cf..932a7ba 100644
--- a/src/cgtop/cgtop.c
+++ b/src/cgtop/cgtop.c
@@ -126,7 +126,9 @@ static int process(const char *controller, const char
*path, Hashmap *a, Hashmap
return r;
}
} else {
-assert_se(hashmap_move_one(a, b, path) == 0);
+r = hashmap_move_one(a, b, path);
+if (r 0)
+return r;
g-cpu_valid = g-memory_valid = g-io_valid =
g-n_tasks_valid = false;
}
}
diff --git a/src/shared/install.c b/src/shared/install.c
index 4b9fb7a..fba46ed 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1395,18 +1395,24 @@ static int install_context_apply(
unsigned *n_changes) {
InstallInfo *i;
-int r = 0, q;
+int r, q;
assert(c);
assert(paths);
assert(config_path);
-while ((i = linked_hashmap_first(c-will_install))) {
+if (!linked_hashmap_isempty(c-will_install)) {
+r = linked_hashmap_ensure_allocated(c-have_installed,
string_hash_ops);
+if (r 0)
+return r;
-q = linked_hashmap_ensure_allocated(c-have_installed,
string_hash_ops);
-if (q 0)
-return q;
+r = linked_hashmap_reserve(c-have_installed,
linked_hashmap_size(c-will_install));
+if (r 0)
+return r;
+}
+r = 0;
+while ((i = linked_hashmap_first(c-will_install))) {
assert_se(linked_hashmap_move_one(c-have_installed,
c-will_install, i-name) == 0);
q = unit_file_search(c, i, paths, root_dir, false, true);
@@ -1434,7 +1440,7 @@ static int install_context_mark_for_removal(
const char *root_dir) {
InstallInfo *i;
-int r = 0, q;
+int r, q;
assert(c);
assert(paths);
@@ -1442,12 +1448,18 @@ static int install_context_mark_for_removal(
/* Marks all items for removal */
-while ((i = linked_hashmap_first(c-will_install))) {
+if (!linked_hashmap_isempty(c-will_install)) {
+r = linked_hashmap_ensure_allocated(c-have_installed,
string_hash_ops);
+if (r 0)
+return r;
-q = linked_hashmap_ensure_allocated(c-have_installed,
string_hash_ops);
-if (q 0)
-return q;
+r = linked_hashmap_reserve(c-have_installed,
linked_hashmap_size(c-will_install));
+if (r 0)
+return r;
+}
+r = 0;
+while ((i = linked_hashmap_first(c-will_install))) {
assert_se(linked_hashmap_move_one(c-have_installed,
c-will_install, i-name) == 0);
q = unit_file_search(c, i, paths, root_dir, false, true);
@@ -1544,11 +1556,17 @@ int unit_file_add_dependency(
return r;
}
-while ((info = linked_hashmap_first(c.will_install))) {
+if (!linked_hashmap_isempty(c.will_install)) {
r = linked_hashmap_ensure_allocated(c.have_installed,
string_hash_ops);
if (r 0)
return r;
+r = linked_hashmap_reserve(c.have_installed,
linked_hashmap_size(c.will_install));
+if (r 0)
+return r;
+}
+
+while ((info = linked_hashmap_first(c.will_install))) {
assert_se(linked_hashmap_move_one(c.have_installed,
c.will_install, info-name) == 0);
r = unit_file_search(c, info, paths, root_dir, false, false);
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 20/26] util: add log2u, log2u_round_up
two's logarithms for unsigned.
---
src/shared/util.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/src/shared/util.h b/src/shared/util.h
index 21a90a4..dfb4341 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -829,6 +829,21 @@ static inline int log2i(int x) {
return __SIZEOF_INT__ * 8 - __builtin_clz(x) - 1;
}
+static inline unsigned log2u(unsigned x) {
+assert(x 0);
+
+return sizeof(unsigned) * 8 - __builtin_clz(x) - 1;
+}
+
+static inline unsigned log2u_round_up(unsigned x) {
+assert(x 0);
+
+if (x == 1)
+return 0;
+
+return log2u(x - 1) + 1;
+}
+
static inline bool logind_running(void) {
return access(/run/systemd/seats/, F_OK) = 0;
}
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 19/26] unit: adjust for the possibility of set_move failing
---
src/core/unit.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/src/core/unit.c b/src/core/unit.c
index 41b9ba4..e40e6f2 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -553,29 +553,38 @@ const char* unit_sub_state_to_string(Unit *u) {
return UNIT_VTABLE(u)-sub_state_to_string(u);
}
-static void complete_move(Set **s, Set **other) {
+static int complete_move(Set **s, Set **other) {
+int r;
+
assert(s);
assert(other);
if (!*other)
-return;
+return 0;
-if (*s)
-set_move(*s, *other);
-else {
+if (*s) {
+r = set_move(*s, *other);
+if (r 0)
+return r;
+} else {
*s = *other;
*other = NULL;
}
+
+return 0;
}
-static void merge_names(Unit *u, Unit *other) {
+static int merge_names(Unit *u, Unit *other) {
char *t;
Iterator i;
+int r;
assert(u);
assert(other);
-complete_move(u-names, other-names);
+r = complete_move(u-names, other-names);
+if (r 0)
+return r;
set_free_free(other-names);
other-names = NULL;
@@ -583,6 +592,8 @@ static void merge_names(Unit *u, Unit *other) {
SET_FOREACH(t, u-names, i)
assert_se(hashmap_replace(u-manager-units, t, u) == 0);
+
+return 0;
}
static int reserve_dependencies(Unit *u, Unit *other, UnitDependency d) {
@@ -639,7 +650,8 @@ static void merge_dependencies(Unit *u, Unit *other, const
char *other_id, UnitD
if (back)
maybe_warn_about_dependency(u-id, other_id, d);
-complete_move(u-dependencies[d], other-dependencies[d]);
+/* The move cannot fail. The caller must have performed a reservation.
*/
+assert_se(complete_move(u-dependencies[d], other-dependencies[d])
== 0);
set_free(other-dependencies[d]);
other-dependencies[d] = NULL;
@@ -694,7 +706,9 @@ int unit_merge(Unit *u, Unit *other) {
}
/* Merge names */
-merge_names(u, other);
+r = merge_names(u, other);
+if (r 0)
+return r;
/* Redirect all references */
while (other-refs)
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 22/26] test: adjust max load factor in test_hashmap_many
---
src/test/test-hashmap-plain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c
index 17b72a7..8ea793d 100644
--- a/src/test/test-hashmap-plain.c
+++ b/src/test/test-hashmap-plain.c
@@ -699,9 +699,9 @@ static void test_hashmap_many(void) {
for (i = 1; i tests[j].n_entries*3; i++)
assert_se(hashmap_contains(h, UINT_TO_PTR(i)) == (i %
3 == 1));
-log_info(%u = %u * 0.75 = %g, hashmap_size(h),
hashmap_buckets(h), hashmap_buckets(h) * 0.75);
+log_info(%u = %u * 0.8 = %g, hashmap_size(h),
hashmap_buckets(h), hashmap_buckets(h) * 0.8);
-assert_se(hashmap_size(h) = hashmap_buckets(h) * 0.75);
+assert_se(hashmap_size(h) = hashmap_buckets(h) * 0.8);
assert_se(hashmap_size(h) == tests[j].n_entries);
while (!hashmap_isempty(h)) {
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 15/26] test: add test for hashmap_reserve()
---
src/test/test-hashmap-plain.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c
index 6b93ed6..1c2c556 100644
--- a/src/test/test-hashmap-plain.c
+++ b/src/test/test-hashmap-plain.c
@@ -795,6 +795,23 @@ static void test_hashmap_clear_free_free(void) {
assert_se(hashmap_isempty(m));
}
+static void test_hashmap_reserve(void) {
+_cleanup_hashmap_free_ Hashmap *m = NULL;
+
+m = hashmap_new(string_hash_ops);
+
+assert_se(hashmap_reserve(m, 1) == 0);
+assert_se(hashmap_buckets(m) 1000);
+assert_se(hashmap_reserve(m, 1000) == 0);
+assert_se(hashmap_buckets(m) = 1000);
+assert_se(hashmap_isempty(m));
+
+assert_se(hashmap_put(m, key 1, (void*) val 1) == 1);
+
+assert_se(hashmap_reserve(m, UINT_MAX) == -ENOMEM);
+assert_se(hashmap_reserve(m, UINT_MAX - 1) == -ENOMEM);
+}
+
void test_hashmap_funcs(void) {
test_hashmap_copy();
test_hashmap_get_strv();
@@ -823,4 +840,5 @@ void test_hashmap_funcs(void) {
test_hashmap_steal_first_key();
test_hashmap_steal_first();
test_hashmap_clear_free_free();
+test_hashmap_reserve();
}
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] sysctl.d: default to fq_codel, fight bufferbloat
Quoting from Jon Corbet's report of Stephen Hemminger's talk at Linux Plumbers Conference 2014 (https://lwn.net/Articles/616241/): [...] So Stephen encouraged everybody to run a command like: sysctl -w net.core.default_qdisc=fq_codel That will cause fq_codel to be used for all future connections (up to the next reboot). Unfortunately, the default queuing discipline cannot be changed, since it will certainly disturb some user's workload somewhere. So kernel developers know the best default is fq_codel, but won't change to it in the kernel itself. Instead they expect distros to do it. Let's have the recommended default in systemd. --- sysctl.d/50-default.conf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sysctl.d/50-default.conf b/sysctl.d/50-default.conf index 8fc9ab7..f189233 100644 --- a/sysctl.d/50-default.conf +++ b/sysctl.d/50-default.conf @@ -25,6 +25,9 @@ net.ipv4.conf.all.accept_source_route = 0 net.ipv4.conf.default.promote_secondaries = 1 net.ipv4.conf.all.promote_secondaries = 1 +# Fair Queue CoDel packet scheduler to fight bufferbloat +net.core.default_qdisc = fq_codel + # Enable hard and soft link protection fs.protected_hardlinks = 1 fs.protected_symlinks = 1 -- 2.1.0 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 21/26] hashmap: rewrite the implementation
On 10/16/2014 09:50 AM, Michal Schmidt wrote: --- a/src/shared/hashmap.h +++ b/src/shared/hashmap.h @@ -6,6 +6,7 @@ This file is part of systemd. Copyright 2010 Lennart Poettering + Copyright 2014 Lennart Poettering Oh, I'll fix this before pushing upstream. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 5 commits - configure.ac Makefile.am src/analyze src/bus-proxyd src/cgtop src/core src/delta src/journal src/journal-remote src/libsystemd src/libsystemd-network
On 09/16/2014 12:07 PM, Zbigniew Jędrzejewski-Szmek wrote: Rather than forcing gcc to always produce colorized error messages whether on tty or not, enable automatic colorization by ensuring GCC_COLORS is set to a non-empty string. Hi, this idea was discussed and rejected before, because with this change standard 'make -j8' does not produce color output anymore. Was the discussion on the mailing list? I could not find it. The coloring with make -j8 works for me on Fedora and Debian. Is there a deciding factor I am missing? Wouldn't the lack of colors in this case be a bug in gcc and/or make? Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] units: cleanup agetty command line
On 06/04/2013 10:57 AM, Karel Zak wrote: On Mon, Jun 03, 2013 at 06:00:45PM +0100, Colin Guthrie wrote: From comment 1: Furthermore, I believe there's a bug in agetty. It attempts to detect the distinction between the vc and a serial line and set TERM accordingly, but it does not work as expected. I will send Karel a fix. ... hmm, I don't remember any discussion. The comment referred to the bug fixed by this util-linux commit: commit c13d60b291cfe3e2c094225195d967c9f195ca54 Author: Michal Schmidt mschm...@redhat.com Date: Mon Oct 29 23:33:01 2012 +0100 agetty: fix autodetection for TERM ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [ANNOUNCE] systemd 203
On 05/07/2013 10:53 AM, Colin Guthrie wrote: 'Twas brillig, and Lennart Poettering at 07/05/13 01:41 did gyre and gimble: This is probably a good release to synchronize a distribution on. For example, it is our goal that this is the version we will include in Fedora 19, more or less. You said that about 195 and then promptly shipped newer versions in Fedora there after In Lennart's defense: He was telling the truth as he saw it at that time. As Jóhann noted correctly, I maintain the systemd package in released Fedora branches. The decision to ignore Lennart's suggestion and rebase systemd to v197 shortly after F18 release was mine. My reason for choosing to rebase instead of cherry-picking was that none of the changes between v195 and v197 looked too scary and I wanted to have some of the new features. The few changes that brought incompatibilities could be easily backed out selectively. The rebase was a success. The update met with positive feedback. There's going to be another rebase in F18: https://admin.fedoraproject.org/updates/FEDORA-2013-5452/systemd-201-2.fc18.6 The backed-out changes are listed in a %doc file shipped in the package: http://pkgs.fedoraproject.org/cgit/systemd.git/plain/README.Fedora-18?h=f18 In earlier Fedora releases rebasing systemd would have been a lot more difficult: - The journal arrived upstream soon after the version in F16. - The udev merge happened after the version in F17. These were the major changes that for me ruled out the possibility of rebasing outright. - ourselves and suse listened to that previous advice any synchronised on that version for the ease of maintainability, but that was somewhat scuppered when newer versions showed up in Fedora instead. Sorry. Well, at least you two have each other :-) Such is the way of life, but by the sounds of upcoming changes, it looks like you might genuinely stick with 203 this time :p Yeah, the coming dbus work will probably scare me sufficiently, so there's a good chance F19 will stay on (now current) v204. Regards, Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd release agent
On 04/16/2013 07:45 PM, Kevin Wilson wrote: No I moved /usr/lib/systemd/systemd-cgroups-agent to some backup. and made sure that: ls /usr/lib/systemd/systemd-cgroups-agent ls: cannot access /usr/lib/systemd/systemd-cgroups-agent: No such file or directory Now I tried killing two services that I know are under systemd cgroups: cat /sys/fs/cgroup/systemd/system/bluetooth.service/tasks 671 Apr 16 20:40:05 localhost systemd[1]: bluetooth.service: main process exited, code=killed, status=9/KILL Apr 16 20:40:05 localhost systemd[1]: Unit bluetooth.service entered failed state And with mcelog it was the same: ... Apr 16 20:33:46 localhost systemd[1]: mcelog.service: main process exited, code=killed, status=9/KILL Apr 16 20:33:46 localhost systemd[1]: Unit mcelog.service entered failed state ... both folders, bluetooth.service and mcelog.service (under /sys/fs/cgroup/systemd/system/) were removed. How come ? could it be that the messages to the DBus are not sent by systemd-cgroups-agent? In these two cases it was sufficient for systemd to receive the SIGCHLD signal to detect the death of the service. But this won't work for every service. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] system shutdown: what unit is taking so long?
On 04/17/2013 04:33 PM, Lennart Poettering wrote: It's so pretty in fact, that it almost makes me want to add sleep(30) or so to all my daemons' startup and shutdown code just to see it. It's like KITT, but in ASCII! No, no, you're not supposed to like it. You should tremble in fear! It's not a cute KITT, it's a scary Cylon's eye! Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/3] unit: do not care if order isn't right when retroactively starting deps
On 03/23/2013 03:14 AM, Lennart Poettering wrote: On Wed, 13.03.13 01:44, Michal Schmidt (mschm...@redhat.com) wrote: Attempt to satisfy requirement dependencies retroactively even if the unexpectedly activated unit would prefer to be started After them. This way remote-fs-pre.target can be pulled in by performing a manual mount (the mount units have both Wants= and After= remote-fs-pre.target). I am a bit concerned abou this. Wouldn't this also mean that if a mount for /foobar/waldo suddenly shows up we'd still retroactively mount /foobar too, if that happens to have a unit file? That sounds wrong, no? You are right. It would do exactly this wrong thing. I need to rethink. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 0/3] alternative fix for manual mounts shutdown ordering
Harald's patch remote-fs.target: want remote-fs-pre.target fixed the problem with shutdown ordering of manually mounted network filesystems, but it also caused NetworkManager-wait-online.service to be pulled in needlessly when no network mounts are used. I said on IRC that I intended to revert that if we can make the manual mount units themselves pull remote-fs-pre.target in retroactively. Here's my proposal. Michal Schmidt (3): Revert remote-fs.target: want remote-fs-pre.target unit: do not care if order isn't right when retroactively starting deps unit: do not start Requisite units retroactively src/core/unit.c| 17 - units/remote-fs.target | 2 -- 2 files changed, 4 insertions(+), 15 deletions(-) -- 1.8.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/3] Revert remote-fs.target: want remote-fs-pre.target
This reverts commit 6bde0b3220e95a08cefb70846f73b2cf24b7734a. remote-fs.target is usually enabled whether there are any remote mounts in fstab or not. remote-fs-pre.target pulls in NetworkManager-wait-online.service. = The commit caused NM-w-o to be pulled into boot. Let's fix the problem of the missing remote-fs-pre.target differently in the next patch. --- units/remote-fs.target | 2 -- 1 file changed, 2 deletions(-) diff --git a/units/remote-fs.target b/units/remote-fs.target index b11074f..9e68878 100644 --- a/units/remote-fs.target +++ b/units/remote-fs.target @@ -8,8 +8,6 @@ [Unit] Description=Remote File Systems Documentation=man:systemd.special(7) -Wants=remote-fs-pre.target -After=remote-fs-pre.target [Install] WantedBy=multi-user.target -- 1.8.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/3] unit: do not care if order isn't right when retroactively starting deps
Attempt to satisfy requirement dependencies retroactively even if
the unexpectedly activated unit would prefer to be started After them.
This way remote-fs-pre.target can be pulled in by performing a manual
mount (the mount units have both Wants= and After= remote-fs-pre.target).
---
src/core/unit.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/core/unit.c b/src/core/unit.c
index a6cc3b6..d1f109d 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1226,28 +1226,23 @@ static void retroactively_start_dependencies(Unit *u) {
assert(UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)));
SET_FOREACH(other, u-dependencies[UNIT_REQUIRES], i)
-if (!set_get(u-dependencies[UNIT_AFTER], other)
-!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
+if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
manager_add_job(u-manager, JOB_START, other,
JOB_REPLACE, true, NULL, NULL);
SET_FOREACH(other, u-dependencies[UNIT_BINDS_TO], i)
-if (!set_get(u-dependencies[UNIT_AFTER], other)
-!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
+if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
manager_add_job(u-manager, JOB_START, other,
JOB_REPLACE, true, NULL, NULL);
SET_FOREACH(other, u-dependencies[UNIT_REQUIRES_OVERRIDABLE], i)
-if (!set_get(u-dependencies[UNIT_AFTER], other)
-!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
+if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
manager_add_job(u-manager, JOB_START, other,
JOB_FAIL, false, NULL, NULL);
SET_FOREACH(other, u-dependencies[UNIT_REQUISITE], i)
-if (!set_get(u-dependencies[UNIT_AFTER], other)
-!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
+if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
manager_add_job(u-manager, JOB_START, other,
JOB_REPLACE, true, NULL, NULL);
SET_FOREACH(other, u-dependencies[UNIT_WANTS], i)
-if (!set_get(u-dependencies[UNIT_AFTER], other)
-!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
+if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
manager_add_job(u-manager, JOB_START, other,
JOB_FAIL, false, NULL, NULL);
SET_FOREACH(other, u-dependencies[UNIT_CONFLICTS], i)
--
1.8.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/3] unit: do not start Requisite units retroactively
Activating Requisite units goes against the reason of existence of this
dependency type.
---
src/core/unit.c | 4
1 file changed, 4 deletions(-)
diff --git a/src/core/unit.c b/src/core/unit.c
index d1f109d..25109ce 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1237,10 +1237,6 @@ static void retroactively_start_dependencies(Unit *u) {
if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
manager_add_job(u-manager, JOB_START, other,
JOB_FAIL, false, NULL, NULL);
-SET_FOREACH(other, u-dependencies[UNIT_REQUISITE], i)
-if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
-manager_add_job(u-manager, JOB_START, other,
JOB_REPLACE, true, NULL, NULL);
-
SET_FOREACH(other, u-dependencies[UNIT_WANTS], i)
if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
manager_add_job(u-manager, JOB_START, other,
JOB_FAIL, false, NULL, NULL);
--
1.8.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] About “systemd-cat --priority=0”
Dne 8.3.2013 10:57, Yan Lei napsal(a): Sorry for troubling you, but I have got an issue to confirm with you. My issue is when I use the following command: systemd-cat --priority=0 echo hello Nothing logged into the journal, and the exit code is 141. I wonder whether it is a bug or it just work as expected. Looks like a bug to me. Fixed now in git: http://cgit.freedesktop.org/systemd/systemd/commit/?id=e14ddf1cac12f91685cbd5dac6c5127f8cf87863 Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: fix getting information about mount unit
On 03/08/2013 03:13 PM, Michal Sekletar wrote:
@@ -447,7 +447,11 @@ static int mount_add_default_dependencies(Mount *m) {
if (UNIT(m)-manager-running_as != SYSTEMD_SYSTEM)
return 0;
-p = get_mount_parameters_fragment(m);
+if (m-from_fragment)
+p = get_mount_parameters_fragment(m);
+else
+p = get_mount_parameters(m);
+
Wouldn't it be sufficient to replace the one line?:
-p = get_mount_parameters_fragment(m);
+p = get_mount_parameters(m);
?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: fix getting information about mount unit
On 03/08/2013 03:25 PM, Umut Tezduyar wrote: I thought mounts coming from mountinfo are not getting default dependencies anyways. mount_add_one() never sets load_extras to true for new mount units. But it does call unit_add_to_load_queue(u), which should eventually result in mount_load() getting called, and from there unit_load_fragment_and_dropin_optional() and mount_add_extras(). Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] core: fix running jobs counters after reload/reexec
All active units will call unit_notify() during coldplug, so we just
make sure we're counting from zero again and get the correct result for
n_on_console.
For n_running_jobs we likewise reset it to zero and then count
the running jobs as we encounter them in deserialization.
---
src/core/manager.c | 3 +++
src/core/unit.c| 8 +++-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/core/manager.c b/src/core/manager.c
index 5527e9d..4242398 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -678,6 +678,9 @@ static void manager_clear_jobs_and_units(Manager *m) {
assert(hashmap_isempty(m-jobs));
assert(hashmap_isempty(m-units));
+
+m-n_on_console = 0;
+m-n_running_jobs = 0;
}
void manager_free(Manager *m) {
diff --git a/src/core/unit.c b/src/core/unit.c
index 601be60..2f0ac00 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1360,11 +1360,6 @@ void unit_notify(Unit *u, UnitActiveState os,
UnitActiveState ns, bool reload_su
if (UNIT_IS_INACTIVE_OR_FAILED(os) != UNIT_IS_INACTIVE_OR_FAILED(ns)) {
ExecContext *ec = unit_get_exec_context(u);
if (ec exec_context_may_touch_console(ec)) {
-/* XXX The counter may get out of sync if the admin
edits
- * TTY-related unit file properties and issues a
daemon-reload
- * while the unit is active. No big deal though,
because
- * it influences only the printing of boot/shutdown
- * status messages. */
if (UNIT_IS_INACTIVE_OR_FAILED(ns))
m-n_on_console--;
else
@@ -2446,6 +2441,9 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) {
return r;
}
+if (j-state == JOB_RUNNING)
+u-manager-n_running_jobs++;
+
r = job_install_deserialized(j);
if (r 0) {
hashmap_remove(u-manager-jobs,
UINT32_TO_PTR(j-id));
--
1.8.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 5/6] coredumpctl: null check before dereferencing
On 10/25/2012 04:16 PM, Michal Sekletar wrote:
---
src/journal/coredumpctl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/journal/coredumpctl.c b/src/journal/coredumpctl.c
index d15a31e..1a4d78f 100644
--- a/src/journal/coredumpctl.c
+++ b/src/journal/coredumpctl.c
@@ -222,6 +222,7 @@ static int retrieve(sd_journal *j, const char *name, const
char **var) {
field = strlen(name) + 1; // name + =
assert(len = field);
+assert(var);
*var = strndup((const char*)data + field, len - field);
if (!var)
It looks like this check really wants to be if (!*var)
Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/6] journal: fix memleak, call set_free before return
Dne 25.10.2012 16:16, Michal Sekletar napsal(a):
---
src/journal/coredumpctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/journal/coredumpctl.c b/src/journal/coredumpctl.c
index 5c442ff..d15a31e 100644
--- a/src/journal/coredumpctl.c
+++ b/src/journal/coredumpctl.c
@@ -58,6 +58,7 @@ static Set *new_matches(void) {
if (!tmp) {
log_oom();
set_clear_free(set);
+set_free(set);
return NULL;
}
OK, set_free(set) is correct here. The set is still empty, so calling
set_clear_free(set) is pointless. Remove it.
@@ -66,6 +67,7 @@ static Set *new_matches(void) {
log_error(failed to add to set: %s, strerror(-r));
free(tmp);
set_clear_free(set);
+set_free(set);
return NULL;
}
Same here.
Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/6] localectl: fix memleak, call set_free before return
Dne 25.10.2012 16:16, Michal Sekletar napsal(a):
---
src/locale/localectl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index c05eba0..16700fe 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -354,6 +354,7 @@ static int list_locales(DBusConnection *bus, char **args,
unsigned n) {
h-locrectab_offset + h-locrectab_size st.st_size ||
h-sumhash_offset + h-sumhash_size st.st_size) {
log_error(Invalid archive file.);
+set_free(locales);
return -EBADMSG;
}
Just use the single return point like the other error paths in this
function.
Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 3/6] localectl: fix memleak, use _cleanup_strv_free_
Dne 25.10.2012 16:16, Michal Sekletar napsal(a):
l might contain zero strings, however there is still memory
allocated for NULL terminator, use _cleanup_strv_free_ instead to
prevent tiny leak in such case.
---
src/locale/localectl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index 16700fe..2f0f6ee 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -483,7 +483,8 @@ static int nftw_cb(
}
static int list_vconsole_keymaps(DBusConnection *bus, char **args, unsigned
n) {
-char **l, **i;
+char _cleanup_strv_free_ **l;
+char **i;
keymaps = set_new(string_hash_func, string_compare_func);
if (!keymaps)
When using _cleanup_strv_free you must initialize l to NULL.
Also you forgot that there's one explicit strv_free(l) in the function.
That would result in freeing it twice.
Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 6/6] sysctl: parse all keys in a config file
Dne 25.10.2012 16:16, Michal Sekletar napsal(a):
https://bugzilla.redhat.com/show_bug.cgi?id=869779
---
src/sysctl/sysctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c
index a68d67f..035e0ec 100644
--- a/src/sysctl/sysctl.c
+++ b/src/sysctl/sysctl.c
@@ -178,7 +178,7 @@ static int parse_file(const char *path, bool ignore_enoent)
{
free(property);
free(new_value);
-if (r != -EEXIST)
+if (r != 0)
goto finish;
}
}
Applied this one.
Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] List all available units
On 08/08/2012 10:21 AM, Václav Pavlín wrote: If we decide to change behaviour of systemctl, the approach, I described here, can be used. If you prefer to edit autocomplete script, it can be done with merge of sytemctl output of list-units and list-unit-files and then pipe to uniq. A lot of users find the behaviour of systemctl --all confusing. If you can find a nice way to make it display even not-currently-loaded units, it would make them happy. And I assume the autocomplete would start working as expected too. See https://bugzilla.redhat.com/show_bug.cgi?id=748512 Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd: enable/disable instances of template
On 07/13/2012 03:59 PM, Michal Sekletar wrote: https://bugzilla.redhat.com/show_bug.cgi?id=752774 --- man/systemctl.xml | 33 +++- src/shared/install.c | 78 +--- src/shared/unit-name.c | 12 src/shared/unit-name.h |1 + 4 files changed, 100 insertions(+), 24 deletions(-) Applied, thanks! ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd: support for new timeouts in .service
On 07/20/2012 12:08 PM, Michal Sekletar wrote: we found this bug https://bugzilla.redhat.com/show_bug.cgi?id=807885 and we already invested some effort to introducing new timeout types. We have a patch prepared introducing new service timeout types, namely TimoutStartSec and TimeoutStopSec. I agree with the decision to introduce the two new timeout options, while still supporting TimeoutSec for compatibility. Semantic would be as follows. Old timeout TimeoutSec defines common timeout for both, start and stop of service. If new timeout is used it will override current setting, which is either specified by TimeoutSec or default. However another variant which was proposed by Michal Schmidt, changes described behavior in a way that timeout which is specified last in unit file is applied and always overrides the previous settings. I don't think this description explains the difference well enough, so let me help by providing some examples: Example #1: /usr/lib/systemd/system/foo.service: ... [Service] TimeoutSec=100 ... /etc/systemd/system/foo.service: .include /usr/lib/systemd/system/foo.service [Service] TimeoutStopSec=200 In this example I believe we both agree that the result would be: start timeout = 100 stop timeout = 200 Example #2: /usr/lib/systemd/system/foo.service: ... [Service] TimeoutStartSec=50 TimeoutStopSec=100 ... /etc/systemd/system/foo.service: .include /usr/lib/systemd/system/foo.service [Service] TimeoutSec=200 In this example we disagree. In your proposal the TimeoutSec would be entirely ignored, because new timeout options were used. I.e. the result would be: start timeout = 50 stop timeout = 100 With my proposal the result would be: start timeout = 200 stop timeout = 200 because the TimeoutSec would have the same effect as setting both TimeoutStartSec= and TimeoutStopSec=. The advantage of my proposal is that the administrator's override (which may have been in place before the introduction of the new timeout options) is still effective. We are interested in opinion of systemd community in regard to this matter. Thank you. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd: return error when asked to stop unknown unit
On 06/19/2012 10:00 AM, Michal Sekletar wrote:
+if (job_type == JOB_STOP u-load_state == UNIT_ERROR
unit_active_state(u) == UNIT_INACTIVE) {
In case anyone's wondering why there's the check for the unit's active
state: It is possible to have units that are active and have a load
error at the same time. We want to be able to stop these units.
Thanks, applied.
Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] git branches of stable Fedora releases
Hello, a few people were interested in seeing the git tree from which I generate the patches for source RPMs in Fedora. The tree is now available at: git://fedorapeople.org/~michich/systemd.git Gitweb is here: http://fedorapeople.org/gitweb?p=michich/public_git/systemd.git The branches of interest are f16 and f17. As each Fedora release grows older, the criteria by which I choose what patches to add from upstream to the branch get increasingly strict: - At the beginning I add bugfixes and even some features, as long as they're not known to cause incompatibilities or regressions. - Later I start avoiding features and add only bugfixes. - If Fedora N is already released, the branch for Fedora N-1 will get only fixes for bugs I consider important or bugs specifically reported by users of Fedora N-1. Regards, Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Showing plymouth shutdown splash earlier during shutdown process
On 05/31/2012 05:46 PM, Daniel Drake wrote: In the case of reboot (or poweroff), what does this mean? plymouth-reboot.service is queued to start, and prefdm.service is queued to stop. What does After= mean in this context, who comes first? 'man systemd.unit' says: If one unit with an ordering dependency on another unit is shut down while the latter is started up, the shut down is ordered before the start-up regardless whether the ordering dependency is actually of type After= or Before=. It is like it is waiting for those services to stop before executing. How can I find out why? Based on the above rule, check all the ordering dependencies the unit has: systemctl show -p After -p Before plymouth-reboot.service Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Have custom agetty behaviour even after upgadres
On 05/16/2012 01:07 PM, Colin Guthrie wrote: This thread went a little tangential, so I'd like to bring it back to the problem stated here. Firstly, that was not my understanding of how things are supposed to work, but perhaps Lennart or Kay can clarify. I thought that the actual end point of the symlink was not all that important... e.g. if I have: /usr/lib/systemd/system/getty@.service /etc/lib/systemd/system/getty@.service /etc/lib/systemd/system/multi-user.target.wants/getty@tty1.service - /usr/lib/systemd/system/getty@.service I thought that the unit file /etc/lib/systemd/system/getty@.service was still the one used. i.e. the symlink is merely indicative of whether the service is enabled or not, and the actual physical file that it points to is not relevant. i.e. The .wants symlink only really states I'm enabled as an instance of getty@.service and then the normal inheritance rules of getty@.service resolution apply *after* that, i.e. getty@.service in /etc/... overrides the one in /lib/... This is maybe not intuitive when looking solely at the symlinks themselves, but it is when you think about what they represent. Colin, your description matches my understanding. In my test it works as expected with ordinary units, but not if templates are involved. I consider it a bug. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-cgls - Memory overflow
On 05/10/2012 02:45 PM, Sven Anders wrote: If I execute the systemd-cgls command, I see the tree but without the command lines of the executables. I only see n/a. open(/proc/300/cmdline, O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 7 mmap2(NULL, 1075859456, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = -1 ENOMEM (Cann ot allocate memory) Do you have the COLUMNS environment variable set to some interesting value? Does it work if you set it to something sane?: COLUMNS=80 systemd-cgls Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Warning to Distros: NetworkManager + systemd + nscd + nmcli deadlock issue on boot.
Colin Guthrie wrote: So in this situation, as systemd is aiming for network.target, but it's not actually reached it yet, systemctl condrestart nscd appears to hang, presumably waiting for network.target to be reached (as nscd.service is ordered After network.target). Does reverting commit dd17d38 job: fix loss of ordering with restart jobs help? As noted in the patch description, try-restart needs to be improved to finish quickly when the service is not running. I will be looking into this. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] automount regression
Tom Gundersen wrote: It seems this was caused by: commit 9ddc4a26e56b06cd7774a03597980351855d8d54 Author: Michal Schmidt mschm...@redhat.com Date: Fri Jan 13 23:55:28 2012 +0100 mount: fix quota quotacheck.service and quotaon.service were not pulled in for fstab mounts. Fix it by not clearing the default_dependencies flag. The root filesystem may have quotas too, so don't check for / there. No need to have duplicate code for adding dependencies on umount.target. https://bugzilla.redhat.com/show_bug.cgi?id=773431 The following (partial) revert fixed my problem, but I'm not sure what the proper fix would be. Thanks for finding the bug. The proper fix only needed one more change. Fixed in commit da375869. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [ANNOUNCE] systemd v39
On 01/25/2012 03:57 PM, Lennart Poettering wrote: On Wed, 25.01.12 11:11, Jan Engelhardt (jeng...@medozas.de) wrote: I would actually prefer if it wrote that to the current tty that invoked the start action, rather than the console which is stowed away in a deep cellar... We explicitly want to avoid that services are entirely isolated from the user session they are started from. Running a service with the tty of the user running the command would be the absolute opposite of isolated. We could make systemctl start ... dump a few of the service's lines from the journal before exiting. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [ANNOUNCE] systemd v39
On 01/25/2012 05:34 PM, Bill Nottingham wrote: Michal Schmidt (mschm...@redhat.com) said: We could make systemctl start ... dump a few of the service's lines from the journal before exiting. At that point you're then trying to define heuristics about what the proper value for 'a few' is, and then you're guaranteed to get it wrong for some set of services. Not sure it's worth the effort. 'A few' could be defined as everything the service wrote between the time we told it to start and the time it finished starting (or failed). systemd has the timestamps. Or these important events can be found in the journal as well. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] socket failed to queue socket startup job: Transport endpoint is not connected
Albert Strasheim wrote: Any chance this can be rolled into an update for Fedora 16? Yes. systemd-37-10.fc16 is in updates-testing: https://admin.fedoraproject.org/updates/FEDORA-2012-0409 It's probably a bit tricky since journald landed in v38. I guess you would either have to branch off v37.1 for fixes only, or package v39 without enabling journald and related changes on F16? You are right that journald makes it a bit tricky. I maintain systemd in F16 using a local git branch I call 'f16'. I use: git format-patch -o $FEDPKG_SYSTEMD_DIR -N v37..f16 to export it as patch files for building the RPM package. I do not publish this 'f16' branch itself (should I?), but you can see the patch files stored in Fedora Git using: fedpkg clone -a systemd cd systemd # note that this is not the 'f16' mentioned above, # it's an entirely different repo: git checkout f16 Or browse: http://pkgs.fedoraproject.org/gitweb/?p=systemd.git;a=tree;h=refs/heads/f16 Regards, Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd+dbus: system boot stops at terminal login screen sometimes
- Original Message - Does the deadlock go away if you just modify rsyslog.service so that instead of stopping systemd-kmsg-syslogd in ExecStartPre it would do it in ExecStartPost? Yeah, it do the trick. Great. People who are hitting this deadlock can use this as a temprorary workaround. IMHO, We need to put functions that may block in separate threads, for example bus_init(), shutdown_connection(), log_meta(). Oh, I'm sure this works, but I'm scared of the additional complexity of threads. Agree, but IMHO, from the architectural point of view, smooth running of systemd should not depend on quick response of outside processes(rsyslogd, dbus). Yes. systemd at least partially defends itself from a hanging syslog by setting SO_SNDTIMEO to 5 seconds. Maybe even that is too much and we could just make the socket completely non-blocking and just fallback to kmsg when we get EAGAIN. I believe there is a way to connect and register to DBus fully asynchronously too. I'm looking into this. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd+dbus: system boot stops at terminal login screen sometimes
Michal Schmidt wrote: systemd at least partially defends itself from a hanging syslog by setting SO_SNDTIMEO to 5 seconds. Maybe even that is too much and we could just make the socket completely non-blocking and just fallback to kmsg when we get EAGAIN. I believe there is a way to connect and register to DBus fully asynchronously too. I'm looking into this. It's now implemented in git. Please give it a try. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd+dbus: system boot stops at terminal login screen sometimes
Chen Jie wrote: Let me explain it further. First syslogv() in dbus may block, while rsyslog.service is starting and meanwhile the kernel socket buffer was full. Attachment syslog-test.c was a program simulates the situation. So while rsyslog.service is starting, on the systemd side: 1. [ExecStartPre] stops systemd-kmsg-syslogd.service 2. It now running bus_init() to publish its API to dbus 3. Step 2 timeout, because dbus-daemon didn't reply in time. Why dbus-daemon didn't response quickly? Because it blocked on syslogv(), which was waiting for someone consumes the message(the kernel socket buffer was full), but sadly the consumer -- rsyslog didn't be started because systemd blocked. So systemd waits for dbus, and dbus waits for startup of rsyslog, rsyslog waits for systemd to start it. Thanks for debugging this. You are right. This kind of a deadlock is possible. Does the deadlock go away if you just modify rsyslog.service so that instead of stopping systemd-kmsg-syslogd in ExecStartPre it would do it in ExecStartPost? IMHO, We need to put functions that may block in separate threads, for example bus_init(), shutdown_connection(), log_meta(). Oh, I'm sure this works, but I'm scared of the additional complexity of threads. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] ExecStartPost= behavior on failure
On 11/15/2011 01:40 PM, Honza Horak wrote: I'm thinking of what is the desired behavior if the command ExecStartPost=somecommand fails. If I understand it correctly, all other ExecStartPost= commands execution is stopped, but the main process continues to work (and the service is still active). From my POV this should happen when ExecStartPost= starts with '-'. But when '-' is not used, the service as well as the main process should be ended by systemd. I agree and I have fixed this in git. http://cgit.freedesktop.org/systemd/commit/?id=2096e009a790073a934f5cd07d17024d3b199d0b Thanks for pointing it out. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Allow 'list-unit-files' to run with --root.
On Tue, 22 Nov 2011 15:45:34 -0500 Bill Nottingham wrote: To do so, move the check for the bus to the bus-using portion of list_unit_files(), and ensure that get_config_path doesn't abort when checking the runtime path with --root. --- src/install.c |5 ++--- src/systemctl.c |5 +++-- 2 files changed, 5 insertions(+), 5 deletions(-) Applied, thanks. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Run rc-local after network.target, as it's often used for frobbing the network.
On 11/22/2011 08:57 PM, Bill Nottingham wrote: diff --git a/units/fedora/rc-local.service b/units/fedora/rc-local.service index 106b12c..9a38e59 100644 --- a/units/fedora/rc-local.service +++ b/units/fedora/rc-local.service @@ -8,6 +8,7 @@ [Unit] Description=/etc/rc.local Compatibility ConditionFileIsExecutable=/etc/rc.d/rc.local +After=network.target [Service] Type=forking Yes, I suppose that people using rc.local want to have it run after the network is ready. The trouble is that this will delay services that are ordered after rc-local.service even if one has no /etc/rc.d/rc.local file. The unit acts as an ordering barrier even if its condition is false, because conditions are evaluated when the service is about to be started. To get rid of this ordering barrier in the usual case, we could add a generator to produce rc-local.service only if /etc/rc.d/rc.local is executable (or the other way around: the generator could runtime-mask the existing rc-local.service if the file is not executable.). Or add support for some kind of early evaluated conditions. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd+dbus: system boot stops at terminal login screen sometimes
On 11/30/2011 10:49 AM, Chen Jie wrote: (see the full syslog at http://lists.freedesktop.org/archives/dbus/attachments/2025/e9c204bb/attachment-0001.obj) The kernel modules failing to load indicate a problem with your kernel installation. Please sort this problem out first before attempting to debug anything else. BTW, the gap between systemd-kmsg-syslogd.service stopped and rsyslog not ready will make it lost some log messages. No, the socket remains open all the time and the messages will be buffered. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] getty login prompt
On 11/23/2011 07:26 AM, Devendra Talegaonkar wrote: 30systemd-stdout-syslog-bridge[78]: Failed to accept new connection: Function not implemented Your kernel or libc does not support the accept4() syscall. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd diagnostics
On 11/22/2011 04:36 AM, Edward Z. Yang wrote: 3. Listing enabled services. We should not have to write horrible scripts like this: In F16 you can use systemctl list-unit-files. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] openvpn@.service
On Fri, 18 Nov 2011 19:16:50 -0500 Michael D. Berger wrote: So now if I want to enable openvpn so it would start on boot, what do I do? ln -s /lib/systemd/system/openvpn@.service \ /etc/systemd/system/multi-user.target.wants/openvpn@foo.service Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] httpd fails to start on boot
On Fri, 18 Nov 2011 19:21:46 -0500 Michael D. Berger wrote: Even though it is enabled, httpd fails to start on boot. After boot, I can start it with no problem. I do have a rather complex httpd.conf . What does systemctl status httpd.service say after boot? Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] httpd fails to start on boot
On Fri, 18 Nov 2011 20:06:59 -0500 Michael D. Berger wrote: At level 3, seconds after boot: # systemctl status httpd.service httpd.service - The Apache HTTP Server (prefork MPM) Loaded: loaded (/lib/systemd/system/httpd.service; enabled) Active: failed since Fri, 18 Nov 2011 19:50:58 -0500; 1min 11s ago Process: 985 ExecStart=/usr/sbin/httpd $OPTIONS -k start (code=exited, status=1/FAILURE) CGroup: name=systemd:/system/httpd.service Anything logged in /var/log/httpd/ ? Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] openvpn@.service
On 11/15/2011 10:10 PM, Mirco Tischler wrote: 2011/11/15 Manuel Amador (Rudd-O)rud...@rudd-o.com: It's a multi-instance service. You can instance it several times based on a parameter, much like tty@.service can be instantiated to be tty@tty1.service. Correct. In the instances of openvpn@.service the @ should be followed by the name of the config file that the instance will use (without the .conf suffix). Most certainly it is a socket activated service in the inetd style, i.e. one service instance per connection. inetd-style socket activated services use template units too, but openvpn@.service in F16 is not an inetd-style service. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] random-seed: break ordering cycle with encrypted tmp partitions
On 11/15/2011 09:20 AM, Tom Gundersen wrote: -After=systemd-readahead-collect.service systemd-readahead-replay.service local-fs.target +After=systemd-readahead-collect.service systemd-readahead-replay.service rootfs-remount.service @localstatedir@.mount What if /var/lib itself is a separate mount too? Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] openvpn@.service
On 11/16/2011 04:14 PM, Michael D. Berger wrote: . On F16_64 can I now do: systemctl start openvpn.service , or perhaps: systemctl start openvpn@.service , or do I have to do something else? If your config file is /etc/openvpn/foo.conf, then: systemctl start openvpn@foo.service Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] can not systemctl enable rc.local in final fc16
On 11/14/2011 11:00 AM, floydsm...@aol.com wrote: I can not systemctl enable rc.local.service in final fc16 - error is: Failed to issue method call: No such file or directory The unit is called rc-local.service. $ systemctl list-unit-files|grep rc-local rc-local.servicestatic Notice the static. It means you are not expected to enable/disable the unit. If you try, you'll get: $ sudo systemctl enable rc-local.service Warning: unit files do not carry install information. No operation executed. The unit is linked from /lib/systemd/system/multi-user.target.wants/rc-local.service and it will be started if /etc/rc.d/rc.local is executable. Make sure you have the right shebang line in the script (e.g. #!/bin/sh) Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Fix same expression on both sides of ''
On Wed, 09 Nov 2011 20:48:31 +0100 Thomas Jarosch wrote: The code should probably look like the statements above it. Please verify, I just detected it using cppcheck. Patch applied. Thanks! Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] contingent After
On 11/05/2011 06:09 PM, Michael D. Berger wrote: Thanks to the help provided by members of the list, myDaemon is now functioning correctly under systemd. Now for myUpperDaemon: IF myDaemon is enabled; then myUpperDaemon must start after myDaemon ELSE myUpperDaemon must start anyway ENDIF This exactly what After=myDaemon.service is for. Realize that in systemd you can express requirement dependencies (Wants, Requires, ...) completely independently of ordering dependencies (Before, After). The situation you described is a case of an ordering dependency, without a requirement dependency. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] FW: pthread_create() fails SysV in myDaemon on boot
On Thu, 03 Nov 2011 12:41:05 -0400 Michael D. Berger wrote: However, in starting a TCP listener socket, from bind(...) I now get EADDRNOTAVAIL which is explained in man 2 bind, so I am still not operational. Do you use NetworkManager? The network interface may not be up yet when your service starts. Does enabling NetworkManager-wait-online.service help? Have you considered using the IP_FREEBIND socket option to avoid the need for ordering your service after network.target? See: http://www.kernel.org/doc/man-pages/online/pages/man7/ip.7.html Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] FW: pthread_create() fails SysV in myDaemon on boot
On 11/04/2011 03:11 AM, Michael D. Berger wrote: [Service] ControlGroupAttribute=cpu.rt_runtime_us 50 ExecStart=/usr/sbin/myDaemon --daemon I'm guessing --daemon tells the program to daemonize itself. In that case you need to add Type=forking and PIDFile= See man systemd.service for what other possibilities of Type= there are. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl-completion: always invoke with --no-legend
On Tue, 11 Oct 2011 20:56:53 -0400 Dave Reisner wrote: In the case of completion for the 'restart' verb, passing the invalid unit name (the colums header) causes completion to cease functioning entirely, with the error: Failed to issue method call: Unit name UNIT is not valid. This adds a small wrapper function for systemctl which can have common options added to it. Thanks! I applied it. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Seeking Advice on debugging failure to reboot
On 10/12/2011 03:46 PM, Barry Scott wrote: Thanks to Michal's observation that swapoff failed we have now found the root cause. swapoff is called while all our production processes are still running. We would have expected systemd to turn off swap after stopping most if not all processes and thus freeing up as much memory as possible. swapoff fails becuase there is not enough ram available to load the pages from swap into. Is this a systemd problem or an issue with the way we wrote our services and targets? Do you use DefaultDependencies=false in any of your service unit files? That could explain the missing ordering. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Seeking Advice on debugging failure to reboot
On 10/12/2011 04:54 PM, Barry Scott wrote: What dependency is supposed to cause the swapoff to be after the production processes are stopped? The units' ordering should be something like this: *.swap Before swap.target Before sysinit.target Before basic.target Before production.service This ordering is used for startup. When shutting down, they should get stopped in the reverse order. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Seeking Advice on debugging failure to reboot
On 10/11/2011 12:44 PM, Barry Scott wrote: The full log is here: http://onelanftp.co.uk/bscott/ntb10117.netconsole.txt The first boot sequence from lines 1-182 works. The second from 188-1182 has the problem. If the kernel is at fault then I would expect to see the About to execute: /bin/systemctl --force reboot line and a failure to reboot. But given that line is not output I suspect its more likely to be an issue with systemd not getting to that point. And question is what stops this happening? In the second run umount.target is not reached. It is not obvious why. The log shows only /data, /boot and /media being unmounted in both runs. There may be a mountpoint that refuses to die. Did you try waiting a few minutes to see if any timeout kicks in? It is unfortunate that we only see the messages after syslog is stopped. Try with systemd.log_target=console, or just stop rsyslog.service manually before invoking reboot. This way netconsole should show more of the events. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Seeking Advice on debugging failure to reboot
On 10/11/2011 05:09 PM, Barry Scott wrote: I have uploaded full netconsole logs with the options you suggested: http://onelanftp.co.uk/bscott/netconsole.noreboot.txt http://onelanftp.co.uk/bscott/netconsole.reboot.txt A suspicious moment occurring only in the failing case is: [ 751.743255] swapoff[7788]: swapoff: /dev/dm-1: swapoff failed: Cannot allocate memory ... [ 751.830279] systemd[1]: dev-dm\x2d1.swap changed deactivating - failed I wonder if this could prevent the reaching of umount.target. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Seeking Advice on debugging failure to reboot
On 10/06/2011 07:26 PM, Barry Scott wrote: On Thursday 06 October 2011 15:56:43 Barry Scott wrote: We can reproducably get an F15 system in a state that it fails to complete a reboot. Then can you suggest how to reproduce it? The obvious difference between a reboot that works and one that fails is that I see these lines in the good case: [ 135.391206] systemd[1]: final.target changed dead - active [ 135.391217] systemd[1]: Job final.target/start finished, result=done [ 135.391249] systemd[1]: About to execute: /bin/systemctl --force reboot What is printed before these lines? Could you attach the complete log? Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] F15: PathExistsGlob
On 10/04/2011 02:22 PM, Reindl Harald wrote: would it be possible to backport PathExistsGlob for F15 https://bugzilla.redhat.com/show_bug.cgi?id=734435 Possible, yes. The bug is not relevant for F15 though, because cups is a SysV/LSB service there. this means we can not make a backport-rebuild for F15 without reverting this patch currently :-( Curious. What's the reason for backporting cups? Backporting of packages sometimes requires more effort than a simple rebuild. That should not be surprising. That said, if you prepare a cleanly applicable and tested patch against the systemd package in F15 to add the feature you need, I can apply it. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] How view a systemd snapshot?
On 09/29/2011 10:29 AM, Nick Urbanik wrote: Dear Folks, I have made a systemd snapshot with systemctl snapshot. How do I view (or record elsewhere) the contents of that snapshot? systemctl show foo.snapshot How is the snapshot stored on the disk? The snapshot unit is generated in memory only. It is in not stored on the disk. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd equivalent of chkconfig --list | grep '3:on'
On 09/29/2011 09:30 AM, Nick Urbanik wrote: How do I do the equivalent of chkconfig --list | grep '3:on'? There's a convenient command systemctl list-unit-files, but it is not available in the version of systemd in Fedora 15. You can explore the /etc/systemd/system/*.target.wants/ directories. It would be less effort to re-install, but I want to understand this systemd, and would appreciate any pointers to how I can better diagnose these serious problems. Since ssh login works, you can get some debug information. Boot with log_buf_len=1M systemd.log_level=debug systemd.log_target=kmsg, login via ssh and save the output of the 'dmesg' command. The output of 'systemctl dump' may also be helpful. Paste them somewhere online where we can take a look. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart=always and ExecStartPre
On Wed, 28 Sep 2011 17:49:54 +0200 Reindl Harald wrote: why does systemd not restart a killed service if the ExecStartPre-process is still running, see below - at my opinion after killall afpd the service should be restarted and in a perfect case even if ExecStartPre-process dies systemd-26-10.fc15.x86_64 I see at least three issues here. See below. [root@testserver:~]$ cat /lib/systemd/system/netatalk.service [Unit] Description=Apple-File-Server After=syslog.target network.target avahi-daemon.service [Service] Type=forking PIDFile=/var/run/netatalk.pid ExecStartPre=/usr/sbin/cnid_metad -l log_note issue #1: ExecStartPre is not supposed to fork off daemons. A future version of systemd might even enforce this rule. The service should be split into two. ... [root@testserver:~]$ systemctl status netatalk.service netatalk.service - Apple-File-Server Loaded: loaded (/lib/systemd/system/netatalk.service) Active: active (running) since Wed, 28 Sep 2011 17:45:55 ... Main PID: 1812 (code=exited, status=0/SUCCESS) issue #2: This Main PID looks like stale information from a previous run of the service. This is a minor bug in systemd that it does not reset it. There are two reasons why systemd failed to detect the new main PID: - issue #3: afpd has a racy daemonization sequence. It writes its PID file too late. My recently proposed patch service: delayed main PID guessing should be able to workaround it. - Given that systemd could not read the information from the PID file, it tried to guess the main PID, but it also failed, because there are two top-level processes in the cgroup: CGroup: name=systemd:/system/netatalk.service ├ 1999 /usr/sbin/cnid_metad -l log_note └ 2002 /usr/sbin/afpd -P /var/run/netatalk.pid -F /etc/netatalk/afpd.conf systemd could tell which one of them is the main PID. When the main PID is not known, the only way to detect the death of the service is to watch for the cgroup getting empty. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] service: delayed main PID guessing
There is a lot of services with incorrectly implemented double-forking.
Their original process exits before the child writes the PID file.
With SysVinit it was rarely a problem, because the PID file was usually
read only when the initscript was called again.
systemd wants to read the PID file after receiving SIGCHLD for the
original process. With a buggy service it may be too early. systemd
warns about the failure to read the PID file and falls back to guessing
the main PID.
Guessing often fails in a bad way for such double-forking services.
systemd is likely to see the first forked child of the service and
assume it is the main PID. This child will fork the second child and
exit itself immediately. systemd will then kill the service.
I see a few options:
(1) Never guess, remove the code completely.
Forking services with incorrect PID file handling would simply not
have their main PID known and systemd would depend solely on the
notification about an empty cgroup as the mechanism to detect the
exit of such services.
(2) Make the guessing opt-in.
Make GuessMainPID=no the default for both native and SysV units.
We could add support for a custom SysV header in the chkconfig
section:
# x-systemd-GuessMainPID: yes
for the benefit of users who like the supervision provided by
systemd, but are unable to fix their services properly for whatever
reason.
(On a tangent... It may be a good idea to allow opting in/out of
main PID supervision in general, even when the PID is known
reliably.)
(3) Improve the guessing so that it is much more likely to give the
right PID.
This is what the patch implements. When no PIDFile= is declared or
when the PID file is missing after the SIGCHLD, give the service
some slack time to put things in order. Then retry the read of the
PID file and if it still fails, do the guessing.
I chose 1 second as the delay. It should be more than enough to
finish the daemonization in the service. It is also on the order
of magnitude that someone doing testing of SysV initscripts by
starting and stopping would have used for sleep in between, thus
masking the bug.
We could actually add (2) in addition to this later.
---
src/service.c | 38 +++---
1 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/src/service.c b/src/service.c
index c2053ce..d2b168b 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1318,7 +1318,8 @@ static int service_load_pid_file(Service *s, bool
warn_if_missing) {
(unsigned long) s-main_pid, (unsigned long) pid);
service_unwatch_main_pid(s);
s-main_pid_known = false;
-}
+} else
+log_debug(Main PID loaded: %lu, (unsigned long) pid);
if ((r = service_set_main_pid(s, pid)) 0)
return r;
@@ -1349,6 +1350,8 @@ static int service_search_main_pid(Service *s) {
if ((pid =
cgroup_bonding_search_main_pid_list(s-meta.cgroup_bondings)) = 0)
return -ENOENT;
+log_debug(Main PID guessed: %lu, (unsigned long) pid);
+
if ((r = service_set_main_pid(s, pid)) 0)
return r;
@@ -1451,6 +1454,7 @@ static void service_set_state(Service *s, ServiceState
state) {
if (state != SERVICE_START_PRE
state != SERVICE_START
state != SERVICE_START_POST
+state != SERVICE_RUNNING
state != SERVICE_RELOAD
state != SERVICE_STOP
state != SERVICE_STOP_SIGTERM
@@ -2003,7 +2007,7 @@ fail:
}
static void service_enter_running(Service *s, bool success) {
-int main_pid_ok, cgroup_ok;
+int main_pid_ok, cgroup_ok, r;
assert(s);
if (!success)
@@ -2012,13 +2016,24 @@ static void service_enter_running(Service *s, bool
success) {
main_pid_ok = main_pid_good(s);
cgroup_ok = cgroup_good(s);
+unit_unwatch_timer(UNIT(s), s-timer_watch);
+
if ((main_pid_ok 0 || (main_pid_ok 0 cgroup_ok != 0))
-(s-bus_name_good || s-type != SERVICE_DBUS))
+(s-bus_name_good || s-type != SERVICE_DBUS)) {
+if (!s-main_pid_known s-guess_main_pid) {
+log_debug(Setting up timer to guess main PID in 1
s.);
+if ((r = unit_watch_timer(UNIT(s), 1*USEC_PER_SEC,
s-timer_watch)) 0)
+goto fail;
+}
service_set_state(s, SERVICE_RUNNING);
-else if (s-remain_after_exit)
+} else if (s-remain_after_exit)
service_set_state(s, SERVICE_EXITED);
else
service_enter_stop(s, true);
+return;
+fail:
+log_warning(%s failed to install guess PID timer: %s, s-meta.id,
strerror(-r));
+service_enter_stop(s, false);
}
static void
[systemd-devel] [PATCH] unit: fix complementing of requirement deps with After deps for targets
'man systemd.target' says:
Unless DefaultDependencies= is set to false, target units will
implicitly complement all configured dependencies of type
Wants=, Requires=, RequiresOverridable= with dependencies of type
After= if the units in question also have DefaultDependencies=true.
It did not work because of a forgotten negation.
---
src/unit.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/unit.c b/src/unit.c
index 0b435cb..903a8e4 100644
--- a/src/unit.c
+++ b/src/unit.c
@@ -774,7 +774,7 @@ int unit_add_default_target_dependency(Unit *u, Unit
*target) {
/* If either side wants no automatic dependencies, then let's
* skip this */
if (!u-meta.default_dependencies ||
-target-meta.default_dependencies)
+!target-meta.default_dependencies)
return 0;
/* Don't create loops */
--
1.7.6.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] what does systemd do if the syslogd errs on start?
On 07/07/2011 08:28 AM, Rainer Gerhards wrote: Let's make sure we are on the same line. Sequece of events: 1. systemd starts syslogd 2. syslogd parses config, detects errors 3. syslogd logs config errors via syslog() Thus it sends them to /dev/log. No process is reading it, but syslog.socket is active, and the messages are stored in the socket buffer in the kernel. 4. syslogd terminates Then systemd notices the traffic on syslog.socket and spawns the associated service: systemd-kmsg-syslogd.service. The service receives the buffered messages. Will that work? Note that at #3, syslogd has not yet terminated, so from a systemd POV it will probably look healthy. I did not test exactly this case, but I expect it will work this way. Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] Preset Files
On Wed, 6 Jul 2011 00:42:25 +0200 Lennart Poettering wrote: Actually, I do want a way how people can reset all service enable states to what the vendor intended. And that should be systemctl preset without arguments I believe. That would be too easy to run accidentally. They can run: cd /lib/systemd/system systemctl preset *.service or require an extra parameter, e.g.: systemctl preset --all Michal ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Return error code 3 with systemctl status after killall LSB server
On Fri, 1 Jul 2011 23:17:57 +0200 Lennart Poettering wrote:
The problem here is that apache is a SysV init script, and for those
it is not really clear whether it is a problem that no process is
running anymore or if that's just the normal case.
This is true in general, but sometimes we do have enough information
to distinguish the two cases.
For instance, the presence of the chkconfig pidfile: header in the
initscript is an excellent indication that it's not a oneshot script
(like iptables), but a real daemon (like httpd).
This patch works for me.
Comments?
Michal
diff --git a/src/service.c b/src/service.c
index 165655e..5c7e62f 100644
--- a/src/service.c
+++ b/src/service.c
@@ -843,7 +843,7 @@ static int service_load_sysv_path(Service *s, const char
*path) {
/* Special setting for all SysV services */
s-type = SERVICE_FORKING;
-s-remain_after_exit = true;
+s-remain_after_exit = !s-pid_file;
s-restart = SERVICE_RESTART_NO;
s-exec_context.std_output =
(s-meta.manager-sysv_console || s-exec_context.std_input ==
EXEC_INPUT_TTY)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
