Re: [B.A.T.M.A.N.] slowpath warning

Marek Lindner
2010-01-26T07:16:38Z

[-- Attachment #1: Type: Text/Plain, Size: 484 bytes --]

On Tuesday 26 January 2010 14:13:11 Linus Lüssing wrote:
> I'm now able to reproduce the second problem as well:
> Activating the vis-server on one node causes the vis-clients to
> throw this call trace with its slow path warning imediately.
> Nevertheless, vis output works anyway, clients are sending
> vis-packets and the server is drawing nice graphs.
> See the attachment for an example call trace.

Please try the attached patch and see if it helps.

Regards,
Marek

[-- Attachment #2: no_lock_while_send.patch --]
[-- Type: text/x-patch, Size: 3998 bytes --]

diff --git a/batman-adv-kernelland/vis.c b/batman-adv-kernelland/vis.c
index b118d1e..a6c235f 100644
--- a/batman-adv-kernelland/vis.c
+++ b/batman-adv-kernelland/vis.c
@@ -405,6 +405,9 @@ static void purge_vis_packets(void)
{
HASHIT(hashit);
struct vis_info *info;
+ unsigned long flags;
+
+ spin_lock_irqsave(&vis_hash_lock, flags);

while (hash_iterate(vis_hash, &hashit)) {
info = hashit.bucket->data;
@@ -416,13 +419,17 @@ static void purge_vis_packets(void)
free_info(info);
}
}
+
+ spin_unlock_irqrestore(&vis_hash_lock, flags);
}

static void broadcast_vis_packet(struct vis_info *info, int packet_length)
{
HASHIT(hashit);
struct orig_node *orig_node;
+ struct batman_if *batman_if;
unsigned long flags;
+ uint8_t dstaddr[ETH_ALEN];

spin_lock_irqsave(&orig_hash_lock, flags);

@@ -431,45 +438,60 @@ 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;

- /* 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;
+ if (!(orig_node->flags & VIS_SERVER))
+ continue;

- memcpy(info->packet.target_orig,
- orig_node->orig, ETH_ALEN);
+ /* 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;

- send_raw_packet((unsigned char *) &info->packet,
- packet_length,
- orig_node->batman_if,
- orig_node->router->addr);
- }
+ spin_unlock_irqrestore(&orig_hash_lock, flags);
+
+ memcpy(info->packet.target_orig, orig_node->orig, ETH_ALEN);
+ batman_if = orig_node->batman_if;
+ memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
+
+ send_raw_packet((unsigned char *)&info->packet,
+ packet_length, batman_if, dstaddr);
+
+ spin_lock_irqsave(&orig_hash_lock, flags);
}
- 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;
+ struct batman_if *batman_if;
unsigned long flags;
+ 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,17 +524,18 @@ static void send_vis_packets(struct work_struct *work)
struct vis_info *info, *temp;
unsigned long flags;

- spin_lock_irqsave(&vis_hash_lock, flags);
purge_vis_packets();

if (generate_vis_packet() == 0)
/* schedule if generation was successful */
list_add_tail(&my_vis_info->send_list, &send_list);

+ spin_lock_irqsave(&vis_hash_lock, flags);
list_for_each_entry_safe(info, temp, &send_list, send_list) {
list_del_init(&info->send_list);
send_vis_packet(info);
}
+
spin_unlock_irqrestore(&vis_hash_lock, flags);
start_vis_timer();
}