reordered rcu (part II)
Hi,
here comes the next batch of changes to make the code "rcu safe". It won't be the last one but should make all follow-ups much easier.
Regards,
Marek
PATCH 1/2 batman-adv: separate ethernet comparing calls from hash functions
Note: The function compare_ether_addr() provided by the Linux kernel requires aligned memory.
--- batman-adv/hard-interface.c | 2 +- batman-adv/main.c | 2 +- batman-adv/main.h | 10 +++++++ batman-adv/routing.c | 55 +++++++++++++++++++-------------------- batman-adv/send.c | 2 +- batman-adv/soft-interface.c | 2 +- batman-adv/translation-table.c | 2 +- batman-adv/vis.c | 18 ++++++------ 8 files changed, 51 insertions(+), 42 deletions(-)
batman-adv/hard-interface.c +1 -1
@@ -187,7 +187,7 @@ static void check_known_mac_addr(struct net_device *net_dev)if (batman_if->net_dev == net_dev)continue ; -if (!compare_orig (batman_if->net_dev->dev_addr, +if (!compare_eth (batman_if->net_dev->dev_addr, net_dev->dev_addr))continue ;
batman-adv/main.c +1 -1
@@ -160,7 +160,7 @@ int is_my_mac(uint8_t *addr)if (batman_if->if_status !=IF_ACTIVE )continue ; -if (compare_orig (batman_if->net_dev->dev_addr, addr)) { +if (compare_eth (batman_if->net_dev->dev_addr, addr)) {rcu_read_unlock ();return 1 ; }
batman-adv/main.h +10 -0
@@ -167,4 +167,14 @@ static inline void bat_dbg(char type __always_unused, pr_err("%s: " fmt, _netdev->name, ## arg); \ } while (0) +/** +* returns 1 if they are the same ethernet addr +* +* note: can't use compare_ether_addr() as it requires aligned memory +*/ +static inline int compare_eth (void *data1,void *data2) +{ +return (memcmp (data1, data2,ETH_ALEN ) ==0 ?1 :0 ); +} +#endif /* _NET_BATMAN_ADV_MAIN_H_ */
batman-adv/routing.c +27 -28
132 lines not on this page · raw message
batman-adv/send.c +1 -1
@@ -326,7 +326,7 @@ void schedule_forward_packet(struct orig_node *orig_node,if ((orig_node->router) && (orig_node->router->tq_avg !=0 )) {/* rebroadcast ogm of best ranking neighbor as is */ -if (!compare_orig (orig_node->router->addr, ethhdr->h_source)) { +if (!compare_eth (orig_node->router->addr, ethhdr->h_source)) { batman_packet->tq = orig_node->router->tq_avg;if (orig_node->router->last_ttl)
batman-adv/soft-interface.c +1 -1
@@ -134,7 +134,7 @@ static struct softif_neigh *softif_neigh_get(struct bat_priv *bat_priv,rcu_read_lock ();hlist_for_each_entry_rcu (softif_neigh, node, &bat_priv->softif_neigh_list, list) { -if (memcmp (softif_neigh->addr, addr,ETH_ALEN ) !=0 ) +if (!compare_eth (softif_neigh->addr, addr))continue ;if (softif_neigh->vid != vid)
batman-adv/translation-table.c +1 -1
@@ -101,7 +101,7 @@ void hna_local_add(struct net_device *soft_iface, uint8_t *addr) hna_local_entry->last_seen = jiffies;/* the batman interface mac address should never be purged */ -if (compare_orig (addr, soft_iface->dev_addr)) +if (compare_eth (addr, soft_iface->dev_addr)) hna_local_entry->never_purge =1 ;else hna_local_entry->never_purge =0 ;
batman-adv/vis.c +9 -9
@@ -76,7 +76,7 @@ static int vis_info_cmp(void *data1, void *data2) d2 = data2; p1 = (struct vis_packet *)d1->skb_packet->data; p2 = (struct vis_packet *)d2->skb_packet->data; -return compare_orig (p1->vis_orig, p2->vis_orig); +return compare_eth (p1->vis_orig, p2->vis_orig); }/* hash function to choose an entry in a hash table of given size */
@@ -114,7 +114,7 @@ static void vis_data_insert_interface(const uint8_t *interface,struct hlist_node *pos;hlist_for_each_entry (entry, pos, if_list, list) { -if (compare_orig (entry->addr, (void *)interface)) +if (compare_eth (entry->addr, (void *)interface))return ; }
@@ -166,7 +166,7 @@ static ssize_t vis_data_read_entry(char *buff, struct vis_info_entry *entry,/* maximal length: max(4+17+2, 3+17+1+3+2) == 26 */ if (primary && entry->quality ==0 )return sprintf (buff,"HNA %pM, " , entry->dest); -else if (compare_orig (entry->src, src)) +else if (compare_eth (entry->src, src))return sprintf (buff,"TQ %pM %d, " , entry->dest, entry->quality);
@@ -213,7 +213,7 @@ int vis_seq_print_text(struct seq_file *seq, void *offset)if (entries[j].quality ==0 )continue ; compare = -compare_orig (entries[j].src, packet->vis_orig); +compare_eth (entries[j].src, packet->vis_orig);vis_data_insert_interface (entries[j].src, &vis_if_list, compare);
@@ -223,7 +223,7 @@ int vis_seq_print_text(struct seq_file *seq, void *offset) buf_size +=18 +26 * packet->entries;/* add primary/secondary records */ -if (compare_orig (entry->addr, packet->vis_orig)) +if (compare_eth (entry->addr, packet->vis_orig)) buf_size +=vis_data_count_prim_sec (&vis_if_list);
@@ -259,7 +259,7 @@ int vis_seq_print_text(struct seq_file *seq, void *offset)if (entries[j].quality ==0 )continue ; compare = -compare_orig (entries[j].src, packet->vis_orig); +compare_eth (entries[j].src, packet->vis_orig);vis_data_insert_interface (entries[j].src, &vis_if_list, compare);
@@ -277,7 +277,7 @@ int vis_seq_print_text(struct seq_file *seq, void *offset) entry->primary);/* add primary/secondary records */ -if (compare_orig (entry->addr, packet->vis_orig)) +if (compare_eth (entry->addr, packet->vis_orig)) buff_pos +=vis_data_read_prim_sec (buff + buff_pos, &vis_if_list);
@@ -345,7 +345,7 @@ static int recv_list_is_in(struct bat_priv *bat_priv,spin_lock_bh (&bat_priv->vis_list_lock);list_for_each_entry (entry, recv_list, list) { -if (memcmp (entry->mac, mac,ETH_ALEN ) ==0 ) { +if (compare_eth (entry->mac, mac)) {spin_unlock_bh (&bat_priv->vis_list_lock);return 1 ; }
@@ -615,7 +615,7 @@ static int generate_vis_packet(struct bat_priv *bat_priv)if (!neigh_node)continue ; -if (!compare_orig (neigh_node->addr, orig_node->orig)) +if (!compare_eth (neigh_node->addr, orig_node->orig))continue ;if (neigh_node->if_incoming->if_status !=IF_ACTIVE )
Signature · 1 line
1.7.2.3
PATCH 2/2 batman-adv: remove extra layer between hash and hash element - hash bucket
In reply to Marek Lindner--- batman-adv/compat.h | 8 ++ batman-adv/hash.c | 8 -- batman-adv/hash.h | 95 +++++-------------- batman-adv/icmp_socket.c | 5 +- batman-adv/originator.c | 71 ++++---------- batman-adv/originator.h | 34 +++++++- batman-adv/routing.c | 41 +++----- batman-adv/translation-table.c | 208 +++++++++++++++++++++++++-------------- batman-adv/types.h | 4 + batman-adv/unicast.c | 12 +-- batman-adv/vis.c | 102 +++++++++++--------- 11 files changed, 296 insertions(+), 292 deletions(-)
batman-adv/compat.h +8 -0
@@ -270,4 +270,12 @@ int bat_seq_printf(struct seq_file *m, const char *f, ...);#endif /* < KERNEL_VERSION(2, 6, 33) */ +#define hlist_first_rcu (head) (*((struct hlist_node **)(&(head)->first))) +#define hlist_next_rcu (node) (*((struct hlist_node **)(&(node)->next))) + +#define __hlist_for_each_rcu (pos, head) \ + for (pos = rcu_dereference(hlist_first_rcu(head)); \ + pos && ({ prefetch(pos->next); 1; }); \ + pos = rcu_dereference(hlist_next_rcu(pos))) +#endif /* _NET_BATMAN_ADV_COMPAT_H_ */
batman-adv/hash.c +0 -8
@@ -68,11 +68,3 @@ free_hash:kfree (hash);return NULL ; } - -void bucket_free_rcu (struct rcu_head *rcu) -{ -struct element_t *bucket; - - bucket = container_of(rcu, struct element_t, rcu); -kfree (bucket); -}
batman-adv/hash.h +24 -71
161 lines not on this page · raw message
batman-adv/icmp_socket.c +1 -4
@@ -219,14 +219,11 @@ static ssize_t bat_socket_write(struct file *file, const char __user *buff, goto dst_unreach;rcu_read_lock (); - orig_node = ((struct orig_node *)hash_find (bat_priv->orig_hash, - compare_orig, choose_orig, - icmp_packet->dst)); + orig_node =orig_hash_find (bat_priv->orig_hash, icmp_packet->dst);if (!orig_node) goto unlock; -kref_get (&orig_node->refcount); neigh_node = orig_node->router;if (!neigh_node)
batman-adv/originator.c +19 -52
165 lines not on this page · raw message
batman-adv/originator.h +33 -1
@@ -22,6 +22,8 @@#ifndef _NET_BATMAN_ADV_ORIGINATOR_H_#define _NET_BATMAN_ADV_ORIGINATOR_H_ +#include "hash.h" +int originator_init (struct bat_priv *bat_priv);void originator_free (struct bat_priv *bat_priv);void purge_orig_ref (struct bat_priv *bat_priv);
@@ -38,8 +40,10 @@ int orig_hash_del_if(struct batman_if *batman_if, int max_if_num);/* returns 1 if they are the same originator */ -static inline int compare_orig (void *data1,void *data2) +static inline int compare_orig (struct hlist_node *node,void *data2) { + void *data1 = container_of(node, struct orig_node, hash_entry); +return (memcmp (data1, data2,ETH_ALEN ) ==0 ?1 :0 ); }
@@ -64,4 +68,32 @@ static inline int choose_orig(void *data, int32_t size) return hash % size; } +static inline struct orig_node *orig_hash_find (struct hashtable_t *hash, +void *data) +{ +int index; +struct hlist_head *head; +struct hlist_node *node; +struct orig_node *orig_node, *orig_node_tmp =NULL ; + +if (!hash) +return NULL ; + + index =choose_orig (data, hash->size); + head = &hash->table[index]; + +rcu_read_lock (); +hlist_for_each_entry_rcu (orig_node, node, head, hash_entry) { +if (!compare_eth (orig_node, data)) +continue ; + + orig_node_tmp = orig_node; +kref_get (&orig_node_tmp->refcount); +break ; + } +rcu_read_unlock (); + +return orig_node_tmp; +} +#endif /* _NET_BATMAN_ADV_ORIGINATOR_H_ */
batman-adv/routing.c +14 -27
109 lines not on this page · raw message
batman-adv/translation-table.c +133 -75
403 lines not on this page · raw message
batman-adv/types.h +4 -0
@@ -85,6 +85,7 @@ struct orig_node {struct list_head frag_list;spinlock_t neigh_list_lock;/* protects neighbor list */ struct kref refcount; +struct hlist_node hash_entry;struct bat_priv *bat_priv;unsigned long last_frag_packet;spinlock_t ogm_cnt_lock;/* protects: bcast_own, bcast_own_sum,
@@ -193,11 +194,13 @@ struct hna_local_entry {uint8_t addr[ETH_ALEN ];unsigned long last_seen;char never_purge; +struct hlist_node hash_entry; };struct hna_global_entry {uint8_t addr[ETH_ALEN ];struct orig_node *orig_node; +struct hlist_node hash_entry; };/**
@@ -247,6 +250,7 @@ struct vis_info {* from. we should not reply to them. */ struct list_head send_list;struct kref refcount; +struct hlist_node hash_entry;struct bat_priv *bat_priv;/* this packet might be part of the vis send queue. */ struct sk_buff *skb_packet;
batman-adv/unicast.c +2 -10
@@ -184,15 +184,9 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv, *new_skb =NULL ; -rcu_read_lock (); - orig_node = ((struct orig_node *) -hash_find (bat_priv->orig_hash, compare_orig, choose_orig, - unicast_packet->orig)); + orig_node =orig_hash_find (bat_priv->orig_hash, unicast_packet->orig);if (!orig_node) - goto unlock; - -kref_get (&orig_node->refcount); -rcu_read_unlock (); + goto out; orig_node->last_frag_packet = jiffies;
@@ -219,8 +213,6 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv, goto out; -unlock: -rcu_read_unlock (); out:if (orig_node)kref_put (&orig_node->refcount, orig_node_free_ref);
batman-adv/vis.c +58 -44
248 lines not on this page · raw message
Signature · 1 line
1.7.2.3
Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: remove extra layer between hash and hash element - hash bucket
Quoted 7 lines
batman-adv/compat.h +0
-0@@ -270,4 +270,12 @@ int bat_seq_printf(struct seq_file *m, const char *f, ...);#endif /* < KERNEL_VERSION(2, 6, 33) */
batman-adv/compat.h int bat_seq_printf(struct seq_file *m, const char *f, ...); · line 281
Quoted 10 lines
batman-adv/compat.h +8
-0+#define hlist_first_rcu (head) (*((struct hlist_node **)(&(head)->first))) +#define hlist_next_rcu (node) (*((struct hlist_node **)(&(node)->next))) + +#define __hlist_for_each_rcu (pos, head) \ + for (pos = rcu_dereference(hlist_first_rcu(head)); \ + pos && ({ prefetch(pos->next); 1; }); \ + pos = rcu_dereference(hlist_next_rcu(pos))) +#endif /* _NET_BATMAN_ADV_COMPAT_H_ */
Isn't there a #define with < KERNEL_VERSION(2, 6, 37) missing? (Haven't checked much else about this patch yet)
Cheers, Linus
Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: remove extra layer between hash and hash element - hash bucket
Quoted 2 lines · On Thursday 10 February 2011 14:03:03 Linus Lüssing wrote:
Isn't there a #define with <
KERNEL_VERSION(2, 6, 37)missing? (Haven't checked much else about this patch yet)
Why would there be a need for .37 (this functionality came with .34) ? Nevertheless, you are right - it is missing here.
Regards,
Marek
PATCH v2 2/4 batman-adv: remove extra layer between hash and hash element - hash bucket
In reply to Marek Lindner---
Rebase on trunk, checking kernel version in compat.h and minor hash_find() API change.
batman-adv/compat.h | 12 +++ batman-adv/hash.c | 8 -- batman-adv/hash.h | 95 +++++-------------- batman-adv/icmp_socket.c | 5 +- batman-adv/originator.c | 71 ++++---------- batman-adv/originator.h | 35 +++++++- batman-adv/routing.c | 41 +++----- batman-adv/translation-table.c | 204 +++++++++++++++++++++++++--------------- batman-adv/types.h | 4 + batman-adv/unicast.c | 12 +-- batman-adv/vis.c | 103 ++++++++++++--------- 11 files changed, 298 insertions(+), 292 deletions(-)
batman-adv/compat.h +12 -0
@@ -270,6 +270,18 @@ int bat_seq_printf(struct seq_file *m, const char *f, ...);#endif /* < KERNEL_VERSION(2, 6, 33) */ +#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,34 ) + +#define hlist_first_rcu (head) (*((struct hlist_node **)(&(head)->first))) +#define hlist_next_rcu (node) (*((struct hlist_node **)(&(node)->next))) + +#define __hlist_for_each_rcu (pos, head) \ + for (pos = rcu_dereference(hlist_first_rcu(head)); \ + pos && ({ prefetch(pos->next); 1; }); \ + pos = rcu_dereference(hlist_next_rcu(pos))) + +#endif /* < KERNEL_VERSION(2, 6, 34) */ +#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,36 )#define __rcu
batman-adv/hash.c +0 -8
@@ -68,11 +68,3 @@ free_hash:kfree (hash);return NULL ; } - -void bucket_free_rcu (struct rcu_head *rcu) -{ -struct element_t *bucket; - - bucket = container_of(rcu, struct element_t, rcu); -kfree (bucket); -}
batman-adv/hash.h +24 -71
161 lines not on this page · raw message
batman-adv/icmp_socket.c +1 -4
@@ -219,14 +219,11 @@ static ssize_t bat_socket_write(struct file *file, const char __user *buff, goto dst_unreach;rcu_read_lock (); - orig_node = ((struct orig_node *)hash_find (bat_priv->orig_hash, - compare_orig, choose_orig, - icmp_packet->dst)); + orig_node =orig_hash_find (bat_priv, icmp_packet->dst);if (!orig_node) goto unlock; -kref_get (&orig_node->refcount); neigh_node = orig_node->router;if (!neigh_node)
batman-adv/originator.c +19 -52
165 lines not on this page · raw message
batman-adv/originator.h +34 -1
@@ -22,6 +22,8 @@#ifndef _NET_BATMAN_ADV_ORIGINATOR_H_#define _NET_BATMAN_ADV_ORIGINATOR_H_ +#include "hash.h" +int originator_init (struct bat_priv *bat_priv);void originator_free (struct bat_priv *bat_priv);void purge_orig_ref (struct bat_priv *bat_priv);
@@ -38,8 +40,10 @@ int orig_hash_del_if(struct batman_if *batman_if, int max_if_num);/* returns 1 if they are the same originator */ -static inline int compare_orig (void *data1,void *data2) +static inline int compare_orig (struct hlist_node *node,void *data2) { + void *data1 = container_of(node, struct orig_node, hash_entry); +return (memcmp (data1, data2,ETH_ALEN ) ==0 ?1 :0 ); }
@@ -64,4 +68,33 @@ static inline int choose_orig(void *data, int32_t size)return hash % size; } +static inline struct orig_node *orig_hash_find (struct bat_priv *bat_priv, +void *data) +{ +struct hashtable_t *hash = bat_priv->orig_hash; +struct hlist_head *head; +struct hlist_node *node; +struct orig_node *orig_node, *orig_node_tmp =NULL ; +int index; + +if (!hash) +return NULL ; + + index =choose_orig (data, hash->size); + head = &hash->table[index]; + +rcu_read_lock (); +hlist_for_each_entry_rcu (orig_node, node, head, hash_entry) { +if (!compare_eth (orig_node, data)) +continue ; + + orig_node_tmp = orig_node; +kref_get (&orig_node_tmp->refcount); +break ; + } +rcu_read_unlock (); + +return orig_node_tmp; +} +#endif /* _NET_BATMAN_ADV_ORIGINATOR_H_ */
batman-adv/routing.c +14 -27
109 lines not on this page · raw message
batman-adv/translation-table.c +129 -75
399 lines not on this page · raw message
batman-adv/types.h +4 -0
@@ -85,6 +85,7 @@ struct orig_node {struct list_head frag_list;spinlock_t neigh_list_lock;/* protects neighbor list */ struct kref refcount; +struct hlist_node hash_entry;struct bat_priv *bat_priv;unsigned long last_frag_packet;spinlock_t ogm_cnt_lock;/* protects: bcast_own, bcast_own_sum,
@@ -193,11 +194,13 @@ struct hna_local_entry {uint8_t addr[ETH_ALEN ];unsigned long last_seen;char never_purge; +struct hlist_node hash_entry; };struct hna_global_entry {uint8_t addr[ETH_ALEN ];struct orig_node *orig_node; +struct hlist_node hash_entry; };/**
@@ -247,6 +250,7 @@ struct vis_info {* from. we should not reply to them. */ struct list_head send_list;struct kref refcount; +struct hlist_node hash_entry;struct bat_priv *bat_priv;/* this packet might be part of the vis send queue. */ struct sk_buff *skb_packet;
batman-adv/unicast.c +2 -10
@@ -184,15 +184,9 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv, *new_skb =NULL ; -rcu_read_lock (); - orig_node = ((struct orig_node *) -hash_find (bat_priv->orig_hash, compare_orig, choose_orig, - unicast_packet->orig)); + orig_node =orig_hash_find (bat_priv, unicast_packet->orig);if (!orig_node) - goto unlock; - -kref_get (&orig_node->refcount); -rcu_read_unlock (); + goto out; orig_node->last_frag_packet = jiffies;
@@ -219,8 +213,6 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv, goto out; -unlock: -rcu_read_unlock (); out:if (orig_node)kref_put (&orig_node->refcount, orig_node_free_ref);
batman-adv/vis.c +59 -44
249 lines not on this page · raw message
Signature · 1 line
1.7.2.3
PATCH 3/4 batman-adv: Correct rcu refcounting for orig_node
In reply to Marek LindnerIt might be possible that 2 threads access the same data in the same rcu grace period. The first thread calls call_rcu() to decrement the refcount and free the data while the second thread increases the refcount to use the data. To avoid this race condition all refcount operations have to be atomic.
--- batman-adv/gateway_client.c | 6 ++++-- batman-adv/icmp_socket.c | 2 +- batman-adv/originator.c | 18 ++++++++++++------ batman-adv/originator.h | 6 ++++-- batman-adv/routing.c | 22 +++++++++++----------- batman-adv/translation-table.c | 15 +++++++++------ batman-adv/types.h | 3 ++- batman-adv/unicast.c | 6 ++---- batman-adv/vis.c | 2 +- 9 files changed, 46 insertions(+), 34 deletions(-)
batman-adv/gateway_client.c +4 -2
@@ -53,9 +53,11 @@ void *gw_get_selected(struct bat_priv *bat_priv) goto out; orig_node = curr_gateway_tmp->orig_node; +if (!orig_node) + goto out; -if (orig_node) -kref_get (&orig_node->refcount); +if (!atomic_inc_not_zero (&orig_node->refcount)) + orig_node =NULL ; out:rcu_read_unlock ();
batman-adv/icmp_socket.c +1 -1
@@ -263,7 +263,7 @@ out:if (neigh_node)neigh_node_free_ref (neigh_node);if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return len; }
batman-adv/originator.c +12 -6
@@ -99,13 +99,13 @@ struct neigh_node *create_neighbor(struct orig_node *orig_node,return neigh_node; } -void orig_node_free_ref (struct kref *refcount) +static void orig_node_free_rcu (struct rcu_head *rcu) {struct hlist_node *node, *node_tmp;struct neigh_node *neigh_node, *tmp_neigh_node;struct orig_node *orig_node; - orig_node = container_of(refcount, struct orig_node, refcount); + orig_node = container_of(rcu, struct orig_node, rcu);spin_lock_bh (&orig_node->neigh_list_lock);
@@ -134,6 +134,12 @@ void orig_node_free_ref(struct kref *refcount)kfree (orig_node); } +void orig_node_free_ref (struct orig_node *orig_node) +{ +if (atomic_dec_and_test (&orig_node->refcount)) +call_rcu (&orig_node->rcu, orig_node_free_rcu); +} +void originator_free (struct bat_priv *bat_priv) {struct hashtable_t *hash = bat_priv->orig_hash;
@@ -159,7 +165,7 @@ void originator_free(struct bat_priv *bat_priv) head, hash_entry) {hlist_del_rcu (node); -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node); }spin_unlock_bh (list_lock); }
@@ -191,7 +197,9 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)spin_lock_init (&orig_node->ogm_cnt_lock);spin_lock_init (&orig_node->bcast_seqno_lock);spin_lock_init (&orig_node->neigh_list_lock); -kref_init (&orig_node->refcount); + +/* extra reference for return */ +atomic_set (&orig_node->refcount,2 ); orig_node->bat_priv = bat_priv;memcpy (orig_node->orig, addr,ETH_ALEN );
@@ -224,8 +232,6 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)if (hash_added <0 ) goto free_bcast_own_sum; -/* extra reference for return */ -kref_get (&orig_node->refcount);return orig_node; free_bcast_own_sum:kfree (orig_node->bcast_own_sum);
batman-adv/originator.h +4 -2
@@ -27,7 +27,7 @@int originator_init (struct bat_priv *bat_priv);void originator_free (struct bat_priv *bat_priv);void purge_orig_ref (struct bat_priv *bat_priv); -void orig_node_free_ref (struct kref *refcount); +void orig_node_free_ref (struct orig_node *orig_node);struct orig_node *get_orig_node (struct bat_priv *bat_priv,uint8_t *addr); struct neigh_node *create_neighbor(struct orig_node *orig_node, struct orig_node *orig_neigh_node,
@@ -88,8 +88,10 @@ static inline struct orig_node *orig_hash_find(struct bat_priv *bat_priv,if (!compare_eth (orig_node, data))continue ; +if (!atomic_inc_not_zero (&orig_node->refcount)) +continue ; + orig_node_tmp = orig_node; -kref_get (&orig_node_tmp->refcount);break ; }rcu_read_unlock ();
batman-adv/routing.c +11 -11
@@ -416,7 +416,7 @@ static void update_orig(struct bat_priv *bat_priv, neigh_node =create_neighbor (orig_node, orig_tmp, ethhdr->h_source, if_incoming); -kref_put (&orig_tmp->refcount, orig_node_free_ref); +orig_node_free_ref (orig_tmp);if (!neigh_node) goto unlock;
@@ -600,7 +600,7 @@ static char count_real_packets(struct ethhdr *ethhdr, out:spin_unlock_bh (&orig_node->ogm_cnt_lock); -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return ret; }
@@ -726,7 +726,7 @@ void receive_bat_packet(struct ethhdr *ethhdr,bat_dbg (DBG_BATMAN , bat_priv,"Drop packet: " "originator packet from myself (via neighbor)\n" ); -kref_put (&orig_neigh_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_neigh_node);return ; }
@@ -832,9 +832,9 @@ void receive_bat_packet(struct ethhdr *ethhdr, out_neigh:if ((orig_neigh_node) && (!is_single_hop_neigh)) -kref_put (&orig_neigh_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_neigh_node); out: -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node); }int recv_bat_packet (struct sk_buff *skb,struct batman_if *batman_if)
@@ -935,7 +935,7 @@ out:if (neigh_node)neigh_node_free_ref (neigh_node);if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return ret; }
@@ -1001,7 +1001,7 @@ out:if (neigh_node)neigh_node_free_ref (neigh_node);if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return ret; }
@@ -1097,7 +1097,7 @@ out:if (neigh_node)neigh_node_free_ref (neigh_node);if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return ret; }
@@ -1152,7 +1152,7 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,if (!primary_orig_node) goto return_router; -kref_put (&primary_orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (primary_orig_node); }/* with less than 2 candidates, we can't do any
@@ -1354,7 +1354,7 @@ out:if (neigh_node)neigh_node_free_ref (neigh_node);if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return ret; }
@@ -1492,7 +1492,7 @@ spin_unlock:spin_unlock_bh (&orig_node->bcast_seqno_lock); out:if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return ret; }
batman-adv/translation-table.c +9 -6
@@ -589,17 +589,20 @@ void hna_global_free(struct bat_priv *bat_priv)struct orig_node *transtable_search (struct bat_priv *bat_priv,uint8_t *addr) {struct hna_global_entry *hna_global_entry; +struct orig_node *orig_node =NULL ;spin_lock_bh (&bat_priv->hna_ghash_lock); hna_global_entry =hna_global_hash_find (bat_priv, addr); -if (hna_global_entry) -kref_get (&hna_global_entry->orig_node->refcount); +if (!hna_global_entry) + goto out; -spin_unlock_bh (&bat_priv->hna_ghash_lock); +if (!atomic_inc_not_zero (&hna_global_entry->orig_node->refcount)) + goto out; -if (!hna_global_entry) -return NULL ; + orig_node = hna_global_entry->orig_node; -return hna_global_entry->orig_node; +out: +spin_unlock_bh (&bat_priv->hna_ghash_lock); +return orig_node; }
batman-adv/types.h +2 -1
@@ -84,7 +84,8 @@ struct orig_node {struct hlist_head neigh_list;struct list_head frag_list;spinlock_t neigh_list_lock;/* protects neighbor list */ -struct kref refcount; +atomic_t refcount; +struct rcu_head rcu;struct hlist_node hash_entry;struct bat_priv *bat_priv;unsigned long last_frag_packet;
batman-adv/unicast.c +2 -4
@@ -211,11 +211,9 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv,if (*new_skb) ret =NET_RX_SUCCESS ; - goto out; - out:if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return ret; }
@@ -344,7 +342,7 @@ out:if (neigh_node)neigh_node_free_ref (neigh_node);if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);if (ret ==1 )kfree_skb (skb);return ret;
batman-adv/vis.c +1 -1
@@ -809,7 +809,7 @@ out:if (neigh_node)neigh_node_free_ref (neigh_node);if (orig_node) -kref_put (&orig_node->refcount, orig_node_free_ref); +orig_node_free_ref (orig_node);return ; }
Signature · 1 line
1.7.2.3
PATCH 4/4 batman-adv: increase refcount in create_neighbor to be consistent
In reply to Marek Lindner--- batman-adv/originator.c | 4 ++- batman-adv/routing.c | 63 ++++++++++++++++++++-------------------------- 2 files changed, 30 insertions(+), 37 deletions(-)
batman-adv/originator.c +3 -1
@@ -91,7 +91,9 @@ struct neigh_node *create_neighbor(struct orig_node *orig_node,memcpy (neigh_node->addr, neigh,ETH_ALEN ); neigh_node->orig_node = orig_neigh_node; neigh_node->if_incoming = if_incoming; -atomic_set (&neigh_node->refcount,1 ); + +/* extra reference for return */ +atomic_set (&neigh_node->refcount,2 );spin_lock_bh (&orig_node->neigh_list_lock);hlist_add_head_rcu (&neigh_node->list, &orig_node->neigh_list);
batman-adv/routing.c +27 -36
109 lines not on this page · raw message
Signature · 1 line
1.7.2.3
Marek Lindner
AppliedQuoted 2 lines · On Wednesday 09 February 2011 19:18:41 Marek Lindner wrote:
here comes the next batch of changes to make the code "rcu safe". It won't be the last one but should make all follow-ups much easier.
Applied in revision 1947-1950.
Regards,
Marek