From: Christian Lamparter <chunkeey@web.de>
To: "linux-wireless" <linux-wireless@vger.kernel.org>
Cc: Larry Finger <Larry.Finger@lwfinger.net>,
Max Filippov <jcmvbkbc@gmail.com>
Subject: [WIP] p54: deal with allocation failures in rx path
Date: Sat, 4 Jul 2009 00:53:05 +0200 [thread overview]
Message-ID: <200907040053.05654.chunkeey@web.de> (raw)
This patch tries to address a long standing issue:
how to survive serve memory starvation situations,
without losing the device due to missing transfer-buffers.
And with a flick of __GFP_NOWARN, we're able to handle ?all? memory
allocation failures on the rx-side during operation without much fuss.
However, there is still an issue within the xmit-part.
This is likely due to p54's demand for a large free headroom for
every outgoing frame:
+ transport header (differs from device to device)
-> 16 bytes transport header (USB 1st gen)
-> 8 bytes for (USB 2nd gen)
-> 0 bytes for spi & pci
+ 12 bytes for p54_hdr
+ 44 bytes for p54_tx_data
+ up to 3 bytes for alignment
(+ 802.11 header as well? )
and this is where ieee80211_skb_resize comes into the play...
which will try to _relocate_ (alloc new, copy, free old) frame data,
as the headroom is most of the time simply not enough.
=>
Call Trace: (from Larry - Bug #13319 )
[<ffffffff80292a7b>] __alloc_pages_internal+0x43d/0x45e
[<ffffffff802b1f1f>] alloc_pages_current+0xbe/0xc6
[<ffffffff802b6362>] new_slab+0xcf/0x28b
[<ffffffff802b4d1f>] ? unfreeze_slab+0x4c/0xbd
[<ffffffff802b672e>] __slab_alloc+0x210/0x44c
[<ffffffff803e7bee>] ? pskb_expand_head+0x52/0x166
[<ffffffff803e7bee>] ? pskb_expand_head+0x52/0x166
[<ffffffff802b7e60>] __kmalloc+0x119/0x194
[<ffffffff803e7bee>] pskb_expand_head+0x52/0x166
[<ffffffffa02913d6>] ieee80211_skb_resize+0x91/0xc7 [mac80211]
[<ffffffffa0291c0f>] ieee80211_master_start_xmit+0x298/0x319 [mac80211]
[<ffffffff803ef72a>] dev_hard_start_xmit+0x229/0x2a8
(sl*b debug option will help to bloat even more.)
So?! how to prevent ieee80211_skb_resize from raping
the bits of memory left?
the simplest answer is probably this one:
https://dev.openwrt.org/changeset/15761
--
back to rx failures.
the attached code below was only usb was tested so far!
you have been warned!
regards,
chr
btw: max what do you think about the p54spi changes, are they total ****?
---
diff --git a/drivers/net/wireless/p54/fwio.c b/drivers/net/wireless/p54/fwio.c
index 349375f..c9bc1da 100644
--- a/drivers/net/wireless/p54/fwio.c
+++ b/drivers/net/wireless/p54/fwio.c
@@ -94,7 +94,7 @@ int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw)
else
priv->rx_mtu = (size_t)
0x620 - priv->tx_hdr_len;
- maxlen = priv->tx_hdr_len + /* USB devices */
+ maxlen = priv->rx_hdr_len + /* USB devices */
sizeof(struct p54_rx_data) +
4 + /* rx alignment */
IEEE80211_MAX_FRAG_THRESHOLD;
@@ -103,6 +103,18 @@ int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw)
"to %d\n", priv->rx_mtu, maxlen);
priv->rx_mtu = maxlen;
}
+
+ /*
+ * Firmware may insert up to 4 padding bytes after
+ * the lmac header, but it doesn't amend the size
+ * of data transfer.
+ * Such packets has correct data size in header, thus
+ * referencing past the end of allocated skb.
+ * Therefore reserve 4 extra bytes for this case.
+ */
+ if (priv->rx_wa_extra_tail_space)
+ priv->rx_mtu += 4;
+
break;
}
case BR_CODE_EXPOSED_IF:
@@ -312,7 +324,7 @@ int p54_setup_mac(struct p54_common *priv)
{
struct sk_buff *skb;
struct p54_setup_mac *setup;
- u16 mode;
+ u16 mode, rx_mtu = priv->rx_mtu;
skb = p54_alloc_skb(priv, P54_HDR_FLAG_CONTROL_OPSET, sizeof(*setup),
P54_CONTROL_TYPE_SETUP, GFP_ATOMIC);
@@ -356,17 +368,25 @@ int p54_setup_mac(struct p54_common *priv)
memcpy(setup->bssid, priv->bssid, ETH_ALEN);
setup->rx_antenna = 2 & priv->rx_diversity_mask; /* automatic */
setup->rx_align = 0;
+
+ /*
+ * reserve additional bytes to compensate for the possible
+ * alignment-caused truncation.
+ */
+ if (priv->rx_wa_extra_tail_space)
+ rx_mtu -= 4;
+
if (priv->fw_var < 0x500) {
setup->v1.basic_rate_mask = cpu_to_le32(priv->basic_rate_mask);
memset(setup->v1.rts_rates, 0, 8);
setup->v1.rx_addr = cpu_to_le32(priv->rx_end);
- setup->v1.max_rx = cpu_to_le16(priv->rx_mtu);
+ setup->v1.max_rx = cpu_to_le16(rx_mtu);
setup->v1.rxhw = cpu_to_le16(priv->rxhw);
setup->v1.wakeup_timer = cpu_to_le16(priv->wakeup_timer);
setup->v1.unalloc0 = cpu_to_le16(0);
} else {
setup->v2.rx_addr = cpu_to_le32(priv->rx_end);
- setup->v2.max_rx = cpu_to_le16(priv->rx_mtu);
+ setup->v2.max_rx = cpu_to_le16(rx_mtu);
setup->v2.rxhw = cpu_to_le16(priv->rxhw);
setup->v2.timer = cpu_to_le16(priv->wakeup_timer);
setup->v2.truncate = cpu_to_le16(48896);
diff --git a/drivers/net/wireless/p54/p54.h b/drivers/net/wireless/p54/p54.h
index 6772ed5..30ebde1 100644
--- a/drivers/net/wireless/p54/p54.h
+++ b/drivers/net/wireless/p54/p54.h
@@ -176,6 +176,8 @@ struct p54_common {
/* firmware/hardware info */
unsigned int tx_hdr_len;
+ unsigned int rx_hdr_len;
+ bool rx_wa_extra_tail_space;
unsigned int fw_var;
unsigned int fw_interface;
u8 version;
@@ -234,7 +236,7 @@ struct p54_common {
};
/* interfaces for the drivers */
-int p54_rx(struct ieee80211_hw *dev, struct sk_buff *skb);
+struct sk_buff *p54_rx(struct ieee80211_hw *dev, struct sk_buff *skb);
void p54_free_skb(struct ieee80211_hw *dev, struct sk_buff *skb);
int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw);
int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len);
@@ -245,5 +247,6 @@ int p54_register_common(struct ieee80211_hw *dev, struct device *pdev);
void p54_free_common(struct ieee80211_hw *dev);
void p54_unregister_common(struct ieee80211_hw *dev);
+struct sk_buff *p54_alloc_rxskb(struct p54_common *priv, gfp_t gfp_mask);
#endif /* P54_H */
diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
index d348c26..7d055da 100644
--- a/drivers/net/wireless/p54/p54pci.c
+++ b/drivers/net/wireless/p54/p54pci.c
@@ -149,7 +149,7 @@ static void p54p_refill_rx_ring(struct ieee80211_hw *dev,
if (!desc->host_addr) {
struct sk_buff *skb;
dma_addr_t mapping;
- skb = dev_alloc_skb(priv->common.rx_mtu + 32);
+ skb = dev_alloc_skb(priv->common.rx_mtu);
if (!skb)
break;
@@ -186,6 +186,7 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
(*index) = idx = le32_to_cpu(ring_control->device_idx[ring_index]);
idx %= ring_limit;
while (i != idx) {
+ dma_addr_t mapping;
u16 len;
struct sk_buff *skb;
desc = &ring[i];
@@ -197,25 +198,32 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
i %= ring_limit;
continue;
}
+
+ pci_unmap_single(priv->pdev,
+ le32_to_cpu(desc->host_addr),
+ priv->common.rx_mtu + 32,
+ PCI_DMA_FROMDEVICE);
+ rx_buf[i] = NULL;
+ desc->host_addr = 0;
skb_put(skb, len);
- if (p54_rx(dev, skb)) {
- pci_unmap_single(priv->pdev,
- le32_to_cpu(desc->host_addr),
- priv->common.rx_mtu + 32,
+ skb = p54_rx(dev, skb);
+ mapping = pci_map_single(priv->pdev,
+ skb_tail_pointer(skb),
+ skb->len,
PCI_DMA_FROMDEVICE);
- rx_buf[i] = NULL;
- desc->host_addr = 0;
- } else {
- skb_trim(skb, 0);
- desc->len = cpu_to_le16(priv->common.rx_mtu + 32);
- }
+
+ desc->host_addr = cpu_to_le32(mapping);
+ desc->device_addr = 0;
+ desc->len = cpu_to_le16(priv->common.rx_mtu + 32);
+ desc->flags = 0;
+ rx_buf[i] = skb;
i++;
i %= ring_limit;
}
-
- p54p_refill_rx_ring(dev, ring_index, ring, ring_limit, rx_buf);
+ wmb();
+ ring_control->host_idx[ring_index] = cpu_to_le32(idx);
}
/* caller must hold priv->lock */
diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
index 9b347ce..4c6ac3f 100644
--- a/drivers/net/wireless/p54/p54spi.c
+++ b/drivers/net/wireless/p54/p54spi.c
@@ -389,14 +389,10 @@ static int p54spi_rx(struct p54s_priv *priv)
return 0;
}
- /* Firmware may insert up to 4 padding bytes after the lmac header,
- * but it does not amend the size of SPI data transfer.
- * Such packets has correct data size in header, thus referencing
- * past the end of allocated skb. Reserve extra 4 bytes for this case */
- skb = dev_alloc_skb(len + 4);
+ skb = skb_dequeue(&priv->rx_pool);
if (!skb) {
p54spi_sleep(priv);
- dev_err(&priv->spi->dev, "could not alloc skb");
+ dev_err(&priv->spi->dev, "could not get skb");
return -ENOMEM;
}
@@ -409,12 +405,9 @@ static int p54spi_rx(struct p54s_priv *priv)
len - READAHEAD_SZ);
}
p54spi_sleep(priv);
- /* Put additional bytes to compensate for the possible
- * alignment-caused truncation */
- skb_put(skb, 4);
- if (p54_rx(priv->hw, skb) == 0)
- dev_kfree_skb(skb);
+ skb = p54_rx(priv->hw, skb);
+ skb_queue_tail(&priv->rx_pool, skb);
return 0;
}
@@ -629,6 +622,7 @@ static void p54spi_op_stop(struct ieee80211_hw *dev)
static int __devinit p54spi_probe(struct spi_device *spi)
{
struct p54s_priv *priv = NULL;
+ struct sk_buff *skb;
struct ieee80211_hw *hw;
int ret = -EINVAL;
@@ -645,6 +639,8 @@ static int __devinit p54spi_probe(struct spi_device *spi)
spi->bits_per_word = 16;
spi->max_speed_hz = 24000000;
+ skb_queue_head_init(&priv->rx_pool);
+ priv->common.rx_wa_extra_tail_space = true;
ret = spi_setup(spi);
if (ret < 0) {
@@ -689,6 +685,11 @@ static int __devinit p54spi_probe(struct spi_device *spi)
priv->common.stop = p54spi_op_stop;
priv->common.tx = p54spi_op_tx;
+ skb = __dev_alloc_skb(priv->common.rx_mtu, GFP_KERNEL);
+ if (!skb)
+ goto err_free_common;
+ skb_queue_tail(&priv->rx_pool, skb);
+
ret = p54spi_request_firmware(hw);
if (ret < 0)
goto err_free_common;
@@ -705,6 +706,7 @@ static int __devinit p54spi_probe(struct spi_device *spi)
err_free_common:
p54_free_common(priv->hw);
+ skb_queue_purge(&priv->rx_pool);
return ret;
}
@@ -715,6 +717,7 @@ static int __devexit p54spi_remove(struct spi_device *spi)
p54_unregister_common(priv->hw);
free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
+ skb_queue_purge(&priv->rx_pool);
gpio_free(p54spi_gpio_power);
gpio_free(p54spi_gpio_irq);
diff --git a/drivers/net/wireless/p54/p54spi.h b/drivers/net/wireless/p54/p54spi.h
index 7fbe8d8..11ef2d5 100644
--- a/drivers/net/wireless/p54/p54spi.h
+++ b/drivers/net/wireless/p54/p54spi.h
@@ -120,6 +120,8 @@ struct p54s_priv {
enum fw_state fw_state;
const struct firmware *firmware;
+
+ struct sk_buff_head rx_pool;
};
#endif /* P54SPI_H */
diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index 461d88f..c521bbc 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -120,37 +120,14 @@ static void p54u_rx_cb(struct urb *urb)
}
skb_put(skb, urb->actual_length);
+ skb = p54_rx(dev, skb);
- if (priv->hw_type == P54U_NET2280)
- skb_pull(skb, priv->common.tx_hdr_len);
- if (priv->common.fw_interface == FW_LM87) {
- skb_pull(skb, 4);
- skb_put(skb, 4);
- }
-
- if (p54_rx(dev, skb)) {
- skb = dev_alloc_skb(priv->common.rx_mtu + 32);
- if (unlikely(!skb)) {
- /* TODO check rx queue length and refill *somewhere* */
- return;
- }
+ info = (struct p54u_rx_info *) skb->cb;
+ info->urb = urb;
+ info->dev = dev;
+ urb->transfer_buffer = skb_tail_pointer(skb);
+ urb->context = skb;
- info = (struct p54u_rx_info *) skb->cb;
- info->urb = urb;
- info->dev = dev;
- urb->transfer_buffer = skb_tail_pointer(skb);
- urb->context = skb;
- } else {
- if (priv->hw_type == P54U_NET2280)
- skb_push(skb, priv->common.tx_hdr_len);
- if (priv->common.fw_interface == FW_LM87) {
- skb_push(skb, 4);
- skb_put(skb, 4);
- }
- skb_reset_tail_pointer(skb);
- skb_trim(skb, 0);
- urb->transfer_buffer = skb_tail_pointer(skb);
- }
skb_queue_tail(&priv->rx_queue, skb);
usb_anchor_urb(urb, &priv->submitted);
if (usb_submit_urb(urb, GFP_ATOMIC)) {
@@ -186,7 +163,7 @@ static int p54u_init_urbs(struct ieee80211_hw *dev)
int ret = 0;
while (skb_queue_len(&priv->rx_queue) < 32) {
- skb = __dev_alloc_skb(priv->common.rx_mtu + 32, GFP_KERNEL);
+ skb = __dev_alloc_skb(priv->common.rx_mtu, GFP_KERNEL);
if (!skb) {
ret = -ENOMEM;
goto err;
@@ -927,14 +904,17 @@ static int __devinit p54u_probe(struct usb_interface *intf,
#endif
priv->hw_type = P54U_3887;
+ priv->common.rx_wa_extra_tail_space = true;
dev->extra_tx_headroom += sizeof(struct lm87_tx_hdr);
priv->common.tx_hdr_len = sizeof(struct lm87_tx_hdr);
+ priv->common.rx_hdr_len = sizeof(struct lm87_rx_hdr);
priv->common.tx = p54u_tx_lm87;
priv->upload_fw = p54u_upload_firmware_3887;
} else {
priv->hw_type = P54U_NET2280;
dev->extra_tx_headroom += sizeof(struct net2280_tx_hdr);
priv->common.tx_hdr_len = sizeof(struct net2280_tx_hdr);
+ priv->common.rx_hdr_len = sizeof(struct net2280_tx_hdr);
priv->common.tx = p54u_tx_net2280;
priv->upload_fw = p54u_upload_firmware_net2280;
}
diff --git a/drivers/net/wireless/p54/p54usb.h b/drivers/net/wireless/p54/p54usb.h
index e935b79..685a2b4 100644
--- a/drivers/net/wireless/p54/p54usb.h
+++ b/drivers/net/wireless/p54/p54usb.h
@@ -77,6 +77,10 @@ struct lm87_tx_hdr {
__le32 chksum;
} __attribute__((packed));
+struct lm87_rx_hdr {
+ __le32 chksum;
+} __packed;
+
/* Some flags for the isl hardware registers controlling DMA inside the
* chip */
#define ISL38XX_DMA_STATUS_DONE 0x00000001
diff --git a/drivers/net/wireless/p54/txrx.c b/drivers/net/wireless/p54/txrx.c
index ea074a6..45e5e88 100644
--- a/drivers/net/wireless/p54/txrx.c
+++ b/drivers/net/wireless/p54/txrx.c
@@ -324,10 +324,11 @@ static void p54_pspoll_workaround(struct p54_common *priv, struct sk_buff *skb)
}
}
-static int p54_rx_data(struct p54_common *priv, struct sk_buff *skb)
+static struct sk_buff *p54_rx_data(struct p54_common *priv, struct sk_buff *skb)
{
struct p54_rx_data *hdr = (struct p54_rx_data *) skb->data;
struct ieee80211_rx_status *rx_status = IEEE80211_SKB_RXCB(skb);
+ struct sk_buff *tmp;
u16 freq = le16_to_cpu(hdr->freq);
size_t header_len = sizeof(*hdr);
u32 tsf32;
@@ -339,10 +340,14 @@ static int p54_rx_data(struct p54_common *priv, struct sk_buff *skb)
* nasty crash.
*/
if (unlikely(priv->mode == NL80211_IFTYPE_UNSPECIFIED))
- return 0;
+ return skb;
if (!(hdr->flags & cpu_to_le16(P54_HDR_FLAG_DATA_IN_FCS_GOOD)))
- return 0;
+ return skb;
+
+ tmp = dev_alloc_skb(priv->rx_mtu);
+ if (!tmp)
+ return skb;
if (hdr->decrypt_status == P54_DECRYPT_OK)
rx_status->flag |= RX_FLAG_DECRYPTED;
@@ -384,7 +389,7 @@ static int p54_rx_data(struct p54_common *priv, struct sk_buff *skb)
queue_delayed_work(priv->hw->workqueue, &priv->work,
msecs_to_jiffies(P54_STATISTICS_UPDATE));
- return -1;
+ return tmp;
}
static void p54_rx_frame_sent(struct p54_common *priv, struct sk_buff *skb)
@@ -566,7 +571,7 @@ static void p54_rx_trap(struct p54_common *priv, struct sk_buff *skb)
}
}
-static int p54_rx_control(struct p54_common *priv, struct sk_buff *skb)
+static struct sk_buff *p54_rx_control(struct p54_common *priv, struct sk_buff *skb)
{
struct p54_hdr *hdr = (struct p54_hdr *) skb->data;
@@ -590,19 +595,52 @@ static int p54_rx_control(struct p54_common *priv, struct sk_buff *skb)
wiphy_name(priv->hw->wiphy), le16_to_cpu(hdr->type));
break;
}
- return 0;
+ return skb;
}
-/* returns zero if skb can be reused */
-int p54_rx(struct ieee80211_hw *dev, struct sk_buff *skb)
+/* returns a new skb in case the old one was send to mac80211. */
+struct sk_buff *p54_rx(struct ieee80211_hw *dev, struct sk_buff *skb)
{
struct p54_common *priv = dev->priv;
- u16 type = le16_to_cpu(*((__le16 *)skb->data));
+ struct sk_buff *replacement;
+ struct p54_hdr *hdr;
+
+ if (unlikely(skb->len < sizeof(*hdr) + priv->rx_hdr_len)) {
+ if (net_ratelimit())
+ printk(KERN_ERR "%s: short read! - if this message "
+ "persists => check the device or cable.\n",
+ wiphy_name(dev->wiphy));
+
+ return skb;
+ }
+
+ if (priv->rx_wa_extra_tail_space) {
+ /*
+ * Put additional bytes to compensate for the possible
+ * alignment-caused truncation.
+ */
+
+ skb_put(skb, 4);
+ }
+
+ skb_pull(skb, priv->rx_hdr_len);
+ hdr = (void *) skb->data;
- if (type & P54_HDR_FLAG_CONTROL)
- return p54_rx_control(priv, skb);
+ if (hdr->flags & cpu_to_le16(P54_HDR_FLAG_CONTROL))
+ replacement = p54_rx_control(priv, skb);
else
- return p54_rx_data(priv, skb);
+ replacement = p54_rx_data(priv, skb);
+
+ if (replacement == skb) {
+ /*
+ * This skb was not _consumed_ by mac80211.
+ * Reinitialize dirty data structure before returning it back.
+ */
+
+ skb_put(skb, priv->rx_hdr_len);
+ skb_trim(skb, 0);
+ }
+ return replacement;
}
EXPORT_SYMBOL_GPL(p54_rx);
next reply other threads:[~2009-07-03 22:53 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-03 22:53 Christian Lamparter [this message]
2009-07-04 1:09 ` [WIP] p54: deal with allocation failures in rx path Larry Finger
2009-07-04 2:16 ` Larry Finger
2009-07-04 10:11 ` Christian Lamparter
2009-07-04 16:40 ` Larry Finger
2009-07-04 17:28 ` Christian Lamparter
2009-07-04 19:56 ` Larry Finger
2009-07-04 21:14 ` Larry Finger
2009-07-05 13:59 ` Christian Lamparter
2009-07-05 17:49 ` Larry Finger
2009-07-05 22:05 ` Christian Lamparter
2009-07-06 1:36 ` Larry Finger
2009-07-06 13:16 ` Christian Lamparter
2009-07-04 7:52 ` Johannes Berg
2009-07-05 0:56 ` Max Filippov
2009-07-05 14:00 ` Christian Lamparter
2009-07-05 19:16 ` Max Filippov
2009-07-05 22:46 ` Max Filippov
2009-07-06 13:11 ` Max Filippov
2009-07-06 14:00 ` Christian Lamparter
2009-07-06 14:18 ` Max Filippov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200907040053.05654.chunkeey@web.de \
--to=chunkeey@web.de \
--cc=Larry.Finger@lwfinger.net \
--cc=jcmvbkbc@gmail.com \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).