Re: [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: add reference counting for type batadv_tt_orig_list_entry

[Antonio Quartulli <ordex@autistici.org>]

[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]

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,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]