[v2,06/18] batman-adv: Place kref_get for orig_node near use
Commit Message
It is hard to understand why the refcnt is increased when it isn't done
near the actual place the new reference is used. So using kref_get right
before the place which requires the reference and in the same function
helps to avoid accidental problems causedy incorrect reference counting.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
- split patch based on type
net/batman-adv/bat_iv_ogm.c | 7 ++++---
net/batman-adv/bat_v_ogm.c | 5 ++---
net/batman-adv/gateway_client.c | 2 +-
net/batman-adv/network-coding.c | 7 +++----
net/batman-adv/originator.c | 1 -
5 files changed, 10 insertions(+), 12 deletions(-)
Comments
On Friday, July 15, 2016 17:39:21 Sven Eckelmann wrote:
> It is hard to understand why the refcnt is increased when it isn't done
> near the actual place the new reference is used. So using kref_get right
> before the place which requires the reference and in the same function
> helps to avoid accidental problems causedy incorrect reference counting.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v2:
> - split patch based on type
>
> net/batman-adv/bat_iv_ogm.c | 7 ++++---
> net/batman-adv/bat_v_ogm.c | 5 ++---
> net/batman-adv/gateway_client.c | 2 +-
> net/batman-adv/network-coding.c | 7 +++----
> net/batman-adv/originator.c | 1 -
> 5 files changed, 10 insertions(+), 12 deletions(-)
Applied in revision de6330c.
Thanks,
Marek
@@ -319,17 +319,18 @@ batadv_iv_ogm_orig_get(struct batadv_priv *bat_priv, const u8 *addr)
if (!orig_node->bat_iv.bcast_own_sum)
goto free_orig_node;
+ kref_get(&orig_node->refcount);
hash_added = batadv_hash_add(bat_priv->orig_hash, batadv_compare_orig,
batadv_choose_orig, orig_node,
&orig_node->hash_entry);
if (hash_added != 0)
- goto free_orig_node;
+ goto free_orig_node_hash;
return orig_node;
-free_orig_node:
- /* free twice, as batadv_orig_node_new sets refcount to 2 */
+free_orig_node_hash:
batadv_orig_node_put(orig_node);
+free_orig_node:
batadv_orig_node_put(orig_node);
return NULL;
@@ -73,13 +73,12 @@ struct batadv_orig_node *batadv_v_ogm_orig_get(struct batadv_priv *bat_priv,
if (!orig_node)
return NULL;
+ kref_get(&orig_node->refcount);
hash_added = batadv_hash_add(bat_priv->orig_hash, batadv_compare_orig,
batadv_choose_orig, orig_node,
&orig_node->hash_entry);
if (hash_added != 0) {
- /* orig_node->refcounter is initialised to 2 by
- * batadv_orig_node_new()
- */
+ /* remove refcnt for newly created orig_node and hash entry */
batadv_orig_node_put(orig_node);
batadv_orig_node_put(orig_node);
orig_node = NULL;
@@ -333,8 +333,8 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv,
if (!gw_node)
return;
- kref_get(&orig_node->refcount);
INIT_HLIST_NODE(&gw_node->list);
+ kref_get(&orig_node->refcount);
gw_node->orig_node = orig_node;
gw_node->bandwidth_down = ntohl(gateway->bandwidth_down);
gw_node->bandwidth_up = ntohl(gateway->bandwidth_up);
@@ -856,14 +856,13 @@ batadv_nc_get_nc_node(struct batadv_priv *bat_priv,
if (!nc_node)
return NULL;
- kref_get(&orig_neigh_node->refcount);
-
/* Initialize nc_node */
INIT_LIST_HEAD(&nc_node->list);
- ether_addr_copy(nc_node->addr, orig_node->orig);
- nc_node->orig_node = orig_neigh_node;
kref_init(&nc_node->refcount);
kref_get(&nc_node->refcount);
+ ether_addr_copy(nc_node->addr, orig_node->orig);
+ kref_get(&orig_neigh_node->refcount);
+ nc_node->orig_node = orig_neigh_node;
/* Select ingoing or outgoing coding node */
if (in_coding) {
@@ -906,7 +906,6 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv,
/* extra reference for return */
kref_init(&orig_node->refcount);
- kref_get(&orig_node->refcount);
orig_node->bat_priv = bat_priv;
ether_addr_copy(orig_node->orig, addr);