[020/200] Staging: batman-adv: dont have interrupts disabled while sending.

Greg KH
2010-07-01T21:13:58Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 14246 bytes --]

2.6.34-stable review patch. If anyone has any objections, please let me know.

------------------

From: Andrew Lunn <andrew@lunn.ch>

commit 107c32fe68f0b64acb7edd31d44d79b87c7fa8b4 upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
drivers/staging/batman-adv/vis.c | 198 +++++++++++++++++++++++++++------------
drivers/staging/batman-adv/vis.h | 1
2 files changed, 139 insertions(+), 60 deletions(-)

--- a/drivers/staging/batman-adv/vis.c
+++ b/drivers/staging/batman-adv/vis.c
@@ -29,22 +29,26 @@

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
}
}

+/* 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(struc
}
/* 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(struc
/* 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(struc
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
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
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
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;
+
+ 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);

- /* 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);
- }
}
- 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
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);
--- a/drivers/staging/batman-adv/vis.h
+++ b/drivers/staging/batman-adv/vis.h
@@ -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*/