Dave,
Here is the latest batch of fixes intended for 3.2. This includes a
correction for a user-visible error in mac80211's debugfs info, a fix
for a potential memory corrupter in prism54, an endian fix for rt2x00,
an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
for handling some spurious interrupts that hardware can generate.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit f23aa62545c18728eb2b9434aa258be27e07dd49:
caif: fix endian conversion in cffrml_transmit() (2011-11-21 16:46:24 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
Ben Greear (1):
mac80211: Fix AMSDU rate printout in debugfs.
Dan Carpenter (1):
prism54: potential memory corruption in prism54_get_essid()
Gertjan van Wingerde (1):
rt2x00: Fix efuse EEPROM reading on PPC32.
Helmut Schaa (1):
mac80211: Fix endian bug in radiotap header generation
Johannes Berg (1):
cfg80211: fix regulatory NULL dereference
John W. Linville (1):
Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem
Michael Buesch (2):
p54spi: Add missing spin_lock_init
p54spi: Fix workqueue deadlock
Stanislaw Gruszka (2):
rt2800pci: handle spurious interrupts
rt2x00: handle spurious pci interrupts
drivers/net/wireless/p54/p54spi.c | 5 +++--
drivers/net/wireless/prism54/isl_ioctl.c | 2 +-
drivers/net/wireless/rt2x00/rt2400pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2500pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
drivers/net/wireless/rt2x00/rt2800pci.c | 7 ++++++-
drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
net/mac80211/debugfs_sta.c | 4 ++--
net/mac80211/status.c | 8 ++++----
net/wireless/reg.c | 4 ++++
10 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
index f18df82..78d0d69 100644
--- a/drivers/net/wireless/p54/p54spi.c
+++ b/drivers/net/wireless/p54/p54spi.c
@@ -588,8 +588,6 @@ static void p54spi_op_stop(struct ieee80211_hw *dev)
WARN_ON(priv->fw_state != FW_STATE_READY);
- cancel_work_sync(&priv->work);
-
p54spi_power_off(priv);
spin_lock_irqsave(&priv->tx_lock, flags);
INIT_LIST_HEAD(&priv->tx_pending);
@@ -597,6 +595,8 @@ static void p54spi_op_stop(struct ieee80211_hw *dev)
priv->fw_state = FW_STATE_OFF;
mutex_unlock(&priv->mutex);
+
+ cancel_work_sync(&priv->work);
}
static int __devinit p54spi_probe(struct spi_device *spi)
@@ -656,6 +656,7 @@ static int __devinit p54spi_probe(struct spi_device *spi)
init_completion(&priv->fw_comp);
INIT_LIST_HEAD(&priv->tx_pending);
mutex_init(&priv->mutex);
+ spin_lock_init(&priv->tx_lock);
SET_IEEE80211_DEV(hw, &spi->dev);
priv->common.open = p54spi_op_start;
priv->common.stop = p54spi_op_stop;
diff --git a/drivers/net/wireless/prism54/isl_ioctl.c b/drivers/net/wireless/prism54/isl_ioctl.c
index d97a2ca..bc2ba80 100644
--- a/drivers/net/wireless/prism54/isl_ioctl.c
+++ b/drivers/net/wireless/prism54/isl_ioctl.c
@@ -778,7 +778,7 @@ prism54_get_essid(struct net_device *ndev, struct iw_request_info *info,
dwrq->flags = 0;
dwrq->length = 0;
}
- essid->octets[essid->length] = '\0';
+ essid->octets[dwrq->length] = '\0';
memcpy(extra, essid->octets, dwrq->length);
kfree(essid);
diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 3a6b402..676c765 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -1380,7 +1380,7 @@ static irqreturn_t rt2400pci_interrupt(int irq, void *dev_instance)
rt2x00pci_register_write(rt2x00dev, CSR7, reg);
if (!reg)
- return IRQ_NONE;
+ return IRQ_HANDLED;
if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
return IRQ_HANDLED;
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index dcc0e1f..d0b627c 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -1512,7 +1512,7 @@ static irqreturn_t rt2500pci_interrupt(int irq, void *dev_instance)
rt2x00pci_register_write(rt2x00dev, CSR7, reg);
if (!reg)
- return IRQ_NONE;
+ return IRQ_HANDLED;
if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
return IRQ_HANDLED;
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 3f183a1..1ba079d 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -3771,7 +3771,7 @@ static void rt2800_efuse_read(struct rt2x00_dev *rt2x00dev, unsigned int i)
/* Apparently the data is read from end to start */
rt2800_register_read_lock(rt2x00dev, EFUSE_DATA3, ®);
/* The returned value is in CPU order, but eeprom is le */
- rt2x00dev->eeprom[i] = cpu_to_le32(reg);
+ *(u32 *)&rt2x00dev->eeprom[i] = cpu_to_le32(reg);
rt2800_register_read_lock(rt2x00dev, EFUSE_DATA2, ®);
*(u32 *)&rt2x00dev->eeprom[i + 2] = cpu_to_le32(reg);
rt2800_register_read_lock(rt2x00dev, EFUSE_DATA1, ®);
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index da48c8a..4dc2d0f 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -880,8 +880,13 @@ static irqreturn_t rt2800pci_interrupt(int irq, void *dev_instance)
rt2x00pci_register_read(rt2x00dev, INT_SOURCE_CSR, ®);
rt2x00pci_register_write(rt2x00dev, INT_SOURCE_CSR, reg);
+ /*
+ * Some devices can generate interrupts with empty CSR register, we
+ * "handle" such irq's to prevent interrupt controller treat them as
+ * spurious interrupts and disable irq line.
+ */
if (!reg)
- return IRQ_NONE;
+ return IRQ_HANDLED;
if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
return IRQ_HANDLED;
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index bf55b4a..9d83e70 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2337,7 +2337,7 @@ static irqreturn_t rt61pci_interrupt(int irq, void *dev_instance)
rt2x00pci_register_write(rt2x00dev, INT_SOURCE_CSR, reg);
if (!reg && !reg_mcu)
- return IRQ_NONE;
+ return IRQ_HANDLED;
if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
return IRQ_HANDLED;
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index c5f3417..3110cbd 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -274,9 +274,9 @@ static ssize_t sta_ht_capa_read(struct file *file, char __user *userbuf,
PRINT_HT_CAP((htc->cap & BIT(10)), "HT Delayed Block Ack");
- PRINT_HT_CAP((htc->cap & BIT(11)), "Max AMSDU length: "
- "3839 bytes");
PRINT_HT_CAP(!(htc->cap & BIT(11)), "Max AMSDU length: "
+ "3839 bytes");
+ PRINT_HT_CAP((htc->cap & BIT(11)), "Max AMSDU length: "
"7935 bytes");
/*
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 80de436..16518f3 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -260,7 +260,7 @@ static void ieee80211_add_tx_radiotap_header(struct ieee80211_supported_band
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
struct ieee80211_radiotap_header *rthdr;
unsigned char *pos;
- __le16 txflags;
+ u16 txflags;
rthdr = (struct ieee80211_radiotap_header *) skb_push(skb, rtap_len);
@@ -290,13 +290,13 @@ static void ieee80211_add_tx_radiotap_header(struct ieee80211_supported_band
txflags = 0;
if (!(info->flags & IEEE80211_TX_STAT_ACK) &&
!is_multicast_ether_addr(hdr->addr1))
- txflags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_FAIL);
+ txflags |= IEEE80211_RADIOTAP_F_TX_FAIL;
if ((info->status.rates[0].flags & IEEE80211_TX_RC_USE_RTS_CTS) ||
(info->status.rates[0].flags & IEEE80211_TX_RC_USE_CTS_PROTECT))
- txflags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_CTS);
+ txflags |= IEEE80211_RADIOTAP_F_TX_CTS;
else if (info->status.rates[0].flags & IEEE80211_TX_RC_USE_RTS_CTS)
- txflags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_RTS);
+ txflags |= IEEE80211_RADIOTAP_F_TX_RTS;
put_unaligned_le16(txflags, pos);
pos += 2;
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index e71f5a6..77e9267 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2037,6 +2037,10 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
}
request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
+ if (!request_wiphy) {
+ reg_set_request_processed();
+ return -ENODEV;
+ }
if (!last_request->intersect) {
int r;
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On 11/22/2011 03:13 PM, David Miller wrote:
> From: David Miller<[email protected]>
> Date: Tue, 22 Nov 2011 16:05:11 -0500 (EST)
>
>> From: "John W. Linville"<[email protected]>
>> Date: Tue, 22 Nov 2011 15:56:55 -0500
>>
>>> On Tue, Nov 22, 2011 at 03:14:29PM -0500, David Miller wrote:
>>>> From: "John W. Linville"<[email protected]>
>>>> Date: Tue, 22 Nov 2011 14:35:05 -0500
>>>>
>>>>> Here is the latest batch of fixes intended for 3.2. This includes a
>>>>> correction for a user-visible error in mac80211's debugfs info, a fix
>>>>> for a potential memory corrupter in prism54, an endian fix for rt2x00,
>>>>> an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
>>>>> locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
>>>>> for handling some spurious interrupts that hardware can generate.
>>>>>
>>>>> Please let me know if there are problems!
>>>>
>>>> The rt2800pci change doesn't look correct.
>>>>
>>>> If the IRQ line is shared with another device, this change will make it
>>>> never see interrupts. Once you say "IRQ_HANDLED" the IRQ dispatch
>>>> stops processing the interrupt handler list.
>>>
>>> I thought this at first as well. But looking at the code in
>>> kernel/irq/handle.c doesn't support that conclusion. In fact, every
>>> handler gets invoked no matter what they all return. All of the irq
>>> handler return values are ORed together and passed to note_interrupt.
>>> Only if every irq handler returns IRQ_NONE does the code in
>>> kernel/irq/spurious.c start getting involved.
>>>
>>> Anyway, this seems to be safe even for shared interrupts. That said,
>>> this is a bit ugly. But it makes a serious difference in performance
>>> for those afflicted with this issue.
>>
>> It just means that we won't notice spurious interrupts if the device
>> sharing the line with rt2800pci generates one.
>>
>> This change is wrong.
>
> BTW, look at it this way, if what you say is true John then what's the point
> in returning any specific value at all?
>
> Everyone can just return IRQ_HANDLED and everything would just work.
>
> But you know that's not the case, and that it's important that this value
> is returned accurately.
I was trying to find the thread that reported the improvement in performance
with this change, but failed. Is it possible that their change just papered over
an interrupt storm from some other device that shared that interrupt?
I'm following this because the rtlwifi-family of drivers already did what was
suggested here. If this is wrong, then so is it.
Larry
From: "John W. Linville" <[email protected]>
Date: Tue, 22 Nov 2011 14:35:05 -0500
> Here is the latest batch of fixes intended for 3.2. This includes a
> correction for a user-visible error in mac80211's debugfs info, a fix
> for a potential memory corrupter in prism54, an endian fix for rt2x00,
> an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
> locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
> for handling some spurious interrupts that hardware can generate.
>
> Please let me know if there are problems!
The rt2800pci change doesn't look correct.
If the IRQ line is shared with another device, this change will make it
never see interrupts. Once you say "IRQ_HANDLED" the IRQ dispatch
stops processing the interrupt handler list.
From: Larry Finger <[email protected]>
Date: Tue, 22 Nov 2011 15:30:33 -0600
> On 11/22/2011 03:13 PM, David Miller wrote:
>> From: David Miller<[email protected]>
>> Date: Tue, 22 Nov 2011 16:05:11 -0500 (EST)
>>
>>> From: "John W. Linville"<[email protected]>
>>> Date: Tue, 22 Nov 2011 15:56:55 -0500
>>>
>>>> On Tue, Nov 22, 2011 at 03:14:29PM -0500, David Miller wrote:
>>>>> From: "John W. Linville"<[email protected]>
>>>>> Date: Tue, 22 Nov 2011 14:35:05 -0500
>>>>>
>>>>>> Here is the latest batch of fixes intended for 3.2. This includes a
>>>>>> correction for a user-visible error in mac80211's debugfs info, a fix
>>>>>> for a potential memory corrupter in prism54, an endian fix for rt2x00,
>>>>>> an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
>>>>>> locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
>>>>>> for handling some spurious interrupts that hardware can generate.
>>>>>>
>>>>>> Please let me know if there are problems!
>>>>>
>>>>> The rt2800pci change doesn't look correct.
>>>>>
>>>>> If the IRQ line is shared with another device, this change will make
>>>>> it
>>>>> never see interrupts. Once you say "IRQ_HANDLED" the IRQ dispatch
>>>>> stops processing the interrupt handler list.
>>>>
>>>> I thought this at first as well. But looking at the code in
>>>> kernel/irq/handle.c doesn't support that conclusion. In fact, every
>>>> handler gets invoked no matter what they all return. All of the irq
>>>> handler return values are ORed together and passed to note_interrupt.
>>>> Only if every irq handler returns IRQ_NONE does the code in
>>>> kernel/irq/spurious.c start getting involved.
>>>>
>>>> Anyway, this seems to be safe even for shared interrupts. That said,
>>>> this is a bit ugly. But it makes a serious difference in performance
>>>> for those afflicted with this issue.
>>>
>>> It just means that we won't notice spurious interrupts if the device
>>> sharing the line with rt2800pci generates one.
>>>
>>> This change is wrong.
>>
>> BTW, look at it this way, if what you say is true John then what's the
>> point
>> in returning any specific value at all?
>>
>> Everyone can just return IRQ_HANDLED and everything would just work.
>>
>> But you know that's not the case, and that it's important that this
>> value
>> is returned accurately.
>
> I was trying to find the thread that reported the improvement in
> performance with this change, but failed. Is it possible that their
> change just papered over an interrupt storm from some other device
> that shared that interrupt?
It doesn't fix a performance problem, it fixes a problem wherein the
IRQ line is disabled by the generic IRQ code because all handlers
return IRQ_NONE.
On Tue, Nov 22, 2011 at 05:37:29PM -0500, David Miller wrote:
> From: "John W. Linville" <[email protected]>
> Date: Tue, 22 Nov 2011 16:56:12 -0500
>
> > Here is the latest batch of fixes intended for 3.2. This includes a
> > correction for a user-visible error in mac80211's debugfs info, a fix
> > for a potential memory corrupter in prism54, an endian fix for rt2x00,
> > an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
> > locking fix for p54spi and a deadlock fix also for p54spi.
> >
> > This reverts the problematic rt2x00 patches from the earlier pull
> > request.
> >
> > Please let me know if there are problems!
>
> I'm still seeing the problematic rt2x00 changes when I pull:
>
> [davem@boricha net]$ git pull git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
> remote: Counting objects: 112, done.
> remote: Compressing objects: 100% (82/82), done.
> remote: Total 82 (delta 71), reused 0 (delta 0)
> Unpacking objects: 100% (82/82), done.
> From git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless
> * branch for-davem -> FETCH_HEAD
> Updating 5eccdf5..02f1ce3
> Fast-forward
> drivers/net/wireless/p54/p54spi.c | 5 +++--
> drivers/net/wireless/prism54/isl_ioctl.c | 2 +-
> drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
> net/mac80211/debugfs_sta.c | 4 ++--
> net/mac80211/status.c | 8 ++++----
> net/wireless/reg.c | 4 ++++
> 6 files changed, 15 insertions(+), 10 deletions(-)
> [davem@boricha net]$ git format-patch ORIG_HEAD
> 0001-rt2800pci-handle-spurious-interrupts.patch
> 0002-rt2x00-handle-spurious-pci-interrupts.patch
They're here...
> 0003-rt2x00-Fix-efuse-EEPROM-reading-on-PPC32.patch
> 0004-p54spi-Add-missing-spin_lock_init.patch
> 0005-p54spi-Fix-workqueue-deadlock.patch
> 0006-mac80211-Fix-AMSDU-rate-printout-in-debugfs.patch
> 0007-mac80211-Fix-endian-bug-in-radiotap-header-generatio.patch
> 0008-cfg80211-fix-regulatory-NULL-dereference.patch
> 0009-prism54-potential-memory-corruption-in-prism54_get_e.patch
> 0010-Revert-rt2x00-handle-spurious-pci-interrupts.patch
> 0011-Revert-rt2800pci-handle-spurious-interrupts.patch
And I revert them here.
Linus always says to live with our mistakes? I was trying to avoid
a rebase both for my own pain and that of my downstream maintainers.
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
From: Stanislaw Gruszka <[email protected]>
Date: Wed, 23 Nov 2011 09:03:54 +0100
> The main problem here, that we have hardware (or firmware) that
> generate interrupt with empty interrupt status register, so we can
> not detect if interrupt is really generated by Ralink device.
>
> Perhaps exist good fix for that problem, i.e. we can write to some
> register to stop hardware generating spurious interrupts, or exist
> other way than reading status register to find out if Ralink device
> generated interrupt. But that require detailed hardware knowledge,
> and I'm not sure if we ever can get such informations. Also this
> could be a hardware bug, device just generate spurious interrupts
> and we can not do anything about it. I looked at driver from Ralink
> site, and it do exactly that, it return IRQ_HANDLED if status register
> is empty.
Then it is also buggy :-)
> Ok, I'm thinking about other fix now ...
Thank you.
From: "John W. Linville" <[email protected]>
Date: Tue, 22 Nov 2011 16:56:12 -0500
> Here is the latest batch of fixes intended for 3.2. This includes a
> correction for a user-visible error in mac80211's debugfs info, a fix
> for a potential memory corrupter in prism54, an endian fix for rt2x00,
> an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
> locking fix for p54spi and a deadlock fix also for p54spi.
>
> This reverts the problematic rt2x00 patches from the earlier pull
> request.
>
> Please let me know if there are problems!
I'm still seeing the problematic rt2x00 changes when I pull:
[davem@boricha net]$ git pull git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
remote: Counting objects: 112, done.
remote: Compressing objects: 100% (82/82), done.
remote: Total 82 (delta 71), reused 0 (delta 0)
Unpacking objects: 100% (82/82), done.
>From git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless
* branch for-davem -> FETCH_HEAD
Updating 5eccdf5..02f1ce3
Fast-forward
drivers/net/wireless/p54/p54spi.c | 5 +++--
drivers/net/wireless/prism54/isl_ioctl.c | 2 +-
drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
net/mac80211/debugfs_sta.c | 4 ++--
net/mac80211/status.c | 8 ++++----
net/wireless/reg.c | 4 ++++
6 files changed, 15 insertions(+), 10 deletions(-)
[davem@boricha net]$ git format-patch ORIG_HEAD
0001-rt2800pci-handle-spurious-interrupts.patch
0002-rt2x00-handle-spurious-pci-interrupts.patch
0003-rt2x00-Fix-efuse-EEPROM-reading-on-PPC32.patch
0004-p54spi-Add-missing-spin_lock_init.patch
0005-p54spi-Fix-workqueue-deadlock.patch
0006-mac80211-Fix-AMSDU-rate-printout-in-debugfs.patch
0007-mac80211-Fix-endian-bug-in-radiotap-header-generatio.patch
0008-cfg80211-fix-regulatory-NULL-dereference.patch
0009-prism54-potential-memory-corruption-in-prism54_get_e.patch
0010-Revert-rt2x00-handle-spurious-pci-interrupts.patch
0011-Revert-rt2800pci-handle-spurious-interrupts.patch
[davem@boricha net]$ head 0001-rt2800pci-handle-spurious-interrupts.patch
>From 4ba7d9997869d25bd223dea7536fc1ce9fab3b3b Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <[email protected]>
Date: Wed, 16 Nov 2011 11:09:17 +0100
Subject: [PATCH 01/11] rt2800pci: handle spurious interrupts
Some devices may generate spurious interrupts, we have to handle them
otherwise interrupt line will be disabled with below message and driver
will not work:
[ 2052.114334] irq 17: nobody cared (try booting with the "irqpoll" option)
[davem@boricha net]$
On Tue, Nov 22, 2011 at 03:30:33PM -0600, Larry Finger wrote:
> On 11/22/2011 03:13 PM, David Miller wrote:
> >From: David Miller<[email protected]>
> >Date: Tue, 22 Nov 2011 16:05:11 -0500 (EST)
> >Everyone can just return IRQ_HANDLED and everything would just work.
> >
> >But you know that's not the case, and that it's important that this value
> >is returned accurately.
>
> I was trying to find the thread that reported the improvement in
> performance with this change, but failed. Is it possible that their
> change just papered over an interrupt storm from some other device
> that shared that interrupt?
Original patch was posted here:
http://marc.info/?l=linux-wireless&m=132143811916674&w=2
And the Red Hat Bugzilla entry is here:
https://bugzilla.redhat.com/show_bug.cgi?id=658451
> I'm following this because the rtlwifi-family of drivers already did
> what was suggested here. If this is wrong, then so is it.
Sounds like that nees to be fixed...
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On Tue, Nov 22, 2011 at 04:13:22PM -0500, David Miller wrote:
> From: David Miller <[email protected]>
> Date: Tue, 22 Nov 2011 16:05:11 -0500 (EST)
>
> > From: "John W. Linville" <[email protected]>
> > Date: Tue, 22 Nov 2011 15:56:55 -0500
> >
> >> On Tue, Nov 22, 2011 at 03:14:29PM -0500, David Miller wrote:
> >>> From: "John W. Linville" <[email protected]>
> >>> Date: Tue, 22 Nov 2011 14:35:05 -0500
> >>>
> >>> > Here is the latest batch of fixes intended for 3.2. This includes a
> >>> > correction for a user-visible error in mac80211's debugfs info, a fix
> >>> > for a potential memory corrupter in prism54, an endian fix for rt2x00,
> >>> > an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
> >>> > locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
> >>> > for handling some spurious interrupts that hardware can generate.
> >>> >
> >>> > Please let me know if there are problems!
> >>>
> >>> The rt2800pci change doesn't look correct.
> >>>
> >>> If the IRQ line is shared with another device, this change will make it
> >>> never see interrupts. Once you say "IRQ_HANDLED" the IRQ dispatch
> >>> stops processing the interrupt handler list.
> >>
> >> I thought this at first as well. But looking at the code in
> >> kernel/irq/handle.c doesn't support that conclusion. In fact, every
> >> handler gets invoked no matter what they all return. All of the irq
> >> handler return values are ORed together and passed to note_interrupt.
> >> Only if every irq handler returns IRQ_NONE does the code in
> >> kernel/irq/spurious.c start getting involved.
> >>
> >> Anyway, this seems to be safe even for shared interrupts. That said,
> >> this is a bit ugly. But it makes a serious difference in performance
> >> for those afflicted with this issue.
> >
> > It just means that we won't notice spurious interrupts if the device
> > sharing the line with rt2800pci generates one.
> >
> > This change is wrong.
>
> BTW, look at it this way, if what you say is true John then what's the point
> in returning any specific value at all?
>
> Everyone can just return IRQ_HANDLED and everything would just work.
>
> But you know that's not the case, and that it's important that this value
> is returned accurately.
Alright, that a good point. I'll drop those two patches.
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
From: "John W. Linville" <[email protected]>
Date: Tue, 22 Nov 2011 17:41:08 -0500
> On Tue, Nov 22, 2011 at 05:37:29PM -0500, David Miller wrote:
> ...
> They're here...
>
>> 0003-rt2x00-Fix-efuse-EEPROM-reading-on-PPC32.patch
>> 0004-p54spi-Add-missing-spin_lock_init.patch
>> 0005-p54spi-Fix-workqueue-deadlock.patch
>> 0006-mac80211-Fix-AMSDU-rate-printout-in-debugfs.patch
>> 0007-mac80211-Fix-endian-bug-in-radiotap-header-generatio.patch
>> 0008-cfg80211-fix-regulatory-NULL-dereference.patch
>> 0009-prism54-potential-memory-corruption-in-prism54_get_e.patch
>> 0010-Revert-rt2x00-handle-spurious-pci-interrupts.patch
>> 0011-Revert-rt2800pci-handle-spurious-interrupts.patch
>
> And I revert them here.
>
> Linus always says to live with our mistakes? I was trying to avoid
> a rebase both for my own pain and that of my downstream maintainers.
Gotcha, I'll repull, thanks.
On Tue, Nov 22, 2011 at 04:40:57PM -0500, David Miller wrote:
> >>> It just means that we won't notice spurious interrupts if the device
> >>> sharing the line with rt2800pci generates one.
> >>>
> >>> This change is wrong.
Ehh, I had a feeling that I was doing something wrong ...
> >> BTW, look at it this way, if what you say is true John then what's the
> >> point
> >> in returning any specific value at all?
> >>
> >> Everyone can just return IRQ_HANDLED and everything would just work.
> >>
> >> But you know that's not the case, and that it's important that this
> >> value
> >> is returned accurately.
> >
> > I was trying to find the thread that reported the improvement in
> > performance with this change, but failed. Is it possible that their
> > change just papered over an interrupt storm from some other device
> > that shared that interrupt?
>
> It doesn't fix a performance problem, it fixes a problem wherein the
> IRQ line is disabled by the generic IRQ code because all handlers
> return IRQ_NONE.
According to Amir, who reports this problem patch fix the performance
(https://bugzilla.redhat.com/show_bug.cgi?id=658451),
from very bad to expected on wifi link. It can be explained, if we
disable interrupt line, driver still operate, but read TX/RX statuses
from hardware when internal driver watchdog timeout. Hence we operate,
but very slow. If we prevent to disable interrupt by IRQ controller
driver, device operate normally.
The main problem here, that we have hardware (or firmware) that
generate interrupt with empty interrupt status register, so we can
not detect if interrupt is really generated by Ralink device.
Perhaps exist good fix for that problem, i.e. we can write to some
register to stop hardware generating spurious interrupts, or exist
other way than reading status register to find out if Ralink device
generated interrupt. But that require detailed hardware knowledge,
and I'm not sure if we ever can get such informations. Also this
could be a hardware bug, device just generate spurious interrupts
and we can not do anything about it. I looked at driver from Ralink
site, and it do exactly that, it return IRQ_HANDLED if status register
is empty.
Ok, I'm thinking about other fix now ...
Stanislaw
Dave,
Here is the latest batch of fixes intended for 3.2. This includes a
correction for a user-visible error in mac80211's debugfs info, a fix
for a potential memory corrupter in prism54, an endian fix for rt2x00,
an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
locking fix for p54spi and a deadlock fix also for p54spi.
This reverts the problematic rt2x00 patches from the earlier pull
request.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit 5eccdf5e06eb67779716ae26142402a1ae9b012c:
tc: comment spelling fixes (2011-11-22 16:37:01 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
Ben Greear (1):
mac80211: Fix AMSDU rate printout in debugfs.
Dan Carpenter (1):
prism54: potential memory corruption in prism54_get_essid()
Gertjan van Wingerde (1):
rt2x00: Fix efuse EEPROM reading on PPC32.
Helmut Schaa (1):
mac80211: Fix endian bug in radiotap header generation
Johannes Berg (1):
cfg80211: fix regulatory NULL dereference
John W. Linville (3):
Revert "rt2x00: handle spurious pci interrupts"
Revert "rt2800pci: handle spurious interrupts"
Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem
Michael Buesch (2):
p54spi: Add missing spin_lock_init
p54spi: Fix workqueue deadlock
Stanislaw Gruszka (2):
rt2800pci: handle spurious interrupts
rt2x00: handle spurious pci interrupts
drivers/net/wireless/p54/p54spi.c | 5 +++--
drivers/net/wireless/prism54/isl_ioctl.c | 2 +-
drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
net/mac80211/debugfs_sta.c | 4 ++--
net/mac80211/status.c | 8 ++++----
net/wireless/reg.c | 4 ++++
6 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
index f18df82..78d0d69 100644
--- a/drivers/net/wireless/p54/p54spi.c
+++ b/drivers/net/wireless/p54/p54spi.c
@@ -588,8 +588,6 @@ static void p54spi_op_stop(struct ieee80211_hw *dev)
WARN_ON(priv->fw_state != FW_STATE_READY);
- cancel_work_sync(&priv->work);
-
p54spi_power_off(priv);
spin_lock_irqsave(&priv->tx_lock, flags);
INIT_LIST_HEAD(&priv->tx_pending);
@@ -597,6 +595,8 @@ static void p54spi_op_stop(struct ieee80211_hw *dev)
priv->fw_state = FW_STATE_OFF;
mutex_unlock(&priv->mutex);
+
+ cancel_work_sync(&priv->work);
}
static int __devinit p54spi_probe(struct spi_device *spi)
@@ -656,6 +656,7 @@ static int __devinit p54spi_probe(struct spi_device *spi)
init_completion(&priv->fw_comp);
INIT_LIST_HEAD(&priv->tx_pending);
mutex_init(&priv->mutex);
+ spin_lock_init(&priv->tx_lock);
SET_IEEE80211_DEV(hw, &spi->dev);
priv->common.open = p54spi_op_start;
priv->common.stop = p54spi_op_stop;
diff --git a/drivers/net/wireless/prism54/isl_ioctl.c b/drivers/net/wireless/prism54/isl_ioctl.c
index d97a2ca..bc2ba80 100644
--- a/drivers/net/wireless/prism54/isl_ioctl.c
+++ b/drivers/net/wireless/prism54/isl_ioctl.c
@@ -778,7 +778,7 @@ prism54_get_essid(struct net_device *ndev, struct iw_request_info *info,
dwrq->flags = 0;
dwrq->length = 0;
}
- essid->octets[essid->length] = '\0';
+ essid->octets[dwrq->length] = '\0';
memcpy(extra, essid->octets, dwrq->length);
kfree(essid);
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 3f183a1..1ba079d 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -3771,7 +3771,7 @@ static void rt2800_efuse_read(struct rt2x00_dev *rt2x00dev, unsigned int i)
/* Apparently the data is read from end to start */
rt2800_register_read_lock(rt2x00dev, EFUSE_DATA3, ®);
/* The returned value is in CPU order, but eeprom is le */
- rt2x00dev->eeprom[i] = cpu_to_le32(reg);
+ *(u32 *)&rt2x00dev->eeprom[i] = cpu_to_le32(reg);
rt2800_register_read_lock(rt2x00dev, EFUSE_DATA2, ®);
*(u32 *)&rt2x00dev->eeprom[i + 2] = cpu_to_le32(reg);
rt2800_register_read_lock(rt2x00dev, EFUSE_DATA1, ®);
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index c5f3417..3110cbd 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -274,9 +274,9 @@ static ssize_t sta_ht_capa_read(struct file *file, char __user *userbuf,
PRINT_HT_CAP((htc->cap & BIT(10)), "HT Delayed Block Ack");
- PRINT_HT_CAP((htc->cap & BIT(11)), "Max AMSDU length: "
- "3839 bytes");
PRINT_HT_CAP(!(htc->cap & BIT(11)), "Max AMSDU length: "
+ "3839 bytes");
+ PRINT_HT_CAP((htc->cap & BIT(11)), "Max AMSDU length: "
"7935 bytes");
/*
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 80de436..16518f3 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -260,7 +260,7 @@ static void ieee80211_add_tx_radiotap_header(struct ieee80211_supported_band
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
struct ieee80211_radiotap_header *rthdr;
unsigned char *pos;
- __le16 txflags;
+ u16 txflags;
rthdr = (struct ieee80211_radiotap_header *) skb_push(skb, rtap_len);
@@ -290,13 +290,13 @@ static void ieee80211_add_tx_radiotap_header(struct ieee80211_supported_band
txflags = 0;
if (!(info->flags & IEEE80211_TX_STAT_ACK) &&
!is_multicast_ether_addr(hdr->addr1))
- txflags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_FAIL);
+ txflags |= IEEE80211_RADIOTAP_F_TX_FAIL;
if ((info->status.rates[0].flags & IEEE80211_TX_RC_USE_RTS_CTS) ||
(info->status.rates[0].flags & IEEE80211_TX_RC_USE_CTS_PROTECT))
- txflags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_CTS);
+ txflags |= IEEE80211_RADIOTAP_F_TX_CTS;
else if (info->status.rates[0].flags & IEEE80211_TX_RC_USE_RTS_CTS)
- txflags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_RTS);
+ txflags |= IEEE80211_RADIOTAP_F_TX_RTS;
put_unaligned_le16(txflags, pos);
pos += 2;
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index e71f5a6..77e9267 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2037,6 +2037,10 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
}
request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
+ if (!request_wiphy) {
+ reg_set_request_processed();
+ return -ENODEV;
+ }
if (!last_request->intersect) {
int r;
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On Tue, Nov 22, 2011 at 03:14:29PM -0500, David Miller wrote:
> From: "John W. Linville" <[email protected]>
> Date: Tue, 22 Nov 2011 14:35:05 -0500
>
> > Here is the latest batch of fixes intended for 3.2. This includes a
> > correction for a user-visible error in mac80211's debugfs info, a fix
> > for a potential memory corrupter in prism54, an endian fix for rt2x00,
> > an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
> > locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
> > for handling some spurious interrupts that hardware can generate.
> >
> > Please let me know if there are problems!
>
> The rt2800pci change doesn't look correct.
>
> If the IRQ line is shared with another device, this change will make it
> never see interrupts. Once you say "IRQ_HANDLED" the IRQ dispatch
> stops processing the interrupt handler list.
I thought this at first as well. But looking at the code in
kernel/irq/handle.c doesn't support that conclusion. In fact, every
handler gets invoked no matter what they all return. All of the irq
handler return values are ORed together and passed to note_interrupt.
Only if every irq handler returns IRQ_NONE does the code in
kernel/irq/spurious.c start getting involved.
Anyway, this seems to be safe even for shared interrupts. That said,
this is a bit ugly. But it makes a serious difference in performance
for those afflicted with this issue.
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
From: David Miller <[email protected]>
Date: Tue, 22 Nov 2011 16:05:11 -0500 (EST)
> From: "John W. Linville" <[email protected]>
> Date: Tue, 22 Nov 2011 15:56:55 -0500
>
>> On Tue, Nov 22, 2011 at 03:14:29PM -0500, David Miller wrote:
>>> From: "John W. Linville" <[email protected]>
>>> Date: Tue, 22 Nov 2011 14:35:05 -0500
>>>
>>> > Here is the latest batch of fixes intended for 3.2. This includes a
>>> > correction for a user-visible error in mac80211's debugfs info, a fix
>>> > for a potential memory corrupter in prism54, an endian fix for rt2x00,
>>> > an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
>>> > locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
>>> > for handling some spurious interrupts that hardware can generate.
>>> >
>>> > Please let me know if there are problems!
>>>
>>> The rt2800pci change doesn't look correct.
>>>
>>> If the IRQ line is shared with another device, this change will make it
>>> never see interrupts. Once you say "IRQ_HANDLED" the IRQ dispatch
>>> stops processing the interrupt handler list.
>>
>> I thought this at first as well. But looking at the code in
>> kernel/irq/handle.c doesn't support that conclusion. In fact, every
>> handler gets invoked no matter what they all return. All of the irq
>> handler return values are ORed together and passed to note_interrupt.
>> Only if every irq handler returns IRQ_NONE does the code in
>> kernel/irq/spurious.c start getting involved.
>>
>> Anyway, this seems to be safe even for shared interrupts. That said,
>> this is a bit ugly. But it makes a serious difference in performance
>> for those afflicted with this issue.
>
> It just means that we won't notice spurious interrupts if the device
> sharing the line with rt2800pci generates one.
>
> This change is wrong.
BTW, look at it this way, if what you say is true John then what's the point
in returning any specific value at all?
Everyone can just return IRQ_HANDLED and everything would just work.
But you know that's not the case, and that it's important that this value
is returned accurately.
From: "John W. Linville" <[email protected]>
Date: Tue, 22 Nov 2011 15:56:55 -0500
> On Tue, Nov 22, 2011 at 03:14:29PM -0500, David Miller wrote:
>> From: "John W. Linville" <[email protected]>
>> Date: Tue, 22 Nov 2011 14:35:05 -0500
>>
>> > Here is the latest batch of fixes intended for 3.2. This includes a
>> > correction for a user-visible error in mac80211's debugfs info, a fix
>> > for a potential memory corrupter in prism54, an endian fix for rt2x00,
>> > an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
>> > locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
>> > for handling some spurious interrupts that hardware can generate.
>> >
>> > Please let me know if there are problems!
>>
>> The rt2800pci change doesn't look correct.
>>
>> If the IRQ line is shared with another device, this change will make it
>> never see interrupts. Once you say "IRQ_HANDLED" the IRQ dispatch
>> stops processing the interrupt handler list.
>
> I thought this at first as well. But looking at the code in
> kernel/irq/handle.c doesn't support that conclusion. In fact, every
> handler gets invoked no matter what they all return. All of the irq
> handler return values are ORed together and passed to note_interrupt.
> Only if every irq handler returns IRQ_NONE does the code in
> kernel/irq/spurious.c start getting involved.
>
> Anyway, this seems to be safe even for shared interrupts. That said,
> this is a bit ugly. But it makes a serious difference in performance
> for those afflicted with this issue.
It just means that we won't notice spurious interrupts if the device
sharing the line with rt2800pci generates one.
This change is wrong.