On Donnerstag, 4. August 2016 07:56:44 CEST Sven Eckelmann wrote:
On Donnerstag, 4. August 2016 01:43:29 CEST Linus Lüssing wrote:
On Wed, Aug 03, 2016 at 11:25:53PM +0200, Linus Lüssing wrote:
On Wed, Aug 03, 2016 at 09:59:27PM +0200, Sven Eckelmann wrote:
Isn't this causing the reference counting cycle (aka really, really, really bad):
Hm, I see what you are getting at. Could indeed be a nasty bug...
But, actually, just tested in VMs, seems like I'm getting a call to batadv_hardif_neigh_release() just fine. Which means it counted to zero successfully o_O?
I don't understand why it seems to work right now :D. Will dig deeper.
Ok, I think it just works because hardif_neigh_create() intializes the nodes refcount to one (in contrast to other allocating functions which initialize to 2). In the end, once neigh_node_create() finishes, it stays at one.
So the regular purging routines will break the cycle in the end when they reduce the refcount of the hardif_neigh_node to zero.
Ok, looks like the neigh_list edge in my graph is incorrectly there.
[...]
Just checked the code and my example graph seems to be correct in regards of this edge. There is an reference for the list in the code:
static struct batadv_neigh_node * batadv_neigh_node_create(struct batadv_orig_node *orig_node, struct batadv_hard_iface *hard_iface, const u8 *neigh_addr) { [...] /* extra reference for return */ kref_init(&neigh_node->refcount);
kref_get(&neigh_node->refcount); hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list);
[...] return neigh_node; }
The function which doesn't get a reference for its list is batadv_hardif_neigh_create. It doesn't store a reference in batadv_hardif_neigh_create for the list because its lifetime doesn't depend on the list & just uses it as management helper and deletes the list entry in its _release function). But this list cannot be found in the example graph.
Kind regards, Sven