On Sat, Jun 30, 2012 at 11:55:25AM +0200, Marek Lindner wrote:
On Wednesday, June 27, 2012 10:23:04 Antonio Quartulli wrote:
@@ -639,12 +641,17 @@ batadv_tt_global_entry_has_orig(const struct batadv_tt_global_entry *entry, rcu_read_lock(); head = &entry->orig_list; hlist_for_each_entry_rcu(tmp_orig_entry, node, head, list) {
if (tmp_orig_entry->orig_node == orig_node) {found = true;break;}
if (tmp_orig_entry->orig_node != orig_node)continue;if (!atomic_inc_not_zero(&tmp_orig_entry->refcount))continue;found = true;batadv_tt_orig_list_entry_free_ref(tmp_orig_entry);Instead of having this weird increment and immediate decrement this patch should add a general "orig_entry_get" / "orig_entry_find" function like we do everywhere else. Check the following functions to get an idea:
- batadv_primary_if_get_selected()
- batadv_orig_node_get_router()
- batadv_tt_hash_find()
Well, actually that is what I implemented in patch 2/4 (ok, I'll fix the function names to reflect what we already have). In this patch I simply tried to implement refcounting without changing too much code.
By the way, I will introduce a get function with this patch directly.
@@ -663,6 +670,7 @@ batadv_tt_global_add_orig_entry(struct batadv_tt_global_entry *tt_global_entry, atomic_inc(&orig_node->tt_size); orig_entry->orig_node = orig_node; orig_entry->ttvn = ttvn;
- atomic_set(&orig_entry->refcount, 0);
This looks extremely broken. We init with 0, never increase the counter and when we should free, we decrease first before checking if it is 0 ?
definitely broken. Will fix it!
Thank you,