Changeset 1579

Timestamp:

03/04/10 11:28:10 (5 months ago)

Author:

marek

Message:

batman-adv: don't have interrupts disabled while sending.

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 Luessing <linus.luessing@…>
Signed-off-by: Andrew Lunn <andrew@…>
Signed-off-by: Marek Lindner <lindner_marek@…>
Signed-off-by: Simon Wunderlich <siwu@…>

Location:

trunk/batman-adv-kernelland

Files:

• vis.c (modified) (28 diffs)
• vis.h (modified) (1 diff)

trunk/batman-adv-kernelland/vis.c

@@ -31 +31 @@
 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 */
@@ -37 +38 @@
 
 /* free the info */
-static void free_info(void *data)
-{
-        struct vis_info *info = data;
+static void free_info(struct kref *ref)
+{
+        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);
 }
@@ -144 +148 @@
 }
 
+/* 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)
@@ -153 +179 @@
 
         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);
 }
 
@@ -160 +188 @@
 {
         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;
@@ -200 +233 @@
                 /* 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);
         }
 
@@ -207 +241 @@
                 return NULL;
 
+        kref_init(&info->refcount);
         INIT_LIST_HEAD(&info->send_list);
         INIT_LIST_HEAD(&info->recv_list);
@@ -216 +251 @@
         *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);
@@ -225 +265 @@
         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;
         }
@@ -236 +276 @@
 {
         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;
@@ -247 +289 @@
         /* 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);
@@ -264 +303 @@
         unsigned long flags;
         int vis_server = atomic_read(&vis_mode);
+        int are_target = 0;
 
         /* clients shall not broadcast. */
@@ -269 +309 @@
                 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;
@@ -277 +322 @@
 
         /* 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:
@@ -363 +403 @@
                 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)
@@ -370 +411 @@
                         /* 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;
@@ -402 +445 @@
 }
 
+/* free old vis packets. Must be called with this vis_hash_lock
+ * held */
 static void purge_vis_packets(void)
 {
@@ -414 +459 @@
                                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);
                 }
         }
@@ -424 +470 @@
         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);
@@ -432 +480 @@
 
                 /* 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) {
-
-                        /* 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);
-
-                        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))
+                        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;
+
+                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);
+
+                send_raw_packet((unsigned char *)&info->packet,
+                                packet_length, batman_if, dstaddr);
+
+                spin_lock_irqsave(&orig_hash_lock, flags);
+
+        }
+        spin_unlock_irqrestore(&orig_hash_lock, flags);
         memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
-        spin_unlock_irqrestore(&orig_hash_lock, flags);
 }
 
@@ -459 +509 @@
         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);
@@ -464 +516 @@
                      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);
 }
@@ -504 +563 @@
 
         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);
@@ -545 +613 @@
         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;
@@ -558 +627 @@
         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;
         }
@@ -574 +643 @@
 }
 
+/* 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)
@@ -585 +663 @@
         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;
@@ -595 +673 @@
 {
         queue_delayed_work(bat_event_workqueue, &vis_timer_wq,
-                           (atomic_read(&vis_interval) * HZ ) / 1000);
-}
+                           (atomic_read(&vis_interval) * HZ) / 1000);
+}

trunk/batman-adv-kernelland/vis.h

@@ -30 +30 @@
                              * 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;