Staging: batman-adv: don't have interrupts disabled while sending.
authorAndrew Lunn <andrew@lunn.ch>
Mon, 22 Mar 2010 21:46:13 +0000 (22:46 +0100)
committerGreg Kroah-Hartman <gregkh@suse.de>
Tue, 11 May 2010 18:35:59 +0000 (11:35 -0700)
send_vis_packets() would disable interrupts before calling
dev_queue_xmit() which resulting in a backtrace in local_bh_enable().
Fix this by using kref on the vis_info object so that we can call
send_vis_packets() without holding vis_hash_lock. vis_hash_lock also
used to protect recv_list, so we now need a new lock to protect that
instead of vis_hash_lock.

Also a few checkpatch cleanups.

Reported-by: Linus Lüssing <linus.luessing@web.de>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/staging/batman-adv/vis.c
drivers/staging/batman-adv/vis.h

index fedec1b..0b5c63f 100644 (file)
 
 struct hashtable_t *vis_hash;
 DEFINE_SPINLOCK(vis_hash_lock);
+static DEFINE_SPINLOCK(recv_list_lock);
 static struct vis_info *my_vis_info;
 static struct list_head send_list;     /* always locked with vis_hash_lock */
 
 static void start_vis_timer(void);
 
 /* free the info */
-static void free_info(void *data)
+static void free_info(struct kref *ref)
 {
-       struct vis_info *info = data;
+       struct vis_info *info = container_of(ref, struct vis_info, refcount);
        struct recvlist_node *entry, *tmp;
+       unsigned long flags;
 
        list_del_init(&info->send_list);
+       spin_lock_irqsave(&recv_list_lock, flags);
        list_for_each_entry_safe(entry, tmp, &info->recv_list, list) {
                list_del(&entry->list);
                kfree(entry);
        }
+       spin_unlock_irqrestore(&recv_list_lock, flags);
        kfree(info);
 }
 
@@ -142,36 +146,65 @@ void proc_vis_read_entry(struct seq_file *seq,
        }
 }
 
+/* add the info packet to the send list, if it was not
+ * already linked in. */
+static void send_list_add(struct vis_info *info)
+{
+       if (list_empty(&info->send_list)) {
+               kref_get(&info->refcount);
+               list_add_tail(&info->send_list, &send_list);
+       }
+}
+
+/* delete the info packet from the send list, if it was
+ * linked in. */
+static void send_list_del(struct vis_info *info)
+{
+       if (!list_empty(&info->send_list)) {
+               list_del_init(&info->send_list);
+               kref_put(&info->refcount, free_info);
+       }
+}
+
 /* tries to add one entry to the receive list. */
 static void recv_list_add(struct list_head *recv_list, char *mac)
 {
        struct recvlist_node *entry;
+       unsigned long flags;
+
        entry = kmalloc(sizeof(struct recvlist_node), GFP_ATOMIC);
        if (!entry)
                return;
 
        memcpy(entry->mac, mac, ETH_ALEN);
+       spin_lock_irqsave(&recv_list_lock, flags);
        list_add_tail(&entry->list, recv_list);
+       spin_unlock_irqrestore(&recv_list_lock, flags);
 }
 
 /* returns 1 if this mac is in the recv_list */
 static int recv_list_is_in(struct list_head *recv_list, char *mac)
 {
        struct recvlist_node *entry;
+       unsigned long flags;
 
+       spin_lock_irqsave(&recv_list_lock, flags);
        list_for_each_entry(entry, recv_list, list) {
-               if (memcmp(entry->mac, mac, ETH_ALEN) == 0)
+               if (memcmp(entry->mac, mac, ETH_ALEN) == 0) {
+                       spin_unlock_irqrestore(&recv_list_lock, flags);
                        return 1;
+               }
        }
-
+       spin_unlock_irqrestore(&recv_list_lock, flags);
        return 0;
 }
 
 /* try to add the packet to the vis_hash. return NULL if invalid (e.g. too old,
- * broken.. ).  vis hash must be locked outside.  is_new is set when the packet
+ * broken.. ). vis hash must be locked outside.  is_new is set when the packet
  * is newer than old entries in the hash. */
 static struct vis_info *add_packet(struct vis_packet *vis_packet,
-                                  int vis_info_len, int *is_new)
+                                  int vis_info_len, int *is_new,
+                                  int make_broadcast)
 {
        struct vis_info *info, *old_info;
        struct vis_info search_elem;
@@ -198,13 +231,15 @@ static struct vis_info *add_packet(struct vis_packet *vis_packet,
                }
                /* remove old entry */
                hash_remove(vis_hash, old_info);
-               free_info(old_info);
+               send_list_del(old_info);
+               kref_put(&old_info->refcount, free_info);
        }
 
        info = kmalloc(sizeof(struct vis_info) + vis_info_len, GFP_ATOMIC);
        if (info == NULL)
                return NULL;
 
+       kref_init(&info->refcount);
        INIT_LIST_HEAD(&info->send_list);
        INIT_LIST_HEAD(&info->recv_list);
        info->first_seen = jiffies;
@@ -214,16 +249,21 @@ static struct vis_info *add_packet(struct vis_packet *vis_packet,
        /* initialize and add new packet. */
        *is_new = 1;
 
+       /* Make it a broadcast packet, if required */
+       if (make_broadcast)
+               memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
+
        /* repair if entries is longer than packet. */
        if (info->packet.entries * sizeof(struct vis_info_entry) > vis_info_len)
-               info->packet.entries = vis_info_len / sizeof(struct vis_info_entry);
+               info->packet.entries = vis_info_len /
+                       sizeof(struct vis_info_entry);
 
        recv_list_add(&info->recv_list, info->packet.sender_orig);
 
        /* try to add it */
        if (hash_add(vis_hash, info) < 0) {
                /* did not work (for some reason) */
-               free_info(info);
+               kref_put(&old_info->refcount, free_info);
                info = NULL;
        }
 
@@ -234,22 +274,21 @@ static struct vis_info *add_packet(struct vis_packet *vis_packet,
 void receive_server_sync_packet(struct vis_packet *vis_packet, int vis_info_len)
 {
        struct vis_info *info;
-       int is_new;
+       int is_new, make_broadcast;
        unsigned long flags;
        int vis_server = atomic_read(&vis_mode);
 
+       make_broadcast = (vis_server == VIS_TYPE_SERVER_SYNC);
+
        spin_lock_irqsave(&vis_hash_lock, flags);
-       info = add_packet(vis_packet, vis_info_len, &is_new);
+       info = add_packet(vis_packet, vis_info_len, &is_new, make_broadcast);
        if (info == NULL)
                goto end;
 
        /* only if we are server ourselves and packet is newer than the one in
         * hash.*/
-       if (vis_server == VIS_TYPE_SERVER_SYNC && is_new) {
-               memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
-               if (list_empty(&info->send_list))
-                       list_add_tail(&info->send_list, &send_list);
-       }
+       if (vis_server == VIS_TYPE_SERVER_SYNC && is_new)
+               send_list_add(info);
 end:
        spin_unlock_irqrestore(&vis_hash_lock, flags);
 }
@@ -262,31 +301,32 @@ void receive_client_update_packet(struct vis_packet *vis_packet,
        int is_new;
        unsigned long flags;
        int vis_server = atomic_read(&vis_mode);
+       int are_target = 0;
 
        /* clients shall not broadcast. */
        if (is_bcast(vis_packet->target_orig))
                return;
 
+       /* Are we the target for this VIS packet? */
+       if (vis_server == VIS_TYPE_SERVER_SYNC  &&
+           is_my_mac(vis_packet->target_orig))
+               are_target = 1;
+
        spin_lock_irqsave(&vis_hash_lock, flags);
-       info = add_packet(vis_packet, vis_info_len, &is_new);
+       info = add_packet(vis_packet, vis_info_len, &is_new, are_target);
        if (info == NULL)
                goto end;
        /* note that outdated packets will be dropped at this point. */
 
 
        /* send only if we're the target server or ... */
-       if (vis_server == VIS_TYPE_SERVER_SYNC  &&
-           is_my_mac(info->packet.target_orig) &&
-           is_new) {
+       if (are_target && is_new) {
                info->packet.vis_type = VIS_TYPE_SERVER_SYNC;   /* upgrade! */
-               memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
-               if (list_empty(&info->send_list))
-                       list_add_tail(&info->send_list, &send_list);
+               send_list_add(info);
 
                /* ... we're not the recipient (and thus need to forward). */
        } else if (!is_my_mac(info->packet.target_orig)) {
-               if (list_empty(&info->send_list))
-                       list_add_tail(&info->send_list, &send_list);
+               send_list_add(info);
        }
 end:
        spin_unlock_irqrestore(&vis_hash_lock, flags);
@@ -361,14 +401,17 @@ static int generate_vis_packet(void)
        while (hash_iterate(orig_hash, &hashit_global)) {
                orig_node = hashit_global.bucket->data;
                if (orig_node->router != NULL
-                       && compare_orig(orig_node->router->addr, orig_node->orig)
+                       && compare_orig(orig_node->router->addr,
+                                       orig_node->orig)
                        && orig_node->batman_if
                        && (orig_node->batman_if->if_active == IF_ACTIVE)
                    && orig_node->router->tq_avg > 0) {
 
                        /* fill one entry into buffer. */
                        entry = &entry_array[info->packet.entries];
-                       memcpy(entry->src, orig_node->batman_if->net_dev->dev_addr, ETH_ALEN);
+                       memcpy(entry->src,
+                              orig_node->batman_if->net_dev->dev_addr,
+                              ETH_ALEN);
                        memcpy(entry->dest, orig_node->orig, ETH_ALEN);
                        entry->quality = orig_node->router->tq_avg;
                        info->packet.entries++;
@@ -400,6 +443,8 @@ static int generate_vis_packet(void)
        return 0;
 }
 
+/* free old vis packets. Must be called with this vis_hash_lock
+ * held */
 static void purge_vis_packets(void)
 {
        HASHIT(hashit);
@@ -412,7 +457,8 @@ static void purge_vis_packets(void)
                if (time_after(jiffies,
                               info->first_seen + (VIS_TIMEOUT*HZ)/1000)) {
                        hash_remove_bucket(vis_hash, &hashit);
-                       free_info(info);
+                       send_list_del(info);
+                       kref_put(&info->refcount, free_info);
                }
        }
 }
@@ -422,6 +468,8 @@ static void broadcast_vis_packet(struct vis_info *info, int packet_length)
        HASHIT(hashit);
        struct orig_node *orig_node;
        unsigned long flags;
+       struct batman_if *batman_if;
+       uint8_t dstaddr[ETH_ALEN];
 
        spin_lock_irqsave(&orig_hash_lock, flags);
 
@@ -430,45 +478,56 @@ static void broadcast_vis_packet(struct vis_info *info, int packet_length)
                orig_node = hashit.bucket->data;
 
                /* if it's a vis server and reachable, send it. */
-               if (orig_node &&
-                   (orig_node->flags & VIS_SERVER) &&
-                   orig_node->batman_if &&
-                   orig_node->router) {
+               if ((!orig_node) || (!orig_node->batman_if) ||
+                   (!orig_node->router))
+                       continue;
+               if (!(orig_node->flags & VIS_SERVER))
+                       continue;
+               /* don't send it if we already received the packet from
+                * this node. */
+               if (recv_list_is_in(&info->recv_list, orig_node->orig))
+                       continue;
 
-                       /* don't send it if we already received the packet from
-                        * this node. */
-                       if (recv_list_is_in(&info->recv_list, orig_node->orig))
-                               continue;
+               memcpy(info->packet.target_orig, orig_node->orig, ETH_ALEN);
+               batman_if = orig_node->batman_if;
+               memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
+               spin_unlock_irqrestore(&orig_hash_lock, flags);
 
-                       memcpy(info->packet.target_orig,
-                              orig_node->orig, ETH_ALEN);
+               send_raw_packet((unsigned char *)&info->packet,
+                               packet_length, batman_if, dstaddr);
+
+               spin_lock_irqsave(&orig_hash_lock, flags);
 
-                       send_raw_packet((unsigned char *) &info->packet,
-                                       packet_length,
-                                       orig_node->batman_if,
-                                       orig_node->router->addr);
-               }
        }
-       memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
        spin_unlock_irqrestore(&orig_hash_lock, flags);
+       memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
 }
 
 static void unicast_vis_packet(struct vis_info *info, int packet_length)
 {
        struct orig_node *orig_node;
        unsigned long flags;
+       struct batman_if *batman_if;
+       uint8_t dstaddr[ETH_ALEN];
 
        spin_lock_irqsave(&orig_hash_lock, flags);
        orig_node = ((struct orig_node *)
                     hash_find(orig_hash, info->packet.target_orig));
 
-       if ((orig_node != NULL) &&
-           (orig_node->batman_if != NULL) &&
-           (orig_node->router != NULL)) {
-               send_raw_packet((unsigned char *) &info->packet, packet_length,
-                               orig_node->batman_if,
-                               orig_node->router->addr);
-       }
+       if ((!orig_node) || (!orig_node->batman_if) || (!orig_node->router))
+               goto out;
+
+       /* don't lock while sending the packets ... we therefore
+        * copy the required data before sending */
+       batman_if = orig_node->batman_if;
+       memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
+       spin_unlock_irqrestore(&orig_hash_lock, flags);
+
+       send_raw_packet((unsigned char *)&info->packet,
+                       packet_length, batman_if, dstaddr);
+       return;
+
+out:
        spin_unlock_irqrestore(&orig_hash_lock, flags);
 }
 
@@ -502,15 +561,24 @@ static void send_vis_packets(struct work_struct *work)
        unsigned long flags;
 
        spin_lock_irqsave(&vis_hash_lock, flags);
+
        purge_vis_packets();
 
-       if (generate_vis_packet() == 0)
+       if (generate_vis_packet() == 0) {
                /* schedule if generation was successful */
-               list_add_tail(&my_vis_info->send_list, &send_list);
+               send_list_add(my_vis_info);
+       }
 
        list_for_each_entry_safe(info, temp, &send_list, send_list) {
-               list_del_init(&info->send_list);
+
+               kref_get(&info->refcount);
+               spin_unlock_irqrestore(&vis_hash_lock, flags);
+
                send_vis_packet(info);
+
+               spin_lock_irqsave(&vis_hash_lock, flags);
+               send_list_del(info);
+               kref_put(&info->refcount, free_info);
        }
        spin_unlock_irqrestore(&vis_hash_lock, flags);
        start_vis_timer();
@@ -543,6 +611,7 @@ int vis_init(void)
        my_vis_info->first_seen = jiffies - atomic_read(&vis_interval);
        INIT_LIST_HEAD(&my_vis_info->recv_list);
        INIT_LIST_HEAD(&my_vis_info->send_list);
+       kref_init(&my_vis_info->refcount);
        my_vis_info->packet.version = COMPAT_VERSION;
        my_vis_info->packet.packet_type = BAT_VIS;
        my_vis_info->packet.ttl = TTL;
@@ -556,9 +625,9 @@ int vis_init(void)
 
        if (hash_add(vis_hash, my_vis_info) < 0) {
                printk(KERN_ERR
-                         "batman-adv:Can't add own vis packet into hash\n");
-               free_info(my_vis_info); /* not in hash, need to remove it
-                                        * manually. */
+                      "batman-adv:Can't add own vis packet into hash\n");
+               /* not in hash, need to remove it manually. */
+               kref_put(&my_vis_info->refcount, free_info);
                goto err;
        }
 
@@ -572,6 +641,15 @@ err:
        return 0;
 }
 
+/* Decrease the reference count on a hash item info */
+static void free_info_ref(void *data)
+{
+       struct vis_info *info = data;
+
+       send_list_del(info);
+       kref_put(&info->refcount, free_info);
+}
+
 /* shutdown vis-server */
 void vis_quit(void)
 {
@@ -583,7 +661,7 @@ void vis_quit(void)
 
        spin_lock_irqsave(&vis_hash_lock, flags);
        /* properly remove, kill timers ... */
-       hash_delete(vis_hash, free_info);
+       hash_delete(vis_hash, free_info_ref);
        vis_hash = NULL;
        my_vis_info = NULL;
        spin_unlock_irqrestore(&vis_hash_lock, flags);
index 0cdafde..465da47 100644 (file)
@@ -29,6 +29,7 @@ struct vis_info {
                            /* list of server-neighbors we received a vis-packet
                             * from.  we should not reply to them. */
        struct list_head send_list;
+       struct kref refcount;
        /* this packet might be part of the vis send queue. */
        struct vis_packet packet;
        /* vis_info may follow here*/