Re: IrDA woes..

[David Miller <davem@davemloft.net>]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu, 2 Jan 2014 13:07:05 -0800

> So I'd be inclined to take the queue lock (self->wx_list.lock) around
> that skb_queue_walk(). Or am I just dazed and confused?

Yes, something like the following untested patch.

I used irq blocking locking to be consistent with what the helpers
like skb_dequeue() use.  And you have to be careful to adjust code
to __skb_dequeue() that you put behind explicit taking of the lock.
skb_dequeue() is just a wrapper that take the lock around a call
to __skb_dequeue().

diff --git a/net/irda/irlap.c b/net/irda/irlap.c
index 005b424..1334625 100644
--- a/net/irda/irlap.c
+++ b/net/irda/irlap.c
@@ -698,19 +698,22 @@ int irlap_generate_rand_time_slot(int S, int s)
 void irlap_update_nr_received(struct irlap_cb *self, int nr)
 {
 	struct sk_buff *skb = NULL;
+	unsigned long flags;
 	int count = 0;
 
 	/*
 	 * Remove all the ack-ed frames from the window queue.
 	 */
 
+	spin_lock_irqsave(&self->wx_list.lock, flags);
+
 	/*
 	 *  Optimize for the common case. It is most likely that the receiver
 	 *  will acknowledge all the frames we have sent! So in that case we
 	 *  delete all frames stored in window.
 	 */
 	if (nr == self->vs) {
-		while ((skb = skb_dequeue(&self->wx_list)) != NULL) {
+		while ((skb = __skb_dequeue(&self->wx_list)) != NULL) {
 			dev_kfree_skb(skb);
 		}
 		/* The last acked frame is the next to send minus one */
@@ -720,7 +723,7 @@ void irlap_update_nr_received(struct irlap_cb *self, int nr)
 		while ((skb_peek(&self->wx_list) != NULL) &&
 		       (((self->va+1) % 8) != nr))
 		{
-			skb = skb_dequeue(&self->wx_list);
+			skb = __skb_dequeue(&self->wx_list);
 			dev_kfree_skb(skb);
 
 			self->va = (self->va + 1) % 8;
@@ -730,6 +733,8 @@ void irlap_update_nr_received(struct irlap_cb *self, int nr)
 
 	/* Advance window */
 	self->window = self->window_size - skb_queue_len(&self->wx_list);
+
+	spin_unlock_irqrestore(&self->wx_list.lock, flags);
 }
 
 /*
diff --git a/net/irda/irlap_frame.c b/net/irda/irlap_frame.c
index 9ea0c93..45fd93f 100644
--- a/net/irda/irlap_frame.c
+++ b/net/irda/irlap_frame.c
@@ -983,10 +983,13 @@ void irlap_resend_rejected_frames(struct irlap_cb *self, int command)
 {
 	struct sk_buff *tx_skb;
 	struct sk_buff *skb;
+	unsigned long flags;
 
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
 
+	spin_lock_irqsave(&self->wx_list.lock, flags);
+
 	/*  Resend unacknowledged frame(s) */
 	skb_queue_walk(&self->wx_list, skb) {
 		irlap_wait_min_turn_around(self, &self->qos_tx);
@@ -1039,16 +1042,20 @@ void irlap_resend_rejected_frames(struct irlap_cb *self, int command)
 		}
 	}
 #endif
+	spin_unlock_irqrestore(&self->wx_list.lock, flags);
 }
 
 void irlap_resend_rejected_frame(struct irlap_cb *self, int command)
 {
 	struct sk_buff *tx_skb;
+	unsigned long flags;
 	struct sk_buff *skb;
 
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
 
+	spin_lock_irqsave(&self->wx_list.lock, flags);
+
 	/*  Resend unacknowledged frame(s) */
 	skb = skb_peek(&self->wx_list);
 	if (skb != NULL) {
@@ -1072,6 +1079,8 @@ void irlap_resend_rejected_frame(struct irlap_cb *self, int command)
 
 		irlap_send_i_frame(self, tx_skb, command);
 	}
+
+	spin_unlock_irqrestore(&self->wx_list.lock, flags);
 }
 
 /*