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
