On Sun, Mar 17, 2013 at 05:44:57AM +0100, Linus Lüssing wrote:
rcu_barrier() only waits for the currently scheduled rcu functions to finish - it won't wait for any function scheduled via another call_rcu() within an rcu scheduled function.
Unfortunately our batadv_tt_orig_list_entry_free_ref() does just that, via a batadv_orig_node_free_ref() call, leading to our rcu_barrier() call potentially missing such a batadv_orig_node_free_ref().
This patch fixes this issue by calling the batadv_orig_node_free_rcu() directly from the rcu callback, removing the unnecessary, additional call_rcu() layer here.
Signed-off-by: Linus Lüssing linus.luessing@web.de
diff --git a/originator.c b/originator.c index 585e684..013c7d0 100644 --- a/originator.c +++ b/originator.c @@ -117,7 +117,7 @@ out: return neigh_node; }
-static void batadv_orig_node_free_rcu(struct rcu_head *rcu) +void batadv_orig_node_free_rcu(struct rcu_head *rcu) { struct hlist_node *node_tmp; struct batadv_neigh_node *neigh_node, *tmp_neigh_node; diff --git a/originator.h b/originator.h index 7df48fa..4f9f88b 100644 --- a/originator.h +++ b/originator.h @@ -25,6 +25,7 @@ int batadv_originator_init(struct batadv_priv *bat_priv); void batadv_originator_free(struct batadv_priv *bat_priv); void batadv_purge_orig_ref(struct batadv_priv *bat_priv); +void batadv_orig_node_free_rcu(struct rcu_head *rcu); void batadv_orig_node_free_ref(struct batadv_orig_node *orig_node); struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv, const uint8_t *addr); diff --git a/translation-table.c b/translation-table.c index 9322320..ee91cc1 100644 --- a/translation-table.c +++ b/translation-table.c @@ -144,7 +144,10 @@ static void batadv_tt_orig_list_entry_free_rcu(struct rcu_head *rcu) struct batadv_tt_orig_list_entry *orig_entry;
orig_entry = container_of(rcu, struct batadv_tt_orig_list_entry, rcu);
- batadv_orig_node_free_ref(orig_entry->orig_node);
- if (atomic_dec_and_test(&orig_entry->orig_node->refcount))
batadv_orig_node_free_rcu(&orig_entry->orig_node->rcu);- kfree(orig_entry);
}
Hi Linus,
I was just re-reading this patch: the code can be beautified later (I think it would be worth defining a new function, e.g. batadv_orig_node_free(orig_node), which can then be invoked both in batadv_orig_node_free_rcu() and here - in this way we avoid to export batadv_orig_node_free_rcu() which should remain private to the originator.c module and we avoid this forced fake rcu invocation), but I think that putting a comment here to explain why we don't invoke call_rcu is definitely needed. In the future somebody else may ask why we don't use it and will try to re-add it.
The rest looks good.
Thanks!