Hello Lennart,
Lennart Poettering [2014-12-09 2:37 +0100]:
> hashmap_put() will actually compare the string, not the pointer to
> it. Our hashmap implementation gets a hash function pointer as well as
> an element comparison function as input, to ensure that that works
> correctly.
Ah right. I understand how these work now, and I extended the test
case to cover this case (equal key, but different pointer).
So the problem in unit_create_cgroups() was that hashmap_put()
returned 0 for the already existing "" key, but the function does not
check that case and re-initializes and re-migrates the cgroup (in
particular, -.slice) over and over again. The recent commit 7b3fd631
seems to mitigate this, pids don't move back to -.slice any more.
However, unit_create_cgroups() still seems to be quite wasteful: It
still gets called dozens of time for -.slice for every unit that gets
started, and thus calls cg_create_everywhere() for the already
existing cgroup. Why doesn't this function just check if
u->cgroup_path already exists, and if so just return right away? Are
there situations where the same unit cgroup needs to be realized and
pids migrated to it multiple times?
Thanks,
Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
>From 0503aa5256446df225859ce91ef7ac27c5be6b55 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.p...@ubuntu.com>
Date: Sat, 13 Dec 2014 04:22:28 +0100
Subject: [PATCH 1/4] test: hashmap_put behaviour for equal keys
Check string ops hashmap_put() for keys with a different pointer but the same
value.
---
src/test/test-hashmap-plain.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c
index 6f0910a..84b508f 100644
--- a/src/test/test-hashmap-plain.c
+++ b/src/test/test-hashmap-plain.c
@@ -245,6 +245,8 @@ static void test_hashmap_put(void) {
Hashmap *m = NULL;
int valid_hashmap_put;
void *val1 = (void*) "val 1";
+ void *val2 = (void*) "val 2";
+ _cleanup_free_ char* key1 = NULL;
assert_se(hashmap_ensure_allocated(&m, &string_hash_ops) >= 0);
assert_se(m);
@@ -252,7 +254,10 @@ static void test_hashmap_put(void) {
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(hashmap_put(m, "key 1", val2) == -EEXIST);
+ key1 = strdup("key 1");
+ assert_se(hashmap_put(m, key1, val1) == 0);
+ assert_se(hashmap_put(m, key1, val2) == -EEXIST);
hashmap_free(m);
}
--
2.1.3
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
