Re: [B.A.T.M.A.N.] [PATCH] batman-adv: protect bonding with rcu locks

[Marek Lindner <lindner_marek@yahoo.de>]

On Thursday 30 December 2010 03:09:17 Simon Wunderlich wrote:
> bonding / alternating candidates need to be secured by rcu locks
> as well. This patch therefore converts the bonding list
> from a plain pointer list to a rcu securable lists and references
> the bonding candidates.

Thanks for your patch! As the bonding / alternating candidate list is the last 
item on the way to an orig_hash_lock free kernel module this patch is a very 
welcome one. However, first tests revealed some memory leaks due to misbehaving 
reference counters. Therefore I took the time to dive deeper into this code 
section and propose an addition to your patch. The changes include:

* The bonding / alternating candidate list is not destroyed & re-created with 
each incoming OGM but candidates are added & deleted in a dynamic fashion.
* The memory leaks have been fixed / a workaround has been found. The "free 
neighbors when an interface is deactivated" patch is also required to get the 
full benefit of these memory leak fixes. 
* I introduced some style changes which are meant to increase readability & 
maintainability. Feedback is welcome.  ;-)

Note: One of the memory leaks turned out to be part of a bigger problem which 
we may or may not have in other code sections as well. purge_orig_neighbors() 
called call_rcu() on the same struct more then once in a short time period, so 
that each consecutive call overwrote the previous one. We have to look into 
this.

While reorganizing the code I stumbled over these lines which are part of the 
interference check:

/* we only care if the other candidate is even considered as candidate. */
if (!list_empty(&tmp_neigh_node2->bonding_list))
         continue;

This seems wrong in this context. If the list is empty then this neighbor is 
not part of the candidate list. Wouldn't you agree ?

Regards,
Marek