-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 12-02-09 03:54 PM, Mathieu Desnoyers wrote: > * David Goulet ([email protected]) wrote: >> Temporary fix the bug #7 issue for the 2.0 stable release. Refer to >> bugs.lttng.org for more information on the issues. >> >> The real stable fix is planned for 2.1+ stable release. >> >> Signed-off-by: David Goulet <[email protected]> >> --- >> src/bin/lttng-sessiond/ust-app.c | 36 +++++++++++++++++++++++++++++++++++- >> src/common/hashtable/hashtable.c | 2 -- >> src/common/hashtable/hashtable.h | 2 ++ >> 3 files changed, 37 insertions(+), 3 deletions(-) >> >> diff --git a/src/bin/lttng-sessiond/ust-app.c >> b/src/bin/lttng-sessiond/ust-app.c >> index 2b66b92..e82d38a 100644 >> --- a/src/bin/lttng-sessiond/ust-app.c >> +++ b/src/bin/lttng-sessiond/ust-app.c >> @@ -29,6 +29,9 @@ >> >> #include <common/common.h> >> >> +/* For add_unique_ust_app quick fix function for 2.0 stable release */ >> +#include <common/hashtable/hashtable.h> >> + >> #include "ust-app.h" >> #include "ust-consumer.h" >> #include "ust-ctl.h" >> @@ -1257,6 +1260,31 @@ error: >> } >> >> /* >> + * TEMP function call to add uniquely a registered apps to a hash table. >> + * >> + * If a duplicate node is found when adding apps, we delete the old one and >> + * close the socket file descriptor as well. Must be inside a read side >> lock. >> + * >> + * Please refer to BUG #7 on bugs.lttng.org to understand the purpose and >> + * lifespan of this function. >> + */ >> +static void add_unique_ust_app(struct lttng_ht *ht, >> + struct lttng_ht_node_ulong *node) >> +{ >> + struct cds_lfht_node *node_ptr; >> + struct lttng_ht_node_ulong *old_node; >> + >> + node_ptr = cds_lfht_add_unique(ht->ht, >> + ht->hash_fct((void *) node->key, HASH_SEED), >> ht->match_fct, >> + (void *) node->key, &node->node); >> + if (node_ptr != &node->node) { >> + /* Double registration with same PID */ >> + old_node = caa_container_of(node_ptr, struct >> lttng_ht_node_ulong, node); >> + call_rcu(&old_node->head, delete_ust_app_rcu); > > ust_app_unregister(): > > /* Get the node reference for a call_rcu */ > lttng_ht_lookup(ust_app_ht, (void *)((unsigned long) lta->key.pid), > &iter); > node = lttng_ht_iter_get_node_ulong(&iter); > if (node == NULL) { > ERR("Unable to find app sock %d by pid %d", sock, > lta->key.pid); > goto error; > } > > Should be allowed to not find the per pid entry without printing an > error. This will happen on the socket fd close that will follow this > removal. > >> + } > > Missing: lttng_ht_del() call before the call_rcu for delete_ust_app_rcu. > > Missing: retry loop! After we cleared room for the new node we want to > add, we need to retry the add_unique. Oh add_unique fails you are right! Should we put a replace instead thus removing the lttng_ht_del.. ? David > > Thanks, > > Mathieu > >> +} >> + >> +/* >> * Using pid and uid (of the app), allocate a new ust_app struct and >> * add it to the global traceable app list. >> * >> @@ -1307,7 +1335,13 @@ int ust_app_register(struct ust_register_msg *msg, >> int sock) >> >> rcu_read_lock(); >> lttng_ht_add_unique_ulong(ust_app_sock_key_map, <a->key.node); >> - lttng_ht_add_unique_ulong(ust_app_ht, <a->node); >> + /* >> + * Changed for now. Refer to BUG #7 called "Double PID registering and >> + * unregistering race" >> + * >> + * lttng_ht_add_unique_ulong(ust_app_ht, <a->node); >> + */ >> + add_unique_ust_app(ust_app_ht, <a->node); >> rcu_read_unlock(); >> >> DBG("App registered with pid:%d ppid:%d uid:%d gid:%d sock:%d name:%s" >> diff --git a/src/common/hashtable/hashtable.c >> b/src/common/hashtable/hashtable.c >> index 2080107..83d4dfd 100644 >> --- a/src/common/hashtable/hashtable.c >> +++ b/src/common/hashtable/hashtable.c >> @@ -27,8 +27,6 @@ >> #include "hashtable.h" >> #include "utils.h" >> >> -#define HASH_SEED 0x42UL /* The answer to life */ >> - >> static unsigned long min_hash_alloc_size = 1; >> static unsigned long max_hash_buckets_size = (1UL << 20); >> >> diff --git a/src/common/hashtable/hashtable.h >> b/src/common/hashtable/hashtable.h >> index 1c2889d..0f0339c 100644 >> --- a/src/common/hashtable/hashtable.h >> +++ b/src/common/hashtable/hashtable.h >> @@ -23,6 +23,8 @@ >> #include "rculfhash.h" >> #include "rculfhash-internal.h" >> >> +#define HASH_SEED 0x42UL /* The answer to life */ >> + >> typedef unsigned long (*hash_fct)(void *_key, unsigned long seed); >> typedef cds_lfht_match_fct hash_match_fct; >> >> -- >> 1.7.9 >> > -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) iQEcBAEBAgAGBQJPNDMmAAoJEELoaioR9I02UZgH+wQA74aT2l4a7xuRb7rpTcXz 15yCjY0m0foSenzo+bMKX4hAK37igvqQc60G6nKC7uDup80yXfXoGrpL7Olm18+I 3JqFGls5QugN83aBJJCWJl+WNb2/xRj7uBqoS9Md2iFSt693bpacwippLejtjFN2 XFvn/XVf0wqF14Xy7nas86NYtQylYIUXPSqQBcfumfdFxpuzsAwtNAm6TKHMexRq KnOi8GWcGKa0EpTfoFj1C/ukRLwZuNAnpfG9QEeBxOnJ0nhkusO/vXNawo+t9CJ9 kf2jnk66UWo14kYeDjYJYZCcDL0o6FhS2Uqo10VNqUkzV9rUmVkvm8MQ6pWGydI= =Z75H -----END PGP SIGNATURE----- _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
