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 <[email protected]>
---
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);
From: Larry Finger <[email protected]>
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.
On Monday 06 July 2009 21:14:29 David Miller wrote:
> From: Larry Finger <[email protected]>
> 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
From: Christian Lamparter <[email protected]>
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.
David Miller wrote:
> From: Christian Lamparter <[email protected]>
> 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
From: Larry Finger <[email protected]>
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 <[email protected]>
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
2009/7/6 David Miller <[email protected]>:
> From: Larry Finger <[email protected]>
> 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 <[email protected]>
> 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 [email protected]
> 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
David Miller wrote:
> From: Larry Finger <[email protected]>
> 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 <[email protected]>
> 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
From: Bjarke Istrup Pedersen <[email protected]>
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 :)
From: Larry Finger <[email protected]>
Date: Mon, 06 Jul 2009 15:42:22 -0500
> This patch fixes my USB device.
Thanks for testing.
> Please push to Linus.
Will do.