From: Christian Lamparter <chunkeey@googlemail.com>
Date: Sun, 17 Jan 2010 22:19:25 +0000 (+0100)
Subject: p54pci: move tx cleanup into tasklet
X-Git-Tag: master-2010-01-19~9
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Flinville%2Fwireless-next-2.6.git;a=commitdiff_plain;h=d713804c6032b95cd3035014e16fadebb9655c6f
p54pci: move tx cleanup into tasklet
This patch moves the tx cleanup routines out of the critical
interrupt context and into the (previously known as rx) tasklet.
The main goal of this operation is to remove the extensive
usage of spin_lock_irqsaves in the generic p54common library.
The next step would be to modify p54usb to do the
rx processing inside a tasklet (just like usbnet).
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
index 4bf4c21..48cae48 100644
--- a/drivers/net/wireless/p54/p54pci.c
+++ b/drivers/net/wireless/p54/p54pci.c
@@ -234,25 +234,26 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
p54p_refill_rx_ring(dev, ring_index, ring, ring_limit, rx_buf);
}
-/* caller must hold priv->lock */
static void p54p_check_tx_ring(struct ieee80211_hw *dev, u32 *index,
int ring_index, struct p54p_desc *ring, u32 ring_limit,
- void **tx_buf)
+ struct sk_buff **tx_buf)
{
+ unsigned long flags;
struct p54p_priv *priv = dev->priv;
struct p54p_ring_control *ring_control = priv->ring_control;
struct p54p_desc *desc;
+ struct sk_buff *skb;
u32 idx, i;
i = (*index) % ring_limit;
(*index) = idx = le32_to_cpu(ring_control->device_idx[ring_index]);
idx %= ring_limit;
+ spin_lock_irqsave(&priv->lock, flags);
while (i != idx) {
desc = &ring[i];
- if (tx_buf[i])
- if (FREE_AFTER_TX((struct sk_buff *) tx_buf[i]))
- p54_free_skb(dev, tx_buf[i]);
+
+ skb = tx_buf[i];
tx_buf[i] = NULL;
pci_unmap_single(priv->pdev, le32_to_cpu(desc->host_addr),
@@ -263,17 +264,32 @@ static void p54p_check_tx_ring(struct ieee80211_hw *dev, u32 *index,
desc->len = 0;
desc->flags = 0;
+ if (skb && FREE_AFTER_TX(skb)) {
+ spin_unlock_irqrestore(&priv->lock, flags);
+ p54_free_skb(dev, skb);
+ spin_lock_irqsave(&priv->lock, flags);
+ }
+
i++;
i %= ring_limit;
}
+ spin_unlock_irqrestore(&priv->lock, flags);
}
-static void p54p_rx_tasklet(unsigned long dev_id)
+static void p54p_tasklet(unsigned long dev_id)
{
struct ieee80211_hw *dev = (struct ieee80211_hw *)dev_id;
struct p54p_priv *priv = dev->priv;
struct p54p_ring_control *ring_control = priv->ring_control;
+ p54p_check_tx_ring(dev, &priv->tx_idx_mgmt, 3, ring_control->tx_mgmt,
+ ARRAY_SIZE(ring_control->tx_mgmt),
+ priv->tx_buf_mgmt);
+
+ p54p_check_tx_ring(dev, &priv->tx_idx_data, 1, ring_control->tx_data,
+ ARRAY_SIZE(ring_control->tx_data),
+ priv->tx_buf_data);
+
p54p_check_rx_ring(dev, &priv->rx_idx_mgmt, 2, ring_control->rx_mgmt,
ARRAY_SIZE(ring_control->rx_mgmt), priv->rx_buf_mgmt);
@@ -288,38 +304,24 @@ static irqreturn_t p54p_interrupt(int irq, void *dev_id)
{
struct ieee80211_hw *dev = dev_id;
struct p54p_priv *priv = dev->priv;
- struct p54p_ring_control *ring_control = priv->ring_control;
__le32 reg;
spin_lock(&priv->lock);
reg = P54P_READ(int_ident);
if (unlikely(reg == cpu_to_le32(0xFFFFFFFF))) {
- spin_unlock(&priv->lock);
- return IRQ_HANDLED;
+ goto out;
}
-
P54P_WRITE(int_ack, reg);
reg &= P54P_READ(int_enable);
- if (reg & cpu_to_le32(ISL38XX_INT_IDENT_UPDATE)) {
- p54p_check_tx_ring(dev, &priv->tx_idx_mgmt,
- 3, ring_control->tx_mgmt,
- ARRAY_SIZE(ring_control->tx_mgmt),
- priv->tx_buf_mgmt);
-
- p54p_check_tx_ring(dev, &priv->tx_idx_data,
- 1, ring_control->tx_data,
- ARRAY_SIZE(ring_control->tx_data),
- priv->tx_buf_data);
-
- tasklet_schedule(&priv->rx_tasklet);
-
- } else if (reg & cpu_to_le32(ISL38XX_INT_IDENT_INIT))
+ if (reg & cpu_to_le32(ISL38XX_INT_IDENT_UPDATE))
+ tasklet_schedule(&priv->tasklet);
+ else if (reg & cpu_to_le32(ISL38XX_INT_IDENT_INIT))
complete(&priv->boot_comp);
+out:
spin_unlock(&priv->lock);
-
return reg ? IRQ_HANDLED : IRQ_NONE;
}
@@ -368,7 +370,7 @@ static void p54p_stop(struct ieee80211_hw *dev)
unsigned int i;
struct p54p_desc *desc;
- tasklet_kill(&priv->rx_tasklet);
+ tasklet_kill(&priv->tasklet);
P54P_WRITE(int_enable, cpu_to_le32(0));
P54P_READ(int_enable);
@@ -559,7 +561,7 @@ static int __devinit p54p_probe(struct pci_dev *pdev,
priv->common.tx = p54p_tx;
spin_lock_init(&priv->lock);
- tasklet_init(&priv->rx_tasklet, p54p_rx_tasklet, (unsigned long)dev);
+ tasklet_init(&priv->tasklet, p54p_tasklet, (unsigned long)dev);
err = request_firmware(&priv->firmware, "isl3886pci",
&priv->pdev->dev);
diff --git a/drivers/net/wireless/p54/p54pci.h b/drivers/net/wireless/p54/p54pci.h
index fbb6839..2feead6 100644
--- a/drivers/net/wireless/p54/p54pci.h
+++ b/drivers/net/wireless/p54/p54pci.h
@@ -92,7 +92,7 @@ struct p54p_priv {
struct p54_common common;
struct pci_dev *pdev;
struct p54p_csr __iomem *map;
- struct tasklet_struct rx_tasklet;
+ struct tasklet_struct tasklet;
const struct firmware *firmware;
spinlock_t lock;
struct p54p_ring_control *ring_control;
@@ -101,8 +101,8 @@ struct p54p_priv {
u32 rx_idx_mgmt, tx_idx_mgmt;
struct sk_buff *rx_buf_data[8];
struct sk_buff *rx_buf_mgmt[4];
- void *tx_buf_data[32];
- void *tx_buf_mgmt[4];
+ struct sk_buff *tx_buf_data[32];
+ struct sk_buff *tx_buf_mgmt[4];
struct completion boot_comp;
};
From: Christian Lamparter <chunkeey@googlemail.com>
Date: Fri, 22 Jan 2010 07:01:11 +0000 (+0100)
Subject: p54pci: revise tx locking
X-Git-Tag: master-2010-01-22~1
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Flinville%2Fwireless-next-2.6.git;a=commitdiff_plain;h=b92f7d30830a319148df2943b7565989494e5ad1
p54pci: revise tx locking
This patch continues the effort which began with:
"[PATCH] p54pci: move tx cleanup into tasklet".
Thanks to these changes, p54pci's interrupt & tx
cleanup routines can be made lock-less.
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
index 48cae48..bda29c0 100644
--- a/drivers/net/wireless/p54/p54pci.c
+++ b/drivers/net/wireless/p54/p54pci.c
@@ -238,7 +238,6 @@ static void p54p_check_tx_ring(struct ieee80211_hw *dev, u32 *index,
int ring_index, struct p54p_desc *ring, u32 ring_limit,
struct sk_buff **tx_buf)
{
- unsigned long flags;
struct p54p_priv *priv = dev->priv;
struct p54p_ring_control *ring_control = priv->ring_control;
struct p54p_desc *desc;
@@ -249,7 +248,6 @@ static void p54p_check_tx_ring(struct ieee80211_hw *dev, u32 *index,
(*index) = idx = le32_to_cpu(ring_control->device_idx[ring_index]);
idx %= ring_limit;
- spin_lock_irqsave(&priv->lock, flags);
while (i != idx) {
desc = &ring[i];
@@ -264,16 +262,12 @@ static void p54p_check_tx_ring(struct ieee80211_hw *dev, u32 *index,
desc->len = 0;
desc->flags = 0;
- if (skb && FREE_AFTER_TX(skb)) {
- spin_unlock_irqrestore(&priv->lock, flags);
+ if (skb && FREE_AFTER_TX(skb))
p54_free_skb(dev, skb);
- spin_lock_irqsave(&priv->lock, flags);
- }
i++;
i %= ring_limit;
}
- spin_unlock_irqrestore(&priv->lock, flags);
}
static void p54p_tasklet(unsigned long dev_id)
@@ -306,7 +300,6 @@ static irqreturn_t p54p_interrupt(int irq, void *dev_id)
struct p54p_priv *priv = dev->priv;
__le32 reg;
- spin_lock(&priv->lock);
reg = P54P_READ(int_ident);
if (unlikely(reg == cpu_to_le32(0xFFFFFFFF))) {
goto out;
@@ -321,15 +314,14 @@ static irqreturn_t p54p_interrupt(int irq, void *dev_id)
complete(&priv->boot_comp);
out:
- spin_unlock(&priv->lock);
return reg ? IRQ_HANDLED : IRQ_NONE;
}
static void p54p_tx(struct ieee80211_hw *dev, struct sk_buff *skb)
{
+ unsigned long flags;
struct p54p_priv *priv = dev->priv;
struct p54p_ring_control *ring_control = priv->ring_control;
- unsigned long flags;
struct p54p_desc *desc;
dma_addr_t mapping;
u32 device_idx, idx, i;
@@ -370,14 +362,14 @@ static void p54p_stop(struct ieee80211_hw *dev)
unsigned int i;
struct p54p_desc *desc;
- tasklet_kill(&priv->tasklet);
-
P54P_WRITE(int_enable, cpu_to_le32(0));
P54P_READ(int_enable);
udelay(10);
free_irq(priv->pdev->irq, dev);
+ tasklet_kill(&priv->tasklet);
+
P54P_WRITE(dev_int, cpu_to_le32(ISL38XX_DEV_INT_RESET));
for (i = 0; i < ARRAY_SIZE(priv->rx_buf_data); i++) {
From: Quintin Pitts <geek4linux@gmail.com>
Date: Fri, 9 Apr 2010 19:37:38 +0000 (+0200)
Subject: p54pci: prevent stuck rx-ring on slow system
X-Git-Tag: master-2010-04-12~9
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Flinville%2Fwireless-next-2.6.git;a=commitdiff_plain;h=5988f385b4cffa9ca72c5be0188e5f4c9ef46d82
p54pci: prevent stuck rx-ring on slow system
This patch fixes an old problem, which - under certain
circumstances - could cause the device to become
unresponsive.
most of p54pci's rx-ring management is implemented in just
two distinct standalone functions. p54p_check_rx_ring takes
care of processing incoming data, while p54p_refill_rx_ring
tries to replenish all depleted communication buffers.
This has always worked fine on my fast machine, but
now I know there is a hidden race...
The most likely candidate here is ring_control->device_idx.
Quintin Pitts had already analyzed the culprit and posted
a patch back in Oct 2009. But sadly, no one's picked up on this.
( https://patchwork.kernel.org/patch/53079/ [2 & 3] ).
This patch does the same way, except that it also prioritize
rx data processing, simply because tx routines *can* wait.
Reported-by: Sean Young <sean@mess.org>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=11386
Reported-by: Quintin Pitts <geek4linux@gmail.com>
Signed-off-by: Quintin Pitts <geek4linux@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
index ed4bdff..aa29663 100644
--- a/drivers/net/wireless/p54/p54pci.c
+++ b/drivers/net/wireless/p54/p54pci.c
@@ -131,7 +131,7 @@ static int p54p_upload_firmware(struct ieee80211_hw *dev)
static void p54p_refill_rx_ring(struct ieee80211_hw *dev,
int ring_index, struct p54p_desc *ring, u32 ring_limit,
- struct sk_buff **rx_buf)
+ struct sk_buff **rx_buf, u32 index)
{
struct p54p_priv *priv = dev->priv;
struct p54p_ring_control *ring_control = priv->ring_control;
@@ -139,7 +139,7 @@ static void p54p_refill_rx_ring(struct ieee80211_hw *dev,
idx = le32_to_cpu(ring_control->host_idx[ring_index]);
limit = idx;
- limit -= le32_to_cpu(ring_control->device_idx[ring_index]);
+ limit -= le32_to_cpu(index);
limit = ring_limit - limit;
i = idx % ring_limit;
@@ -231,7 +231,7 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
i %= ring_limit;
}
- p54p_refill_rx_ring(dev, ring_index, ring, ring_limit, rx_buf);
+ p54p_refill_rx_ring(dev, ring_index, ring, ring_limit, rx_buf, *index);
}
static void p54p_check_tx_ring(struct ieee80211_hw *dev, u32 *index,
@@ -276,14 +276,6 @@ static void p54p_tasklet(unsigned long dev_id)
struct p54p_priv *priv = dev->priv;
struct p54p_ring_control *ring_control = priv->ring_control;
- p54p_check_tx_ring(dev, &priv->tx_idx_mgmt, 3, ring_control->tx_mgmt,
- ARRAY_SIZE(ring_control->tx_mgmt),
- priv->tx_buf_mgmt);
-
- p54p_check_tx_ring(dev, &priv->tx_idx_data, 1, ring_control->tx_data,
- ARRAY_SIZE(ring_control->tx_data),
- priv->tx_buf_data);
-
p54p_check_rx_ring(dev, &priv->rx_idx_mgmt, 2, ring_control->rx_mgmt,
ARRAY_SIZE(ring_control->rx_mgmt), priv->rx_buf_mgmt);
@@ -292,6 +284,14 @@ static void p54p_tasklet(unsigned long dev_id)
wmb();
P54P_WRITE(dev_int, cpu_to_le32(ISL38XX_DEV_INT_UPDATE));
+
+ p54p_check_tx_ring(dev, &priv->tx_idx_mgmt, 3, ring_control->tx_mgmt,
+ ARRAY_SIZE(ring_control->tx_mgmt),
+ priv->tx_buf_mgmt);
+
+ p54p_check_tx_ring(dev, &priv->tx_idx_data, 1, ring_control->tx_data,
+ ARRAY_SIZE(ring_control->tx_data),
+ priv->tx_buf_data);
}
static irqreturn_t p54p_interrupt(int irq, void *dev_id)
@@ -444,10 +444,10 @@ static int p54p_open(struct ieee80211_hw *dev)
priv->rx_idx_mgmt = priv->tx_idx_mgmt = 0;
p54p_refill_rx_ring(dev, 0, priv->ring_control->rx_data,
- ARRAY_SIZE(priv->ring_control->rx_data), priv->rx_buf_data);
+ ARRAY_SIZE(priv->ring_control->rx_data), priv->rx_buf_data, 0);
p54p_refill_rx_ring(dev, 2, priv->ring_control->rx_mgmt,
- ARRAY_SIZE(priv->ring_control->rx_mgmt), priv->rx_buf_mgmt);
+ ARRAY_SIZE(priv->ring_control->rx_mgmt), priv->rx_buf_mgmt, 0);
P54P_WRITE(ring_control_base, cpu_to_le32(priv->ring_control_dma));
P54P_READ(ring_control_base);
From: Christian Lamparter <chunkeey@googlemail.com>
Date: Thu, 15 Apr 2010 12:17:07 +0000 (+0200)
Subject: p54pci: fix serious sparse warning
X-Git-Tag: master-2010-04-16~117
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Flinville%2Fwireless-next-2.6.git;a=commitdiff_plain;h=103823db62ffca028c7a214c80266519d2ea7d8d
p54pci: fix serious sparse warning
This patch fixes a bug which was just recently introduced by
("p54pci: prevent stuck rx-ring on slow system").
make M=drivers/net/wireless/p54 C=2 CF=-D__CHECK_ENDIAN__
CHECK drivers/net/wireless/p54/p54pci.c
drivers/net/wireless/p54/p54pci.c:143:11: warning: cast to restricted __le32
CC [M] drivers/net/wireless/p54/p54pci.o
Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
index aa29663..0a516c8 100644
--- a/drivers/net/wireless/p54/p54pci.c
+++ b/drivers/net/wireless/p54/p54pci.c
@@ -139,7 +139,7 @@ static void p54p_refill_rx_ring(struct ieee80211_hw *dev,
idx = le32_to_cpu(ring_control->host_idx[ring_index]);
limit = idx;
- limit -= le32_to_cpu(index);
+ limit -= index;
limit = ring_limit - limit;
i = idx % ring_limit;
Subject:
[PATCH 2/2] p54pci: fix regression from prevent stuck rx-ring on slow system
From:
Christian Lamparter <chunkeey@googlemail.com>
Date:
Thu, 22 Apr 2010 19:52:43 +0200
To:
linux-wireless@vger.kernel.org
CC:
linville@tuxdriver.com, hdegoede@redhat.com
From: Hans de Goede <hdegoede@redhat.com>
This patch fixes a recently introduced use-after-free regression
from "p54pci: prevent stuck rx-ring on slow system".
Hans de Goede reported a use-after-free regression:
> >BUG: unable to handle kernel paging request at 6b6b6b6b
> >IP: [<e122284a>] p54p_check_tx_ring+0x84/0xb1 [p54pci]
> >*pde = 00000000
> >Oops: 0000 [#1] SMP
> >EIP: 0060:[<e122284a>] EFLAGS: 00010286 CPU: 0
> >EIP is at p54p_check_tx_ring+0x84/0xb1 [p54pci]
> >EAX: 6b6b6b6b EBX: df10b170 ECX: 00000003 EDX: 00000001
> >ESI: dc471500 EDI: d8acaeb0 EBP: c098be9c ESP: c098be84
> > DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> >Process swapper (pid: 0, ti=c098a000 task=c09ccfe0 task.ti=c098a000)
> >Call Trace:
> > [<e1222b02>] ? p54p_tasklet+0xaa/0xb5 [p54pci]
> > [<c0440568>] ? tasklet_action+0x78/0xcb
> > [<c0440ed3>] ? __do_softirq+0xbc/0x173
Quote from comment #17:
"The problem is the innocent looking moving of the tx processing to
after the rx processing in the tasklet. Quoting from the changelog:
This patch does it the same way, except that it also prioritize
rx data processing, simply because tx routines *can* wait.
This is causing an issue with us referencing already freed memory,
because some skb's we transmit, we immediately receive back, such
as those for reading the eeprom (*) and getting stats.
What can happen because of the moving of the tx processing to after
the rx processing is that when the tasklet first runs after doing a
special skb tx (such as eeprom) we've already received the answer
to it.
Then the rx processing ends up calling p54_find_and_unlink_skb to
find the matching tx skb for the just received special rx skb and
frees the tx skb.
Then after the processing of the rx skb answer, and thus freeing
the tx skb, we go process the completed tx ring entires, and then
dereference the free-ed skb, to see if it should free free-ed by
p54p_check_tx_ring()."
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=583623
Bug-Identified-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
index ca42ccb..07c4528 100644
--- a/drivers/net/wireless/p54/p54pci.c
+++ b/drivers/net/wireless/p54/p54pci.c
@@ -277,6 +277,14 @@ static void p54p_tasklet(unsigned long dev_id)
struct p54p_priv *priv = dev->priv;
struct p54p_ring_control *ring_control = priv->ring_control;
+ p54p_check_tx_ring(dev, &priv->tx_idx_mgmt, 3, ring_control->tx_mgmt,
+ ARRAY_SIZE(ring_control->tx_mgmt),
+ priv->tx_buf_mgmt);
+
+ p54p_check_tx_ring(dev, &priv->tx_idx_data, 1, ring_control->tx_data,
+ ARRAY_SIZE(ring_control->tx_data),
+ priv->tx_buf_data);
+
p54p_check_rx_ring(dev, &priv->rx_idx_mgmt, 2, ring_control->rx_mgmt,
ARRAY_SIZE(ring_control->rx_mgmt), priv->rx_buf_mgmt);
@@ -285,14 +293,6 @@ static void p54p_tasklet(unsigned long dev_id)
wmb();
P54P_WRITE(dev_int, cpu_to_le32(ISL38XX_DEV_INT_UPDATE));
-
- p54p_check_tx_ring(dev, &priv->tx_idx_mgmt, 3, ring_control->tx_mgmt,
- ARRAY_SIZE(ring_control->tx_mgmt),
- priv->tx_buf_mgmt);
-
- p54p_check_tx_ring(dev, &priv->tx_idx_data, 1, ring_control->tx_data,
- ARRAY_SIZE(ring_control->tx_data),
- priv->tx_buf_data);
}
static irqreturn_t p54p_interrupt(int irq, void *dev_id)