[PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[Larry Finger]

Beginning with commit a1091aae19b1d9c85d91c86915a611387f67a26b entitled
"p54: Use SKB list handling helpers instead of by-hand code.", the following
appears in my logs whenever I try to use my p54usb wireless device:

BUG: sleeping function called from invalid context at arch/x86/mm/fault.c:1085
 in_atomic(): 0, irqs_disabled(): 1, pid: 8538, name: modprobe
 2 locks held by modprobe/8538:
  #0:  (&local->queue_stop_reason_lock){-.-...}, at: [<ffffffffa023b99f>]
        ieee80211_stop_queues_by_reason+0x28/0x5e [mac80211]
  #1:  (&mm->mmap_sem){++++++}, at: [<ffffffff80225a4f>] do_page_fault+0xd2/0x1fe
 irq event stamp: 25212
 hardirqs last  enabled at (25211): [<ffffffff80451f28>] _spin_unlock_irqrestore+0x3f/0x47
 hardirqs last disabled at (25212): [<ffffffff804520e3>] _spin_lock_irqsave+0x17/0x4b
 softirqs last  enabled at (24064): [<ffffffff803e9997>] sk_filter+0xba/0xc3
 softirqs last disabled at (24062): [<ffffffff803e98f3>] sk_filter+0x16/0xc3
 Pid: 8538, comm: modprobe Not tainted 2.6.30-rc6-Linus-00905-g46c3767-dirty #180
 Call Trace:
  [<ffffffff8025af90>] ? print_irqtrace_events+0xd0/0xd4
  [<ffffffff80233384>] __might_sleep+0xf4/0xf6
  [<ffffffff80225a99>] do_page_fault+0x11c/0x1fe
  [<ffffffff80451f28>] ? _spin_unlock_irqrestore+0x3f/0x47
  [<ffffffff804525cf>] page_fault+0x1f/0x30
  [<ffffffff80451f28>] ? _spin_unlock_irqrestore+0x3f/0x47
  [<ffffffffa023b99f>] ? ieee80211_stop_queues_by_reason+0x28/0x5e [mac80211]
  [<ffffffffa023b92b>] ? __ieee80211_stop_queue+0x36/0x82 [mac80211]
  [<ffffffff8045210b>] ? _spin_lock_irqsave+0x3f/0x4b
  [<ffffffffa023b9b3>] ieee80211_stop_queues_by_reason+0x3c/0x5e [mac80211]
  [<ffffffffa023b9e0>] ieee80211_stop_queues+0xb/0xd [mac80211]
  [<ffffffffa047c35c>] p54_assign_address+0x164/0x1ec [p54common]
  [<ffffffffa047c49a>] p54_alloc_skb+0xb6/0xd3 [p54common]
...

Reverting the hunk that affects p54_assign_address() fixes the problem. When I
tried to determine which change(s) caused the problem, the skb_peek_tail()
seemed to be the problem; however, the system would freeze. I was not able to
recover any log information.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

diff --git a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
index b618bd1..c5bc55a 100644
--- a/drivers/net/wireless/p54/p54common.c
+++ b/drivers/net/wireless/p54/p54common.c
@@ -1164,21 +1164,23 @@ static int p54_assign_address(struct ieee80211_hw *dev, struct sk_buff *skb,
 		}
 	}
 
-	skb_queue_walk(&priv->tx_queue, entry) {
+	entry = priv->tx_queue.next;
+	while (left--) {
 		u32 hole_size;
 		info = IEEE80211_SKB_CB(entry);
 		range = (void *)info->rate_driver_data;
 		hole_size = range->start_addr - last_addr;
 		if (!target_skb && hole_size >= len) {
-			target_skb = skb_queue_prev(&priv->tx_queue, entry);
+			target_skb = entry->prev;
 			hole_size -= len;
 			target_addr = last_addr;
 		}
 		largest_hole = max(largest_hole, hole_size);
 		last_addr = range->end_addr;
+		entry = entry->next;
 	}
 	if (!target_skb && priv->rx_end - last_addr >= len) {
-		target_skb = skb_peek_tail(&priv->tx_queue);
+		target_skb = priv->tx_queue.prev;
 		largest_hole = max(largest_hole, priv->rx_end - last_addr - len);
 		if (!skb_queue_empty(&priv->tx_queue)) {
 			info = IEEE80211_SKB_CB(target_skb);

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[David Miller]

From: Larry Finger <Larry.Finger@lwfinger.net>
Date: Mon, 06 Jul 2009 14:07:01 -0500

> Beginning with commit a1091aae19b1d9c85d91c86915a611387f67a26b entitled
> "p54: Use SKB list handling helpers instead of by-hand code.", the following
> appears in my logs whenever I try to use my p54usb wireless device:
> 
> BUG: sleeping function called from invalid context at arch/x86/mm/fault.c:1085

Thanks for tracking this down Larry.  I'll have a closer look.

If I can't figure it out quickly I'll just simply revert.

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[Christian Lamparter]

On Monday 06 July 2009 21:14:29 David Miller wrote:
> From: Larry Finger <Larry.Finger@lwfinger.net>
> Date: Mon, 06 Jul 2009 14:07:01 -0500
> 
> > Beginning with commit a1091aae19b1d9c85d91c86915a611387f67a26b entitled
> > "p54: Use SKB list handling helpers instead of by-hand code.", the following
> > appears in my logs whenever I try to use my p54usb wireless device:
> > 
> > BUG: sleeping function called from invalid context at arch/x86/mm/fault.c:1085
> 
> Thanks for tracking this down Larry.  I'll have a closer look.
> 
> If I can't figure it out quickly I'll just simply revert.
hmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm
[2.6.31] p54: band-aid to restore driver functionality
http://patchwork.ozlabs.org/patch/28918/

in case someone missed it.

Regards,
	Chr

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[David Miller]

From: Christian Lamparter <chunkeey@web.de>
Date: Mon, 6 Jul 2009 21:19:25 +0200

> On Monday 06 July 2009 21:14:29 David Miller wrote:
>> If I can't figure it out quickly I'll just simply revert.
> hmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm
> [2.6.31] p54: band-aid to restore driver functionality
> http://patchwork.ozlabs.org/patch/28918/
> 
> in case someone missed it.

It is so much cleaner to not do multiple things in one patch.

It's also better to completely revert the SKB list handling
changes first, then add whatever other fixes you want make
on top.

Partial revert really makes no sense here and is only asking
for even more potential regressions.

So, like I said, I'm reverting this wholesale unless I can figure
out a fix quickly.

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[Larry Finger]

David Miller wrote:
> From: Christian Lamparter <chunkeey@web.de>
> Date: Mon, 6 Jul 2009 21:19:25 +0200
> 
>> On Monday 06 July 2009 21:14:29 David Miller wrote:
>>> If I can't figure it out quickly I'll just simply revert.
>> hmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm
>> [2.6.31] p54: band-aid to restore driver functionality
>> http://patchwork.ozlabs.org/patch/28918/
>>
>> in case someone missed it.
> 
> It is so much cleaner to not do multiple things in one patch.
> 
> It's also better to completely revert the SKB list handling
> changes first, then add whatever other fixes you want make
> on top.
> 
> Partial revert really makes no sense here and is only asking
> for even more potential regressions.
> 
> So, like I said, I'm reverting this wholesale unless I can figure
> out a fix quickly.

I'm available for testing.

Larry

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[David Miller]

From: Larry Finger <Larry.Finger@lwfinger.net>
Date: Mon, 06 Jul 2009 14:43:45 -0500

> I'm available for testing.

I just committed the following to my net-2.6 tree, give
it a spin:

>From 0ca1b08eba627b4245efd0f71b55a062bf163777 Mon Sep 17 00:00:00 2001
From: David S. Miller <davem@davemloft.net>
Date: Mon, 6 Jul 2009 12:49:18 -0700
Subject: [PATCH] Revert "p54: Use SKB list handling helpers instead of by-hand code."

This reverts commit a1091aae19b1d9c85d91c86915a611387f67a26b.
---
 drivers/net/wireless/p54/p54common.c |   46 ++++++++++++++++++++--------------
 1 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
index b618bd1..48d81d9 100644
--- a/drivers/net/wireless/p54/p54common.c
+++ b/drivers/net/wireless/p54/p54common.c
@@ -823,30 +823,30 @@ void p54_free_skb(struct ieee80211_hw *dev, struct sk_buff *skb)
 	struct p54_tx_info *range;
 	unsigned long flags;
 
-	if (unlikely(!skb || !dev || skb_queue_empty(&priv->tx_queue)))
+	if (unlikely(!skb || !dev || !skb_queue_len(&priv->tx_queue)))
 		return;
 
-	/* There used to be a check here to see if the SKB was on the
-	 * TX queue or not.  This can never happen because all SKBs we
-	 * see here successfully went through p54_assign_address()
-	 * which means the SKB is on the ->tx_queue.
+	/*
+	 * don't try to free an already unlinked skb
 	 */
+	if (unlikely((!skb->next) || (!skb->prev)))
+		return;
 
 	spin_lock_irqsave(&priv->tx_queue.lock, flags);
 	info = IEEE80211_SKB_CB(skb);
 	range = (void *)info->rate_driver_data;
-	if (!skb_queue_is_first(&priv->tx_queue, skb)) {
+	if (skb->prev != (struct sk_buff *)&priv->tx_queue) {
 		struct ieee80211_tx_info *ni;
 		struct p54_tx_info *mr;
 
-		ni = IEEE80211_SKB_CB(skb_queue_prev(&priv->tx_queue, skb));
+		ni = IEEE80211_SKB_CB(skb->prev);
 		mr = (struct p54_tx_info *)ni->rate_driver_data;
 	}
-	if (!skb_queue_is_last(&priv->tx_queue, skb)) {
+	if (skb->next != (struct sk_buff *)&priv->tx_queue) {
 		struct ieee80211_tx_info *ni;
 		struct p54_tx_info *mr;
 
-		ni = IEEE80211_SKB_CB(skb_queue_next(&priv->tx_queue, skb));
+		ni = IEEE80211_SKB_CB(skb->next);
 		mr = (struct p54_tx_info *)ni->rate_driver_data;
 	}
 	__skb_unlink(skb, &priv->tx_queue);
@@ -864,13 +864,15 @@ static struct sk_buff *p54_find_tx_entry(struct ieee80211_hw *dev,
 	unsigned long flags;
 
 	spin_lock_irqsave(&priv->tx_queue.lock, flags);
-	skb_queue_walk(&priv->tx_queue, entry) {
+	entry = priv->tx_queue.next;
+	while (entry != (struct sk_buff *)&priv->tx_queue) {
 		struct p54_hdr *hdr = (struct p54_hdr *) entry->data;
 
 		if (hdr->req_id == req_id) {
 			spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
 			return entry;
 		}
+		entry = entry->next;
 	}
 	spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
 	return NULL;
@@ -888,22 +890,24 @@ static void p54_rx_frame_sent(struct ieee80211_hw *dev, struct sk_buff *skb)
 	int count, idx;
 
 	spin_lock_irqsave(&priv->tx_queue.lock, flags);
-	skb_queue_walk(&priv->tx_queue, entry) {
+	entry = (struct sk_buff *) priv->tx_queue.next;
+	while (entry != (struct sk_buff *)&priv->tx_queue) {
 		struct ieee80211_tx_info *info = IEEE80211_SKB_CB(entry);
 		struct p54_hdr *entry_hdr;
 		struct p54_tx_data *entry_data;
 		unsigned int pad = 0, frame_len;
 
 		range = (void *)info->rate_driver_data;
-		if (range->start_addr != addr)
+		if (range->start_addr != addr) {
+			entry = entry->next;
 			continue;
+		}
 
-		if (!skb_queue_is_last(&priv->tx_queue, entry)) {
+		if (entry->next != (struct sk_buff *)&priv->tx_queue) {
 			struct ieee80211_tx_info *ni;
 			struct p54_tx_info *mr;
 
-			ni = IEEE80211_SKB_CB(skb_queue_next(&priv->tx_queue,
-							     entry));
+			ni = IEEE80211_SKB_CB(entry->next);
 			mr = (struct p54_tx_info *)ni->rate_driver_data;
 		}
 
@@ -1164,21 +1168,23 @@ static int p54_assign_address(struct ieee80211_hw *dev, struct sk_buff *skb,
 		}
 	}
 
-	skb_queue_walk(&priv->tx_queue, entry) {
+	entry = priv->tx_queue.next;
+	while (left--) {
 		u32 hole_size;
 		info = IEEE80211_SKB_CB(entry);
 		range = (void *)info->rate_driver_data;
 		hole_size = range->start_addr - last_addr;
 		if (!target_skb && hole_size >= len) {
-			target_skb = skb_queue_prev(&priv->tx_queue, entry);
+			target_skb = entry->prev;
 			hole_size -= len;
 			target_addr = last_addr;
 		}
 		largest_hole = max(largest_hole, hole_size);
 		last_addr = range->end_addr;
+		entry = entry->next;
 	}
 	if (!target_skb && priv->rx_end - last_addr >= len) {
-		target_skb = skb_peek_tail(&priv->tx_queue);
+		target_skb = priv->tx_queue.prev;
 		largest_hole = max(largest_hole, priv->rx_end - last_addr - len);
 		if (!skb_queue_empty(&priv->tx_queue)) {
 			info = IEEE80211_SKB_CB(target_skb);
@@ -2084,6 +2090,7 @@ out:
 static void p54_stop(struct ieee80211_hw *dev)
 {
 	struct p54_common *priv = dev->priv;
+	struct sk_buff *skb;
 
 	mutex_lock(&priv->conf_mutex);
 	priv->mode = NL80211_IFTYPE_UNSPECIFIED;
@@ -2098,7 +2105,8 @@ static void p54_stop(struct ieee80211_hw *dev)
 		p54_tx_cancel(dev, priv->cached_beacon);
 
 	priv->stop(dev);
-	skb_queue_purge(&priv->tx_queue);
+	while ((skb = skb_dequeue(&priv->tx_queue)))
+		kfree_skb(skb);
 	priv->cached_beacon = NULL;
 	priv->tsf_high32 = priv->tsf_low32 = 0;
 	mutex_unlock(&priv->conf_mutex);
-- 
1.6.3.3

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[Bjarke Istrup Pedersen]

2009/7/6 David Miller <davem@davemloft.net>:
> From: Larry Finger <Larry.Finger@lwfinger.net>
> Date: Mon, 06 Jul 2009 14:43:45 -0500
>
>> I'm available for testing.
>
> I just committed the following to my net-2.6 tree, give
> it a spin:
>
> From 0ca1b08eba627b4245efd0f71b55a062bf163777 Mon Sep 17 00:00:00 2001
> From: David S. Miller <davem@davemloft.net>
> Date: Mon, 6 Jul 2009 12:49:18 -0700
> Subject: [PATCH] Revert "p54: Use SKB list handling helpers instead of by-hand code."
>
> This reverts commit a1091aae19b1d9c85d91c86915a611387f67a26b.
> ---
>  drivers/net/wireless/p54/p54common.c |   46 ++++++++++++++++++++--------------
>  1 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
> index b618bd1..48d81d9 100644
> --- a/drivers/net/wireless/p54/p54common.c
> +++ b/drivers/net/wireless/p54/p54common.c
> @@ -823,30 +823,30 @@ void p54_free_skb(struct ieee80211_hw *dev, struct sk_buff *skb)
>        struct p54_tx_info *range;
>        unsigned long flags;
>
> -       if (unlikely(!skb || !dev || skb_queue_empty(&priv->tx_queue)))
> +       if (unlikely(!skb || !dev || !skb_queue_len(&priv->tx_queue)))
>                return;
>
> -       /* There used to be a check here to see if the SKB was on the
> -        * TX queue or not.  This can never happen because all SKBs we
> -        * see here successfully went through p54_assign_address()
> -        * which means the SKB is on the ->tx_queue.
> +       /*
> +        * don't try to free an already unlinked skb
>         */
> +       if (unlikely((!skb->next) || (!skb->prev)))
> +               return;
>
>        spin_lock_irqsave(&priv->tx_queue.lock, flags);
>        info = IEEE80211_SKB_CB(skb);
>        range = (void *)info->rate_driver_data;
> -       if (!skb_queue_is_first(&priv->tx_queue, skb)) {
> +       if (skb->prev != (struct sk_buff *)&priv->tx_queue) {
>                struct ieee80211_tx_info *ni;
>                struct p54_tx_info *mr;
>
> -               ni = IEEE80211_SKB_CB(skb_queue_prev(&priv->tx_queue, skb));
> +               ni = IEEE80211_SKB_CB(skb->prev);
>                mr = (struct p54_tx_info *)ni->rate_driver_data;
>        }
> -       if (!skb_queue_is_last(&priv->tx_queue, skb)) {
> +       if (skb->next != (struct sk_buff *)&priv->tx_queue) {
>                struct ieee80211_tx_info *ni;
>                struct p54_tx_info *mr;
>
> -               ni = IEEE80211_SKB_CB(skb_queue_next(&priv->tx_queue, skb));
> +               ni = IEEE80211_SKB_CB(skb->next);
>                mr = (struct p54_tx_info *)ni->rate_driver_data;
>        }
>        __skb_unlink(skb, &priv->tx_queue);
> @@ -864,13 +864,15 @@ static struct sk_buff *p54_find_tx_entry(struct ieee80211_hw *dev,
>        unsigned long flags;
>
>        spin_lock_irqsave(&priv->tx_queue.lock, flags);
> -       skb_queue_walk(&priv->tx_queue, entry) {
> +       entry = priv->tx_queue.next;
> +       while (entry != (struct sk_buff *)&priv->tx_queue) {
>                struct p54_hdr *hdr = (struct p54_hdr *) entry->data;
>
>                if (hdr->req_id == req_id) {
>                        spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
>                        return entry;
>                }
> +               entry = entry->next;
>        }
>        spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
>        return NULL;
> @@ -888,22 +890,24 @@ static void p54_rx_frame_sent(struct ieee80211_hw *dev, struct sk_buff *skb)
>        int count, idx;
>
>        spin_lock_irqsave(&priv->tx_queue.lock, flags);
> -       skb_queue_walk(&priv->tx_queue, entry) {
> +       entry = (struct sk_buff *) priv->tx_queue.next;
> +       while (entry != (struct sk_buff *)&priv->tx_queue) {
>                struct ieee80211_tx_info *info = IEEE80211_SKB_CB(entry);
>                struct p54_hdr *entry_hdr;
>                struct p54_tx_data *entry_data;
>                unsigned int pad = 0, frame_len;
>
>                range = (void *)info->rate_driver_data;
> -               if (range->start_addr != addr)
> +               if (range->start_addr != addr) {
> +                       entry = entry->next;
>                        continue;
> +               }
>
> -               if (!skb_queue_is_last(&priv->tx_queue, entry)) {
> +               if (entry->next != (struct sk_buff *)&priv->tx_queue) {
>                        struct ieee80211_tx_info *ni;
>                        struct p54_tx_info *mr;
>
> -                       ni = IEEE80211_SKB_CB(skb_queue_next(&priv->tx_queue,
> -                                                            entry));
> +                       ni = IEEE80211_SKB_CB(entry->next);
>                        mr = (struct p54_tx_info *)ni->rate_driver_data;
>                }
>
> @@ -1164,21 +1168,23 @@ static int p54_assign_address(struct ieee80211_hw *dev, struct sk_buff *skb,
>                }
>        }
>
> -       skb_queue_walk(&priv->tx_queue, entry) {
> +       entry = priv->tx_queue.next;
> +       while (left--) {
>                u32 hole_size;
>                info = IEEE80211_SKB_CB(entry);
>                range = (void *)info->rate_driver_data;
>                hole_size = range->start_addr - last_addr;
>                if (!target_skb && hole_size >= len) {
> -                       target_skb = skb_queue_prev(&priv->tx_queue, entry);
> +                       target_skb = entry->prev;
>                        hole_size -= len;
>                        target_addr = last_addr;
>                }
>                largest_hole = max(largest_hole, hole_size);
>                last_addr = range->end_addr;
> +               entry = entry->next;
>        }
>        if (!target_skb && priv->rx_end - last_addr >= len) {
> -               target_skb = skb_peek_tail(&priv->tx_queue);
> +               target_skb = priv->tx_queue.prev;
>                largest_hole = max(largest_hole, priv->rx_end - last_addr - len);
>                if (!skb_queue_empty(&priv->tx_queue)) {
>                        info = IEEE80211_SKB_CB(target_skb);
> @@ -2084,6 +2090,7 @@ out:
>  static void p54_stop(struct ieee80211_hw *dev)
>  {
>        struct p54_common *priv = dev->priv;
> +       struct sk_buff *skb;
>
>        mutex_lock(&priv->conf_mutex);
>        priv->mode = NL80211_IFTYPE_UNSPECIFIED;
> @@ -2098,7 +2105,8 @@ static void p54_stop(struct ieee80211_hw *dev)
>                p54_tx_cancel(dev, priv->cached_beacon);
>
>        priv->stop(dev);
> -       skb_queue_purge(&priv->tx_queue);
> +       while ((skb = skb_dequeue(&priv->tx_queue)))
> +               kfree_skb(skb);
>        priv->cached_beacon = NULL;
>        priv->tsf_high32 = priv->tsf_low32 = 0;
>        mutex_unlock(&priv->conf_mutex);
> --
> 1.6.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

I can confirm that the patch fixes the problem with my PCMCIA card :-)

Best regards,
Bjarke Istrup Pedersen

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[David Miller]

From: Bjarke Istrup Pedersen <gurligebis@gmail.com>
Date: Mon, 6 Jul 2009 22:26:09 +0200

> I can confirm that the patch fixes the problem with my PCMCIA card :-)

Thanks for testing :)

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[Larry Finger]

David Miller wrote:
> From: Larry Finger <Larry.Finger@lwfinger.net>
> Date: Mon, 06 Jul 2009 14:43:45 -0500
> 
>> I'm available for testing.
> 
> I just committed the following to my net-2.6 tree, give
> it a spin:
> 
>>From 0ca1b08eba627b4245efd0f71b55a062bf163777 Mon Sep 17 00:00:00 2001
> From: David S. Miller <davem@davemloft.net>
> Date: Mon, 6 Jul 2009 12:49:18 -0700
> Subject: [PATCH] Revert "p54: Use SKB list handling helpers instead of by-hand code."
> 
> This reverts commit a1091aae19b1d9c85d91c86915a611387f67a26b.
> ---
>  drivers/net/wireless/p54/p54common.c |   46 ++++++++++++++++++++--------------
>  1 files changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
> index b618bd1..48d81d9 100644
> --- a/drivers/net/wireless/p54/p54common.c
> +++ b/drivers/net/wireless/p54/p54common.c
> @@ -823,30 +823,30 @@ void p54_free_skb(struct ieee80211_hw *dev, struct sk_buff *skb)
>  	struct p54_tx_info *range;
>  	unsigned long flags;
>  
> -	if (unlikely(!skb || !dev || skb_queue_empty(&priv->tx_queue)))
> +	if (unlikely(!skb || !dev || !skb_queue_len(&priv->tx_queue)))
>  		return;
>  
> -	/* There used to be a check here to see if the SKB was on the
> -	 * TX queue or not.  This can never happen because all SKBs we
> -	 * see here successfully went through p54_assign_address()
> -	 * which means the SKB is on the ->tx_queue.
> +	/*
> +	 * don't try to free an already unlinked skb
>  	 */
> +	if (unlikely((!skb->next) || (!skb->prev)))
> +		return;
>  
>  	spin_lock_irqsave(&priv->tx_queue.lock, flags);
>  	info = IEEE80211_SKB_CB(skb);
>  	range = (void *)info->rate_driver_data;
> -	if (!skb_queue_is_first(&priv->tx_queue, skb)) {
> +	if (skb->prev != (struct sk_buff *)&priv->tx_queue) {
>  		struct ieee80211_tx_info *ni;
>  		struct p54_tx_info *mr;
>  
> -		ni = IEEE80211_SKB_CB(skb_queue_prev(&priv->tx_queue, skb));
> +		ni = IEEE80211_SKB_CB(skb->prev);
>  		mr = (struct p54_tx_info *)ni->rate_driver_data;
>  	}
> -	if (!skb_queue_is_last(&priv->tx_queue, skb)) {
> +	if (skb->next != (struct sk_buff *)&priv->tx_queue) {
>  		struct ieee80211_tx_info *ni;
>  		struct p54_tx_info *mr;
>  
> -		ni = IEEE80211_SKB_CB(skb_queue_next(&priv->tx_queue, skb));
> +		ni = IEEE80211_SKB_CB(skb->next);
>  		mr = (struct p54_tx_info *)ni->rate_driver_data;
>  	}
>  	__skb_unlink(skb, &priv->tx_queue);
> @@ -864,13 +864,15 @@ static struct sk_buff *p54_find_tx_entry(struct ieee80211_hw *dev,
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&priv->tx_queue.lock, flags);
> -	skb_queue_walk(&priv->tx_queue, entry) {
> +	entry = priv->tx_queue.next;
> +	while (entry != (struct sk_buff *)&priv->tx_queue) {
>  		struct p54_hdr *hdr = (struct p54_hdr *) entry->data;
>  
>  		if (hdr->req_id == req_id) {
>  			spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
>  			return entry;
>  		}
> +		entry = entry->next;
>  	}
>  	spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
>  	return NULL;
> @@ -888,22 +890,24 @@ static void p54_rx_frame_sent(struct ieee80211_hw *dev, struct sk_buff *skb)
>  	int count, idx;
>  
>  	spin_lock_irqsave(&priv->tx_queue.lock, flags);
> -	skb_queue_walk(&priv->tx_queue, entry) {
> +	entry = (struct sk_buff *) priv->tx_queue.next;
> +	while (entry != (struct sk_buff *)&priv->tx_queue) {
>  		struct ieee80211_tx_info *info = IEEE80211_SKB_CB(entry);
>  		struct p54_hdr *entry_hdr;
>  		struct p54_tx_data *entry_data;
>  		unsigned int pad = 0, frame_len;
>  
>  		range = (void *)info->rate_driver_data;
> -		if (range->start_addr != addr)
> +		if (range->start_addr != addr) {
> +			entry = entry->next;
>  			continue;
> +		}
>  
> -		if (!skb_queue_is_last(&priv->tx_queue, entry)) {
> +		if (entry->next != (struct sk_buff *)&priv->tx_queue) {
>  			struct ieee80211_tx_info *ni;
>  			struct p54_tx_info *mr;
>  
> -			ni = IEEE80211_SKB_CB(skb_queue_next(&priv->tx_queue,
> -							     entry));
> +			ni = IEEE80211_SKB_CB(entry->next);
>  			mr = (struct p54_tx_info *)ni->rate_driver_data;
>  		}
>  
> @@ -1164,21 +1168,23 @@ static int p54_assign_address(struct ieee80211_hw *dev, struct sk_buff *skb,
>  		}
>  	}
>  
> -	skb_queue_walk(&priv->tx_queue, entry) {
> +	entry = priv->tx_queue.next;
> +	while (left--) {
>  		u32 hole_size;
>  		info = IEEE80211_SKB_CB(entry);
>  		range = (void *)info->rate_driver_data;
>  		hole_size = range->start_addr - last_addr;
>  		if (!target_skb && hole_size >= len) {
> -			target_skb = skb_queue_prev(&priv->tx_queue, entry);
> +			target_skb = entry->prev;
>  			hole_size -= len;
>  			target_addr = last_addr;
>  		}
>  		largest_hole = max(largest_hole, hole_size);
>  		last_addr = range->end_addr;
> +		entry = entry->next;
>  	}
>  	if (!target_skb && priv->rx_end - last_addr >= len) {
> -		target_skb = skb_peek_tail(&priv->tx_queue);
> +		target_skb = priv->tx_queue.prev;
>  		largest_hole = max(largest_hole, priv->rx_end - last_addr - len);
>  		if (!skb_queue_empty(&priv->tx_queue)) {
>  			info = IEEE80211_SKB_CB(target_skb);
> @@ -2084,6 +2090,7 @@ out:
>  static void p54_stop(struct ieee80211_hw *dev)
>  {
>  	struct p54_common *priv = dev->priv;
> +	struct sk_buff *skb;
>  
>  	mutex_lock(&priv->conf_mutex);
>  	priv->mode = NL80211_IFTYPE_UNSPECIFIED;
> @@ -2098,7 +2105,8 @@ static void p54_stop(struct ieee80211_hw *dev)
>  		p54_tx_cancel(dev, priv->cached_beacon);
>  
>  	priv->stop(dev);
> -	skb_queue_purge(&priv->tx_queue);
> +	while ((skb = skb_dequeue(&priv->tx_queue)))
> +		kfree_skb(skb);
>  	priv->cached_beacon = NULL;
>  	priv->tsf_high32 = priv->tsf_low32 = 0;
>  	mutex_unlock(&priv->conf_mutex);

This patch fixes my USB device.

Please push to Linus.

Thanks,

Larry

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[David Miller]

From: Larry Finger <Larry.Finger@lwfinger.net>
Date: Mon, 06 Jul 2009 15:42:22 -0500

> This patch fixes my USB device.

Thanks for testing.

> Please push to Linus.

Will do.

Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context

[Bjarke Istrup Pedersen]

2009/7/6 Larry Finger <Larry.Finger@lwfinger.net>:
> Beginning with commit a1091aae19b1d9c85d91c86915a611387f67a26b entitled
> "p54: Use SKB list handling helpers instead of by-hand code.", the following
> appears in my logs whenever I try to use my p54usb wireless device:
>
> BUG: sleeping function called from invalid context at arch/x86/mm/fault.c:1085
>  in_atomic(): 0, irqs_disabled(): 1, pid: 8538, name: modprobe
>  2 locks held by modprobe/8538:
>  #0:  (&local->queue_stop_reason_lock){-.-...}, at: [<ffffffffa023b99f>]
>        ieee80211_stop_queues_by_reason+0x28/0x5e [mac80211]
>  #1:  (&mm->mmap_sem){++++++}, at: [<ffffffff80225a4f>] do_page_fault+0xd2/0x1fe
>  irq event stamp: 25212
>  hardirqs last  enabled at (25211): [<ffffffff80451f28>] _spin_unlock_irqrestore+0x3f/0x47
>  hardirqs last disabled at (25212): [<ffffffff804520e3>] _spin_lock_irqsave+0x17/0x4b
>  softirqs last  enabled at (24064): [<ffffffff803e9997>] sk_filter+0xba/0xc3
>  softirqs last disabled at (24062): [<ffffffff803e98f3>] sk_filter+0x16/0xc3
>  Pid: 8538, comm: modprobe Not tainted 2.6.30-rc6-Linus-00905-g46c3767-dirty #180
>  Call Trace:
>  [<ffffffff8025af90>] ? print_irqtrace_events+0xd0/0xd4
>  [<ffffffff80233384>] __might_sleep+0xf4/0xf6
>  [<ffffffff80225a99>] do_page_fault+0x11c/0x1fe
>  [<ffffffff80451f28>] ? _spin_unlock_irqrestore+0x3f/0x47
>  [<ffffffff804525cf>] page_fault+0x1f/0x30
>  [<ffffffff80451f28>] ? _spin_unlock_irqrestore+0x3f/0x47
>  [<ffffffffa023b99f>] ? ieee80211_stop_queues_by_reason+0x28/0x5e [mac80211]
>  [<ffffffffa023b92b>] ? __ieee80211_stop_queue+0x36/0x82 [mac80211]
>  [<ffffffff8045210b>] ? _spin_lock_irqsave+0x3f/0x4b
>  [<ffffffffa023b9b3>] ieee80211_stop_queues_by_reason+0x3c/0x5e [mac80211]
>  [<ffffffffa023b9e0>] ieee80211_stop_queues+0xb/0xd [mac80211]
>  [<ffffffffa047c35c>] p54_assign_address+0x164/0x1ec [p54common]
>  [<ffffffffa047c49a>] p54_alloc_skb+0xb6/0xd3 [p54common]
> ...
>
> Reverting the hunk that affects p54_assign_address() fixes the problem. When I
> tried to determine which change(s) caused the problem, the skb_peek_tail()
> seemed to be the problem; however, the system would freeze. I was not able to
> recover any log information.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>
> diff --git a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
> index b618bd1..c5bc55a 100644
> --- a/drivers/net/wireless/p54/p54common.c
> +++ b/drivers/net/wireless/p54/p54common.c
> @@ -1164,21 +1164,23 @@ static int p54_assign_address(struct ieee80211_hw *dev, struct sk_buff *skb,
>                }
>        }
>
> -       skb_queue_walk(&priv->tx_queue, entry) {
> +       entry = priv->tx_queue.next;
> +       while (left--) {
>                u32 hole_size;
>                info = IEEE80211_SKB_CB(entry);
>                range = (void *)info->rate_driver_data;
>                hole_size = range->start_addr - last_addr;
>                if (!target_skb && hole_size >= len) {
> -                       target_skb = skb_queue_prev(&priv->tx_queue, entry);
> +                       target_skb = entry->prev;
>                        hole_size -= len;
>                        target_addr = last_addr;
>                }
>                largest_hole = max(largest_hole, hole_size);
>                last_addr = range->end_addr;
> +               entry = entry->next;
>        }
>        if (!target_skb && priv->rx_end - last_addr >= len) {
> -               target_skb = skb_peek_tail(&priv->tx_queue);
> +               target_skb = priv->tx_queue.prev;
>                largest_hole = max(largest_hole, priv->rx_end - last_addr - len);
>                if (!skb_queue_empty(&priv->tx_queue)) {
>                        info = IEEE80211_SKB_CB(target_skb);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

I've been having this problem too.
Just completed a bisect to track the commit causing the problem when
this mail hit my inbox :-)

The oops of it is logged it kerneloops here:
http://kerneloops.org/guilty.php?guilty=__ieee80211_stop_queue&version=2.6.31-rc&start=2031616&end=2064383&class=oops

Best regards,
Bjarke Istrup Pedersen