The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
@ 2009-07-06 19:07 Larry Finger
  2009-07-06 19:14 ` David Miller
  0 siblings, 1 reply; 10+ messages in thread
From: Larry Finger @ 2009-07-06 19:07 UTC (permalink / raw)
  To: davem; +Cc: linux-wireless, chunkeey, linux-kernel

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);



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
  2009-07-06 19:07 [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context Larry Finger
@ 2009-07-06 19:14 ` David Miller
  2009-07-06 19:19   ` Christian Lamparter
  0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2009-07-06 19:14 UTC (permalink / raw)
  To: Larry.Finger; +Cc: linux-wireless, chunkeey, linux-kernel

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
  2009-07-06 19:14 ` David Miller
@ 2009-07-06 19:19   ` Christian Lamparter
  2009-07-06 19:34     ` David Miller
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Lamparter @ 2009-07-06 19:19 UTC (permalink / raw)
  To: David Miller; +Cc: Larry.Finger, linux-wireless, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
  2009-07-06 19:19   ` Christian Lamparter
@ 2009-07-06 19:34     ` David Miller
  2009-07-06 19:43       ` Larry Finger
  0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2009-07-06 19:34 UTC (permalink / raw)
  To: chunkeey; +Cc: Larry.Finger, linux-wireless, linux-kernel

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
  2009-07-06 19:34     ` David Miller
@ 2009-07-06 19:43       ` Larry Finger
  2009-07-06 19:50         ` David Miller
  0 siblings, 1 reply; 10+ messages in thread
From: Larry Finger @ 2009-07-06 19:43 UTC (permalink / raw)
  To: David Miller; +Cc: chunkeey, linux-wireless, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
  2009-07-06 19:43       ` Larry Finger
@ 2009-07-06 19:50         ` David Miller
  2009-07-06 20:26           ` Bjarke Istrup Pedersen
  2009-07-06 20:42           ` Larry Finger
  0 siblings, 2 replies; 10+ messages in thread
From: David Miller @ 2009-07-06 19:50 UTC (permalink / raw)
  To: Larry.Finger; +Cc: chunkeey, linux-wireless, linux-kernel

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa -  sleeping function called from invalid context
  2009-07-06 19:50         ` David Miller
@ 2009-07-06 20:26           ` Bjarke Istrup Pedersen
  2009-07-06 21:36             ` David Miller
  2009-07-06 20:42           ` Larry Finger
  1 sibling, 1 reply; 10+ messages in thread
From: Bjarke Istrup Pedersen @ 2009-07-06 20:26 UTC (permalink / raw)
  To: David Miller; +Cc: Larry.Finger, chunkeey, linux-wireless, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
  2009-07-06 19:50         ` David Miller
  2009-07-06 20:26           ` Bjarke Istrup Pedersen
@ 2009-07-06 20:42           ` Larry Finger
  2009-07-06 21:36             ` David Miller
  1 sibling, 1 reply; 10+ messages in thread
From: Larry Finger @ 2009-07-06 20:42 UTC (permalink / raw)
  To: David Miller; +Cc: chunkeey, linux-wireless, linux-kernel

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
  2009-07-06 20:26           ` Bjarke Istrup Pedersen
@ 2009-07-06 21:36             ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2009-07-06 21:36 UTC (permalink / raw)
  To: gurligebis; +Cc: Larry.Finger, chunkeey, linux-wireless, linux-kernel

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 :)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context
  2009-07-06 20:42           ` Larry Finger
@ 2009-07-06 21:36             ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2009-07-06 21:36 UTC (permalink / raw)
  To: Larry.Finger; +Cc: chunkeey, linux-wireless, linux-kernel

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-07-06 21:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-06 19:07 [PATCH] p54: Fix regression in 2.6.31-rcX since commit a1091aa - sleeping function called from invalid context Larry Finger
2009-07-06 19:14 ` David Miller
2009-07-06 19:19   ` Christian Lamparter
2009-07-06 19:34     ` David Miller
2009-07-06 19:43       ` Larry Finger
2009-07-06 19:50         ` David Miller
2009-07-06 20:26           ` Bjarke Istrup Pedersen
2009-07-06 21:36             ` David Miller
2009-07-06 20:42           ` Larry Finger
2009-07-06 21:36             ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox