* [RFC][PATCH] p54: fix memory management
@ 2008-10-02 1:59 Christian Lamparter
2008-10-03 2:43 ` Larry Finger
0 siblings, 1 reply; 5+ messages in thread
From: Christian Lamparter @ 2008-10-02 1:59 UTC (permalink / raw)
To: linux-wireless; +Cc: John W Linville, Larry Finger, Johannes Berg, Kalle Valo
Well, since a lot of people have worked with the memory management of p54 driver
(or similar projects ;-) ), it's probably the best to give this _urgent_ thing a RFC round.
The problem is that if multiple "control frames" are passed in a very short intervals to
the device's firmware (e.g: QoS and frequency are the best candidates)
the data might get corrupted. As p54_assign_address always put them into same
memory location and the device's firmware is too slow to pick the frames up,
before they're overwritten again.
P.S: the code inside the #if 0 - #endif will go away in the final version,
but for now its very useful for debugging. So don't complain about it ;-).
Regards,
Chr
---
diff -Nurp a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
--- a/drivers/net/wireless/p54/p54common.c 2008-09-29 20:41:44.000000000 +0200
+++ b/drivers/net/wireless/p54/p54common.c 2008-10-02 03:17:37.000000000 +0200
@@ -537,11 +537,106 @@ static void inline p54_wake_free_queues(
struct p54_common *priv = dev->priv;
int i;
+ if (priv->mode == NL80211_IFTYPE_UNSPECIFIED)
+ return ;
+
for (i = 0; i < dev->queues; i++)
if (priv->tx_stats[i + 4].len < priv->tx_stats[i + 4].limit)
ieee80211_wake_queue(dev, i);
}
+#if 0
+static void p54_dump_txqueue(struct ieee80211_hw *dev)
+{
+ struct p54_common *priv = dev->priv;
+ struct sk_buff *skb = priv->tx_queue.next;
+ struct ieee80211_tx_info *info;
+ struct memrecord *range;
+ unsigned long flags;
+ u32 last_addr = priv->rx_start, free = 0, total_free = 0;
+ u32 queue_len, biggest_free_hole = 0;
+
+ spin_lock_irqsave(&priv->tx_queue.lock, flags);
+
+ queue_len = skb_queue_len(&priv->tx_queue);
+
+ printk(KERN_DEBUG "%s: Tx queue entries: %d\n",
+ wiphy_name(dev->wiphy), queue_len);
+
+ while (queue_len--) {
+ info = IEEE80211_SKB_CB(skb);
+ range = (void *)info->driver_data;
+ free = range->start_addr - last_addr;
+ if (free > biggest_free_hole)
+ biggest_free_hole = free;
+ total_free += free;
+ printk(KERN_DEBUG "\tentry: %p (range: [%x-%x]=(%x) free:%x)\n",
+ skb, range->start_addr, range->end_addr,
+ range->end_addr - range->start_addr, free);
+ last_addr = range->end_addr;
+ skb = skb->next;
+ }
+ spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
+
+ free = priv->rx_end - last_addr;
+ if (free > biggest_free_hole)
+ biggest_free_hole = free;
+
+ total_free += free;
+ printk(KERN_DEBUG "\tlast entry: (range: [%x-%x] free: %d)\n",
+ last_addr, priv->rx_end, free);
+ printk(KERN_DEBUG "\ttotal_free:%d bytes, biggest free hole:%d bytes\n",
+ total_free, biggest_free_hole);
+}
+#endif
+
+void p54_free_skb(struct ieee80211_hw *dev, struct sk_buff *skb)
+{
+ struct p54_common *priv = dev->priv;
+ struct ieee80211_tx_info *info;
+ struct memrecord *range;
+ unsigned long flags;
+ u32 freed = 0, last_addr = priv->rx_start;
+
+ if (!skb || !dev)
+ return;
+
+ spin_lock_irqsave(&priv->tx_queue.lock, flags);
+
+ info = IEEE80211_SKB_CB(skb);
+ range = (void *)info->driver_data;
+
+ if (skb->prev != (struct sk_buff *)&priv->tx_queue) {
+ struct ieee80211_tx_info *ni;
+ struct memrecord *mr;
+
+ ni = IEEE80211_SKB_CB(skb->prev);
+ mr = (struct memrecord *)ni->driver_data;
+ last_addr = mr->end_addr;
+ }
+
+ if (skb->next != (struct sk_buff *)&priv->tx_queue) {
+ struct ieee80211_tx_info *ni;
+ struct memrecord *mr;
+
+ ni = IEEE80211_SKB_CB(skb->next);
+ mr = (struct memrecord *)ni->driver_data;
+ freed = mr->start_addr - last_addr;
+ } else
+ freed = priv->rx_end - last_addr;
+
+ __skb_unlink(skb, &priv->tx_queue);
+
+ spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
+
+ kfree_skb(skb);
+
+ if (freed >= IEEE80211_MAX_RTS_THRESHOLD + 0x170 +
+ sizeof(struct p54_control_hdr))
+ p54_wake_free_queues(dev);
+}
+EXPORT_SYMBOL(p54_free_skb);
+
static void p54_rx_frame_sent(struct ieee80211_hw *dev, struct sk_buff *skb)
{
struct p54_common *priv = dev->priv;
@@ -703,6 +798,8 @@ static void p54_assign_address(struct ie
unsigned int left;
len = (len + priv->headroom + priv->tailroom + 3) & ~0x3;
+ WARN_ON(!skb);
+
spin_lock_irqsave(&priv->tx_queue.lock, flags);
left = skb_queue_len(&priv->tx_queue);
while (left--) {
@@ -735,6 +832,7 @@ static void p54_assign_address(struct ie
struct memrecord *range = (void *)info->driver_data;
range->start_addr = target_addr;
range->end_addr = target_addr + len;
+ range->dev = dev;
__skb_queue_after(&priv->tx_queue, target_skb, skb);
if (largest_hole < priv->rx_mtu + priv->headroom +
priv->tailroom +
@@ -751,15 +849,18 @@ int p54_read_eeprom(struct ieee80211_hw
struct p54_common *priv = dev->priv;
struct p54_control_hdr *hdr = NULL;
struct p54_eeprom_lm86 *eeprom_hdr;
+ struct sk_buff *skb;
size_t eeprom_size = 0x2020, offset = 0, blocksize;
int ret = -ENOMEM;
void *eeprom = NULL;
- hdr = (struct p54_control_hdr *)kzalloc(sizeof(*hdr) +
- sizeof(*eeprom_hdr) + EEPROM_READBACK_LEN, GFP_KERNEL);
- if (!hdr)
- goto free;
+ skb = dev_alloc_skb(sizeof(*hdr) + sizeof(*eeprom_hdr) +
+ EEPROM_READBACK_LEN + priv->tx_hdr_len);
+ if (!skb)
+ goto free;
+ skb_reserve(skb, priv->tx_hdr_len);
+ hdr = (struct p54_control_hdr *) skb_put(skb, sizeof(*hdr));
priv->eeprom = kzalloc(EEPROM_READBACK_LEN, GFP_KERNEL);
if (!priv->eeprom)
goto free;
@@ -771,16 +872,18 @@ int p54_read_eeprom(struct ieee80211_hw
hdr->magic1 = cpu_to_le16(0x8000);
hdr->type = cpu_to_le16(P54_CONTROL_TYPE_EEPROM_READBACK);
hdr->retry1 = hdr->retry2 = 0;
- eeprom_hdr = (struct p54_eeprom_lm86 *) hdr->data;
+ eeprom_hdr = (struct p54_eeprom_lm86 *) skb_put(skb,
+ sizeof(*eeprom_hdr) + EEPROM_READBACK_LEN);
+
+ p54_assign_address(dev, skb, hdr, skb->len);
while (eeprom_size) {
blocksize = min(eeprom_size, (size_t)EEPROM_READBACK_LEN);
hdr->len = cpu_to_le16(blocksize + sizeof(*eeprom_hdr));
eeprom_hdr->offset = cpu_to_le16(offset);
eeprom_hdr->len = cpu_to_le16(blocksize);
- p54_assign_address(dev, NULL, hdr, le16_to_cpu(hdr->len) +
- sizeof(*hdr));
- priv->tx(dev, hdr, le16_to_cpu(hdr->len) + sizeof(*hdr), 0);
+ priv->tx(dev, hdr, skb, le16_to_cpu(hdr->len) +
+ sizeof(*hdr), 0);
if (!wait_for_completion_interruptible_timeout(&priv->eeprom_comp, HZ)) {
printk(KERN_ERR "%s: device does not respond!\n",
@@ -798,7 +901,7 @@ int p54_read_eeprom(struct ieee80211_hw
free:
kfree(priv->eeprom);
priv->eeprom = NULL;
- kfree(hdr);
+ p54_free_skb(dev, skb);
kfree(eeprom);
return ret;
@@ -881,7 +984,7 @@ static int p54_tx(struct ieee80211_hw *d
/* modifies skb->cb and with it info, so must be last! */
p54_assign_address(dev, skb, hdr, skb->len);
- priv->tx(dev, hdr, skb->len, 0);
+ priv->tx(dev, hdr, skb, skb->len, 0);
return 0;
}
@@ -890,21 +993,23 @@ static int p54_set_filter(struct ieee802
{
struct p54_common *priv = dev->priv;
struct p54_control_hdr *hdr;
+ struct sk_buff *skb;
struct p54_tx_control_filter *filter;
size_t data_len;
- hdr = kzalloc(sizeof(*hdr) + sizeof(*filter) +
+ skb = __dev_alloc_skb(sizeof(*hdr) + sizeof(*filter) +
priv->tx_hdr_len, GFP_ATOMIC);
- if (!hdr)
+ if (!skb)
return -ENOMEM;
- hdr = (void *)hdr + priv->tx_hdr_len;
-
- filter = (struct p54_tx_control_filter *) hdr->data;
+ skb_reserve(skb, priv->tx_hdr_len);
+ hdr = (struct p54_control_hdr *) skb_put(skb, sizeof(*hdr));
hdr->magic1 = cpu_to_le16(0x8001);
hdr->type = cpu_to_le16(P54_CONTROL_TYPE_FILTER_SET);
+ hdr->retry1 = hdr->retry2 = 0;
- priv->filter_type = filter->filter_type = cpu_to_le16(filter_type);
+ filter = (struct p54_tx_control_filter *) skb_put(skb, sizeof(*filter));
+ filter->filter_type = priv->filter_type = cpu_to_le16(filter_type);
memcpy(filter->mac_addr, priv->mac_addr, ETH_ALEN);
if (!bssid)
memset(filter->bssid, ~0, ETH_ALEN);
@@ -929,33 +1034,34 @@ static int p54_set_filter(struct ieee802
}
hdr->len = cpu_to_le16(data_len);
- p54_assign_address(dev, NULL, hdr, sizeof(*hdr) + data_len);
- priv->tx(dev, hdr, sizeof(*hdr) + data_len, 1);
+ p54_assign_address(dev, skb, hdr, sizeof(*hdr) + data_len);
+
+ priv->tx(dev, hdr, skb, sizeof(*hdr) + data_len, 1);
return 0;
}
static int p54_set_freq(struct ieee80211_hw *dev, __le16 freq)
{
struct p54_common *priv = dev->priv;
+ struct sk_buff *skb;
struct p54_control_hdr *hdr;
struct p54_tx_control_channel *chan;
unsigned int i;
size_t data_len;
void *entry;
- hdr = kzalloc(sizeof(*hdr) + sizeof(*chan) +
- priv->tx_hdr_len, GFP_KERNEL);
- if (!hdr)
- return -ENOMEM;
+ skb = dev_alloc_skb(priv->tx_hdr_len + sizeof(*hdr) + sizeof(*chan));
- hdr = (void *)hdr + priv->tx_hdr_len;
+ if (!skb)
+ return -ENOMEM;
- chan = (struct p54_tx_control_channel *) hdr->data;
+ skb_reserve(skb, priv->tx_hdr_len);
+ hdr = (struct p54_control_hdr *)skb_put(skb, sizeof(*hdr));
hdr->magic1 = cpu_to_le16(0x8001);
-
hdr->type = cpu_to_le16(P54_CONTROL_TYPE_CHANNEL_CHANGE);
-
+ hdr->retry1 = hdr->retry2 = 0;
+ chan = (struct p54_tx_control_channel *) skb_put(skb, sizeof(*chan));
chan->flags = cpu_to_le16(0x1);
chan->dwell = cpu_to_le16(0x0);
@@ -1018,32 +1124,35 @@ static int p54_set_freq(struct ieee80211
}
hdr->len = cpu_to_le16(data_len);
- p54_assign_address(dev, NULL, hdr, sizeof(*hdr) + data_len);
- priv->tx(dev, hdr, sizeof(*hdr) + data_len, 1);
+ p54_assign_address(dev, skb, hdr, sizeof(*hdr) + data_len);
+ priv->tx(dev, hdr, skb, sizeof(*hdr) + data_len, 1);
return 0;
err:
printk(KERN_ERR "%s: frequency change failed\n", wiphy_name(dev->wiphy));
- kfree(hdr);
+ kfree_skb(skb);
return -EINVAL;
}
static int p54_set_leds(struct ieee80211_hw *dev, int mode, int link, int act)
{
struct p54_common *priv = dev->priv;
+ struct sk_buff *skb;
struct p54_control_hdr *hdr;
struct p54_tx_control_led *led;
- hdr = kzalloc(sizeof(*hdr) + sizeof(*led) +
- priv->tx_hdr_len, GFP_KERNEL);
- if (!hdr)
+ skb = dev_alloc_skb(sizeof(*hdr) + sizeof(*led) +
+ priv->tx_hdr_len);
+ if (!skb)
return -ENOMEM;
- hdr = (void *)hdr + priv->tx_hdr_len;
+ skb_reserve(skb, priv->tx_hdr_len);
+
+ hdr = (void *)skb_put(skb, sizeof(*hdr) + sizeof(*led));
hdr->magic1 = cpu_to_le16(0x8001);
hdr->len = cpu_to_le16(sizeof(*led));
hdr->type = cpu_to_le16(P54_CONTROL_TYPE_LED);
- p54_assign_address(dev, NULL, hdr, sizeof(*hdr) + sizeof(*led));
+ hdr->retry1 = hdr->retry2 = 0;
led = (struct p54_tx_control_led *) hdr->data;
led->mode = cpu_to_le16(mode);
@@ -1051,7 +1160,8 @@ static int p54_set_leds(struct ieee80211
led->led_temporary = cpu_to_le16(act);
led->duration = cpu_to_le16(1000);
- priv->tx(dev, hdr, sizeof(*hdr) + sizeof(*led), 1);
+ p54_assign_address(dev, skb, hdr, skb->len);
+ priv->tx(dev, hdr, skb, skb->len, 1);
return 0;
}
@@ -1064,53 +1174,78 @@ do { \
queue.txop = cpu_to_le16(_txop); \
} while(0)
-static void p54_init_vdcf(struct ieee80211_hw *dev)
+static int p54_set_vdcf(struct ieee80211_hw *dev)
{
struct p54_common *priv = dev->priv;
struct p54_control_hdr *hdr;
struct p54_tx_control_vdcf *vdcf;
+ struct sk_buff *skb;
+
+ skb = dev_alloc_skb(priv->tx_hdr_len +
+ sizeof(struct p54_control_hdr) +
+ sizeof(struct p54_tx_control_vdcf));
+
+ if (!skb)
+ return -ENOMEM;
+
+ skb_reserve(skb, priv->tx_hdr_len);
- /* all USB V1 adapters need a extra headroom */
- hdr = (void *)priv->cached_vdcf + priv->tx_hdr_len;
+ hdr = (void *)skb_put(skb, sizeof(*hdr));
hdr->magic1 = cpu_to_le16(0x8001);
hdr->len = cpu_to_le16(sizeof(*vdcf));
hdr->type = cpu_to_le16(P54_CONTROL_TYPE_DCFINIT);
hdr->req_id = cpu_to_le32(priv->rx_start);
+ hdr->retry1 = hdr->retry2 = 0;
+ vdcf = (void *)skb_put(skb, sizeof(*vdcf));
+ if (dev->conf.flags & IEEE80211_CONF_SHORT_SLOT_TIME) {
+ vdcf->slottime = 9;
+ vdcf->sifs = 0x10;
+ vdcf->eofpad = 0x00;
+ } else {
+ vdcf->slottime = 20;
+ vdcf->sifs = 0x0a;
+ vdcf->eofpad = 0x06;
+ }
+ memset(vdcf->mapping, 0, sizeof(vdcf->mapping));
+ memcpy(vdcf->queue, priv->edcf, sizeof(priv->edcf));
+
+ /* (see prism54/isl_oid.h for further details) */
+ vdcf->frameburst = cpu_to_le16(0);
- vdcf = (struct p54_tx_control_vdcf *) hdr->data;
+ p54_assign_address(dev, skb, hdr, skb->len);
+ priv->tx(dev, hdr, skb, skb->len, 1);
- P54_SET_QUEUE(vdcf->queue[0], 0x0002, 0x0003, 0x0007, 47);
- P54_SET_QUEUE(vdcf->queue[1], 0x0002, 0x0007, 0x000f, 94);
- P54_SET_QUEUE(vdcf->queue[2], 0x0003, 0x000f, 0x03ff, 0);
- P54_SET_QUEUE(vdcf->queue[3], 0x0007, 0x000f, 0x03ff, 0);
+ return 0;
}
-static void p54_set_vdcf(struct ieee80211_hw *dev)
+static int p54_init_stats(struct ieee80211_hw *dev)
{
struct p54_common *priv = dev->priv;
struct p54_control_hdr *hdr;
- struct p54_tx_control_vdcf *vdcf;
+ struct p54_statistics *stats;
- hdr = (void *)priv->cached_vdcf + priv->tx_hdr_len;
+ priv->cached_stats = dev_alloc_skb(priv->tx_hdr_len +
+ sizeof(struct p54_control_hdr) +
+ sizeof(struct p54_statistics));
- p54_assign_address(dev, NULL, hdr, sizeof(*hdr) + sizeof(*vdcf));
+ if (!priv->cached_stats)
+ return -ENOMEM;
- vdcf = (struct p54_tx_control_vdcf *) hdr->data;
+ skb_reserve(priv->cached_stats, priv->tx_hdr_len);
- if (dev->conf.flags & IEEE80211_CONF_SHORT_SLOT_TIME) {
- vdcf->slottime = 9;
- vdcf->magic1 = 0x10;
- vdcf->magic2 = 0x00;
- } else {
- vdcf->slottime = 20;
- vdcf->magic1 = 0x0a;
- vdcf->magic2 = 0x06;
- }
+ hdr = (void *) skb_put(priv->cached_stats, sizeof(*hdr));
+ hdr->magic1 = cpu_to_le16(0x8000);
+ hdr->len = cpu_to_le16(sizeof(*stats));
+ hdr->type = cpu_to_le16(P54_CONTROL_TYPE_STAT_READBACK);
+ hdr->retry1 = hdr->retry2 = 0;
+ stats = (void *) skb_put(priv->cached_stats, sizeof(*stats));
- /* (see prism54/isl_oid.h for further details) */
- vdcf->frameburst = cpu_to_le16(0);
+ p54_assign_address(dev, priv->cached_stats,
+ hdr, priv->cached_stats->len);
+
+ mod_timer(&priv->stats_timer, jiffies + HZ);
- priv->tx(dev, hdr, sizeof(*hdr) + sizeof(*vdcf), 0);
+ return 0;
}
static int p54_start(struct ieee80211_hw *dev)
@@ -1118,34 +1253,18 @@ static int p54_start(struct ieee80211_hw
struct p54_common *priv = dev->priv;
int err;
- if (!priv->cached_vdcf) {
- priv->cached_vdcf = kzalloc(sizeof(struct p54_tx_control_vdcf)+
- priv->tx_hdr_len + sizeof(struct p54_control_hdr),
- GFP_KERNEL);
-
- if (!priv->cached_vdcf)
- return -ENOMEM;
- }
-
- if (!priv->cached_stats) {
- priv->cached_stats = kzalloc(sizeof(struct p54_statistics) +
- priv->tx_hdr_len + sizeof(struct p54_control_hdr),
- GFP_KERNEL);
-
- if (!priv->cached_stats) {
- kfree(priv->cached_vdcf);
- priv->cached_vdcf = NULL;
- return -ENOMEM;
- }
- }
-
err = priv->open(dev);
if (!err)
priv->mode = NL80211_IFTYPE_MONITOR;
- p54_init_vdcf(dev);
+ P54_SET_QUEUE(priv->edcf[0], 0x0002, 0x0003, 0x0007, 47);
+ P54_SET_QUEUE(priv->edcf[1], 0x0002, 0x0007, 0x000f, 94);
+ P54_SET_QUEUE(priv->edcf[2], 0x0003, 0x000f, 0x03ff, 0);
+ P54_SET_QUEUE(priv->edcf[3], 0x0007, 0x000f, 0x03ff, 0);
+ err = p54_set_vdcf(dev);
+ if (!err)
+ err = p54_init_stats(dev);
- mod_timer(&priv->stats_timer, jiffies + HZ);
return err;
}
@@ -1155,8 +1274,11 @@ static void p54_stop(struct ieee80211_hw
struct sk_buff *skb;
del_timer(&priv->stats_timer);
+ p54_free_skb(dev, priv->cached_stats);
+ priv->cached_stats = NULL;
while ((skb = skb_dequeue(&priv->tx_queue)))
kfree_skb(skb);
+
priv->stop(dev);
priv->tsf_high32 = priv->tsf_low32 = 0;
priv->mode = NL80211_IFTYPE_UNSPECIFIED;
@@ -1270,46 +1392,39 @@ static int p54_conf_tx(struct ieee80211_
const struct ieee80211_tx_queue_params *params)
{
struct p54_common *priv = dev->priv;
- struct p54_tx_control_vdcf *vdcf;
-
- vdcf = (struct p54_tx_control_vdcf *)(((struct p54_control_hdr *)
- ((void *)priv->cached_vdcf + priv->tx_hdr_len))->data);
if ((params) && !(queue > 4)) {
- P54_SET_QUEUE(vdcf->queue[queue], params->aifs,
+ P54_SET_QUEUE(priv->edcf[queue], params->aifs,
params->cw_min, params->cw_max, params->txop);
} else
return -EINVAL;
- p54_set_vdcf(dev);
-
- return 0;
+ return p54_set_vdcf(dev);
}
static int p54_init_xbow_synth(struct ieee80211_hw *dev)
{
struct p54_common *priv = dev->priv;
+ struct sk_buff *skb;
struct p54_control_hdr *hdr;
struct p54_tx_control_xbow_synth *xbow;
- hdr = kzalloc(sizeof(*hdr) + sizeof(*xbow) +
- priv->tx_hdr_len, GFP_KERNEL);
- if (!hdr)
+ skb = dev_alloc_skb(priv->tx_hdr_len + sizeof(*hdr) + sizeof(*xbow));
+ if (!skb)
return -ENOMEM;
+ skb_reserve(skb, priv->tx_hdr_len);
- hdr = (void *)hdr + priv->tx_hdr_len;
+ hdr = (struct p54_control_hdr *)skb_put(skb, sizeof(*hdr));
hdr->magic1 = cpu_to_le16(0x8001);
hdr->len = cpu_to_le16(sizeof(*xbow));
hdr->type = cpu_to_le16(P54_CONTROL_TYPE_XBOW_SYNTH_CFG);
- p54_assign_address(dev, NULL, hdr, sizeof(*hdr) + sizeof(*xbow));
-
- xbow = (struct p54_tx_control_xbow_synth *) hdr->data;
+ xbow = (struct p54_tx_control_xbow_synth *)skb_put(skb, sizeof(*xbow));
xbow->magic1 = cpu_to_le16(0x1);
xbow->magic2 = cpu_to_le16(0x2);
xbow->freq = cpu_to_le16(5390);
- priv->tx(dev, hdr, sizeof(*hdr) + sizeof(*xbow), 1);
-
+ p54_assign_address(dev, skb, hdr, sizeof(*hdr) + sizeof(*xbow));
+ priv->tx(dev, hdr, skb, sizeof(*hdr) + sizeof(*xbow), 1);
return 0;
}
@@ -1318,17 +1433,11 @@ static void p54_statistics_timer(unsigne
struct ieee80211_hw *dev = (struct ieee80211_hw *) data;
struct p54_common *priv = dev->priv;
struct p54_control_hdr *hdr;
- struct p54_statistics *stats;
BUG_ON(!priv->cached_stats);
- hdr = (void *)priv->cached_stats + priv->tx_hdr_len;
- hdr->magic1 = cpu_to_le16(0x8000);
- hdr->len = cpu_to_le16(sizeof(*stats));
- hdr->type = cpu_to_le16(P54_CONTROL_TYPE_STAT_READBACK);
- p54_assign_address(dev, NULL, hdr, sizeof(*hdr) + sizeof(*stats));
-
- priv->tx(dev, hdr, sizeof(*hdr) + sizeof(*stats), 0);
+ hdr = (struct p54_control_hdr *)priv->cached_stats->data;
+ priv->tx(dev, hdr, priv->cached_stats, priv->cached_stats->len, 0);
}
static int p54_get_stats(struct ieee80211_hw *dev,
@@ -1418,11 +1527,12 @@ EXPORT_SYMBOL_GPL(p54_init_common);
void p54_free_common(struct ieee80211_hw *dev)
{
struct p54_common *priv = dev->priv;
- kfree(priv->cached_stats);
+
+ del_timer(&priv->stats_timer);
+ kfree_skb(priv->cached_stats);
kfree(priv->iq_autocal);
kfree(priv->output_limit);
kfree(priv->curve_data);
- kfree(priv->cached_vdcf);
}
EXPORT_SYMBOL_GPL(p54_free_common);
diff -Nurp a/drivers/net/wireless/p54/p54common.h b/drivers/net/wireless/p54/p54common.h
--- a/drivers/net/wireless/p54/p54common.h 2008-09-29 20:39:31.000000000 +0200
+++ b/drivers/net/wireless/p54/p54common.h 2008-10-02 02:19:27.000000000 +0200
@@ -169,6 +169,7 @@ struct pda_pa_curve_data {
struct memrecord {
u32 start_addr;
u32 end_addr;
+ struct ieee80211_hw *dev;
};
struct p54_eeprom_lm86 {
@@ -285,21 +286,15 @@ struct p54_tx_control_led {
__le16 duration;
} __attribute__ ((packed));
-struct p54_tx_vdcf_queues {
- __le16 aifs;
- __le16 cwmin;
- __le16 cwmax;
- __le16 txop;
-} __attribute__ ((packed));
-
struct p54_tx_control_vdcf {
u8 padding;
u8 slottime;
- u8 magic1;
- u8 magic2;
+ u8 sifs;
+ u8 eofpad;
struct p54_tx_vdcf_queues queue[8];
- u8 pad2[4];
+ u8 mapping[4];
__le16 frameburst;
+ __le16 round_trip_delay;
} __attribute__ ((packed));
struct p54_statistics {
diff -Nurp a/drivers/net/wireless/p54/p54.h b/drivers/net/wireless/p54/p54.h
--- a/drivers/net/wireless/p54/p54.h 2008-09-29 20:39:31.000000000 +0200
+++ b/drivers/net/wireless/p54/p54.h 2008-10-02 01:11:30.000000000 +0200
@@ -49,6 +49,13 @@ struct p54_control_hdr {
u8 data[0];
} __attribute__ ((packed));
+struct p54_tx_vdcf_queues {
+ __le16 aifs;
+ __le16 cwmin;
+ __le16 cwmax;
+ __le16 txop;
+} __attribute__ ((packed));
+
#define EEPROM_READBACK_LEN 0x3fc
#define ISL38XX_DEV_FIRMWARE_ADDR 0x20000
@@ -63,7 +70,7 @@ struct p54_common {
u32 rx_end;
struct sk_buff_head tx_queue;
void (*tx)(struct ieee80211_hw *dev, struct p54_control_hdr *data,
- size_t len, int free_on_tx);
+ struct sk_buff *skb, size_t len, int free_on_tx);
int (*open)(struct ieee80211_hw *dev);
void (*stop)(struct ieee80211_hw *dev);
int mode;
@@ -85,23 +92,24 @@ struct p54_common {
u8 version;
u8 rx_antenna;
unsigned int tx_hdr_len;
- void *cached_vdcf;
unsigned int fw_var;
unsigned int fw_interface;
unsigned int output_power;
u32 tsf_low32;
u32 tsf_high32;
struct ieee80211_tx_queue_stats tx_stats[8];
+ struct p54_tx_vdcf_queues edcf[8];
struct ieee80211_low_level_stats stats;
struct timer_list stats_timer;
struct completion stats_comp;
- void *cached_stats;
+ struct sk_buff *cached_stats;
int noise;
void *eeprom;
struct completion eeprom_comp;
};
int 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_read_eeprom(struct ieee80211_hw *dev);
struct ieee80211_hw *p54_init_common(size_t priv_data_len);
diff -Nurp a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
--- a/drivers/net/wireless/p54/p54pci.c 2008-09-29 20:39:31.000000000 +0200
+++ b/drivers/net/wireless/p54/p54pci.c 2008-10-02 03:21:13.000000000 +0200
@@ -235,7 +235,7 @@ static void p54p_check_tx_ring(struct ie
while (i != idx) {
desc = &ring[i];
- kfree(tx_buf[i]);
+ p54_free_skb(dev, tx_buf[i]);
tx_buf[i] = NULL;
pci_unmap_single(priv->pdev, le32_to_cpu(desc->host_addr),
@@ -307,7 +307,7 @@ static irqreturn_t p54p_interrupt(int ir
}
static void p54p_tx(struct ieee80211_hw *dev, struct p54_control_hdr *data,
- size_t len, int free_on_tx)
+ struct sk_buff *skb, size_t len, int free_on_tx)
{
struct p54p_priv *priv = dev->priv;
struct p54p_ring_control *ring_control = priv->ring_control;
@@ -333,7 +333,7 @@ static void p54p_tx(struct ieee80211_hw
ring_control->host_idx[1] = cpu_to_le32(idx + 1);
if (free_on_tx)
- priv->tx_buf_data[i] = data;
+ priv->tx_buf_data[i] = skb;
spin_unlock_irqrestore(&priv->lock, flags);
@@ -342,8 +342,10 @@ static void p54p_tx(struct ieee80211_hw
/* FIXME: unlikely to happen because the device usually runs out of
memory before we fill the ring up, but we can make it impossible */
- if (idx - device_idx > ARRAY_SIZE(ring_control->tx_data) - 2)
+ if (idx - device_idx > ARRAY_SIZE(ring_control->tx_data) - 2) {
+ p54_free_skb(dev, skb);
printk(KERN_INFO "%s: tx overflow.\n", wiphy_name(dev->wiphy));
+ }
}
static int p54p_open(struct ieee80211_hw *dev)
@@ -455,7 +457,7 @@ static void p54p_stop(struct ieee80211_h
le16_to_cpu(desc->len),
PCI_DMA_TODEVICE);
- kfree(priv->tx_buf_data[i]);
+ p54_free_skb(dev, priv->tx_buf_data[i]);
priv->tx_buf_data[i] = NULL;
}
@@ -467,7 +469,7 @@ static void p54p_stop(struct ieee80211_h
le16_to_cpu(desc->len),
PCI_DMA_TODEVICE);
- kfree(priv->tx_buf_mgmt[i]);
+ p54_free_skb(dev, priv->tx_buf_mgmt[i]);
priv->tx_buf_mgmt[i] = NULL;
}
diff -Nurp a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
--- a/drivers/net/wireless/p54/p54usb.c 2008-09-29 20:39:31.000000000 +0200
+++ b/drivers/net/wireless/p54/p54usb.c 2008-10-02 03:08:40.000000000 +0200
@@ -22,6 +22,7 @@
#include <net/mac80211.h>
#include "p54.h"
+#include "p54common.h"
#include "p54usb.h"
MODULE_AUTHOR("Michael Wu <flamingice@sourmilk.net>");
@@ -134,6 +135,18 @@ static void p54u_rx_cb(struct urb *urb)
usb_submit_urb(urb, GFP_ATOMIC);
}
+static void p54u_tx_reuse_skb_cb(struct urb *urb)
+{
+ struct sk_buff *skb = urb->context;
+ struct ieee80211_tx_info *ni = IEEE80211_SKB_CB(skb);
+ struct memrecord *mr = (struct memrecord *) ni->driver_data;
+ struct p54u_priv *priv = (struct p54u_priv *) mr->dev->priv;
+
+ skb_pull(skb, priv->common.tx_hdr_len);
+
+ usb_free_urb(urb);
+}
+
static void p54u_tx_cb(struct urb *urb)
{
usb_free_urb(urb);
@@ -145,6 +158,16 @@ static void p54u_tx_free_cb(struct urb *
usb_free_urb(urb);
}
+static void p54u_tx_free_skb_cb(struct urb *urb)
+{
+ struct sk_buff *skb = urb->context;
+ struct ieee80211_tx_info *ni = IEEE80211_SKB_CB(skb);
+ struct memrecord *mr = (struct memrecord *)ni->driver_data;
+
+ p54_free_skb(mr->dev, skb);
+ usb_free_urb(urb);
+}
+
static int p54u_init_urbs(struct ieee80211_hw *dev)
{
struct p54u_priv *priv = dev->priv;
@@ -192,7 +215,7 @@ static void p54u_free_urbs(struct ieee80
}
static void p54u_tx_3887(struct ieee80211_hw *dev, struct p54_control_hdr *data,
- size_t len, int free_on_tx)
+ struct sk_buff *skb, size_t len, int free_on_tx)
{
struct p54u_priv *priv = dev->priv;
struct urb *addr_urb, *data_urb;
@@ -212,7 +235,7 @@ static void p54u_tx_3887(struct ieee8021
sizeof(data->req_id), p54u_tx_cb, dev);
usb_fill_bulk_urb(data_urb, priv->udev,
usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), data, len,
- free_on_tx ? p54u_tx_free_cb : p54u_tx_cb, dev);
+ free_on_tx ? p54u_tx_free_skb_cb : p54u_tx_reuse_skb_cb, skb);
usb_submit_urb(addr_urb, GFP_ATOMIC);
usb_submit_urb(data_urb, GFP_ATOMIC);
@@ -231,13 +254,13 @@ static __le32 p54u_lm87_chksum(const u32
return cpu_to_le32(chk);
}
-static void p54u_tx_lm87(struct ieee80211_hw *dev,
- struct p54_control_hdr *data,
- size_t len, int free_on_tx)
+static void p54u_tx_lm87(struct ieee80211_hw *dev, struct p54_control_hdr *data,
+ struct sk_buff *skb, size_t len, int free_on_tx)
{
struct p54u_priv *priv = dev->priv;
struct urb *data_urb;
- struct lm87_tx_hdr *hdr = (void *)data - sizeof(*hdr);
+ struct lm87_tx_hdr *hdr = (struct lm87_tx_hdr *)
+ skb_push(skb, sizeof(*hdr));
data_urb = usb_alloc_urb(0, GFP_ATOMIC);
if (!data_urb)
@@ -248,14 +271,14 @@ static void p54u_tx_lm87(struct ieee8021
usb_fill_bulk_urb(data_urb, priv->udev,
usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), hdr,
- len + sizeof(*hdr), free_on_tx ? p54u_tx_free_cb : p54u_tx_cb,
- dev);
+ len + sizeof(*hdr), free_on_tx ? p54u_tx_free_skb_cb :
+ p54u_tx_reuse_skb_cb, skb);
usb_submit_urb(data_urb, GFP_ATOMIC);
}
static void p54u_tx_net2280(struct ieee80211_hw *dev, struct p54_control_hdr *data,
- size_t len, int free_on_tx)
+ struct sk_buff *skb, size_t len, int free_on_tx)
{
struct p54u_priv *priv = dev->priv;
struct urb *int_urb, *data_urb;
@@ -296,7 +319,7 @@ static void p54u_tx_net2280(struct ieee8
usb_fill_bulk_urb(data_urb, priv->udev,
usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), hdr, len + sizeof(*hdr),
- free_on_tx ? p54u_tx_free_cb : p54u_tx_cb, dev);
+ free_on_tx ? p54u_tx_free_skb_cb : p54u_tx_reuse_skb_cb, skb);
usb_submit_urb(data_urb, GFP_ATOMIC);
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC][PATCH] p54: fix memory management
2008-10-02 1:59 Christian Lamparter
@ 2008-10-03 2:43 ` Larry Finger
2008-10-03 20:02 ` Christian Lamparter
0 siblings, 1 reply; 5+ messages in thread
From: Larry Finger @ 2008-10-03 2:43 UTC (permalink / raw)
To: Christian Lamparter
Cc: linux-wireless, John W Linville, Johannes Berg, Kalle Valo
Christian Lamparter wrote:
> Well, since a lot of people have worked with the memory management of p54 driver
> (or similar projects ;-) ), it's probably the best to give this _urgent_ thing a RFC round.
>
> The problem is that if multiple "control frames" are passed in a very short intervals to
> the device's firmware (e.g: QoS and frequency are the best candidates)
> the data might get corrupted. As p54_assign_address always put them into same
> memory location and the device's firmware is too slow to pick the frames up,
> before they're overwritten again.
>
> P.S: the code inside the #if 0 - #endif will go away in the final version,
> but for now its very useful for debugging. So don't complain about it ;-).
When the patch works, it seems to be OK; however, I got two oops and
one system lockup (caps lock blinking at a 1 Hz rate). The first one
is as follows:
Oct 2 17:22:52 larrylap kernel: BUG: unable to handle kernel NULL
pointer dereference at 0000000000000000
Oct 2 17:22:52 larrylap kernel: IP: [<ffffffffa024916e>]
p54_assign_address+0xff/0x162 [p54common]
Oct 2 17:22:52 larrylap kernel: PGD 0
Oct 2 17:22:52 larrylap kernel: Oops: 0000 [1] SMP
Oct 2 17:22:52 larrylap kernel: CPU 0
Oct 2 17:22:52 larrylap kernel: Modules linked in: snd_pcm_oss
snd_mixer_oss snd_seq snd_seq_device af_packet sunrpc rfkill_input
cpufreq_conservative cpufreq_userspace cpufreq_powersave powernow_k8
fuse loop dm_mod ide_cd_mod cdrom arc4 ecb crypto_blkcipher p54usb b43
snd_hda_intel rfkill amd74xx p54common snd_pcm led_class
ide_pci_generic mac80211 snd_timer k8temp input_polldev snd battery ac
button hwmon joydev ide_core forcedeth soundcore cfg80211 ssb
snd_page_alloc serio_raw sg sd_mod ehci_hcd ohci_hcd usbcore edd fan
thermal processor ext3 mbcache jbd ahci libata scsi_mod dock
Oct 2 17:22:52 larrylap kernel: Pid: 2021, comm: p54usb Tainted: G
M 2.6.27-rc8-wl #51
Oct 2 17:22:52 larrylap kernel: RIP: 0010:[<ffffffffa024916e>]
[<ffffffffa024916e>] p54_assign_address+0xff/0x162 [p54common]
Oct 2 17:22:52 larrylap kernel: RSP: 0018:ffff8800ba0ebd20 EFLAGS:
00010006
Oct 2 17:22:52 larrylap kernel: RAX: ffff8800ba0b04a0 RBX:
ffff8800ba0b1d00 RCX: 000000000000009c
Oct 2 17:22:52 larrylap kernel: RDX: ffff8800b9260f40 RSI:
0000000000000000 RDI: ffff8800b9260840
Oct 2 17:22:52 larrylap kernel: RBP: ffff8800ba0ebd70 R08:
0000000000000002 R09: ffffffffa02490d0
Oct 2 17:22:52 larrylap kernel: R10: ffff8800b9260f00 R11:
ffff8800a7de2480 R12: 00000000000000c0
Oct 2 17:22:52 larrylap kernel: R13: ffff8800b9260f00 R14:
0000000000024178 R15: ffff8800ba0b1d08
Oct 2 17:22:52 larrylap kernel: FS: 00007f9a5f2fe6f0(0000)
GS:ffffffff8069d680(0000) knlGS:00000000f7caf6c0
Oct 2 17:22:52 larrylap kernel: CS: 0010 DS: 0018 ES: 0018 CR0:
000000008005003b
Oct 2 17:22:52 larrylap kernel: CR2: 0000000000000000 CR3:
00000000b9cdc000 CR4: 00000000000006e0
Oct 2 17:22:52 larrylap kernel: DR0: 0000000000000000 DR1:
0000000000000000 DR2: 0000000000000000
Oct 2 17:22:52 larrylap kernel: DR3: 0000000000000000 DR6:
00000000ffff0ff0 DR7: 0000000000000400
Oct 2 17:22:52 larrylap kernel: Process p54usb (pid: 2021, threadinfo
ffff8800ba0ea000, task ffff8800b8a4c790)
Oct 2 17:22:52 larrylap kernel: Stack: ffff8800a7e69e10
ffff8800ba0b04a0 ffff8800ba0b1d20 0002020000000000
Oct 2 17:22:52 larrylap kernel: 0000000000000286 ffff8800b9260f00
ffff8800a7e69e10 ffff8800ba0b1d00
Oct 2 17:22:52 larrylap kernel: ffff8800ba0b04a0 ffff8800ba0b04a0
ffff8800ba0ebdb0 ffffffffa02499fb
Oct 2 17:22:52 larrylap kernel: Call Trace:
Oct 2 17:22:52 larrylap kernel: [<ffffffffa02499fb>]
p54_set_vdcf+0xe1/0x104 [p54common]
Oct 2 17:22:52 larrylap kernel: [<ffffffffa024a0fb>]
p54_config+0x2b0/0x2ca [p54common]
Oct 2 17:22:52 larrylap kernel: [<ffffffff802339a2>] ?
finish_task_switch+0x0/0xb9
Oct 2 17:22:52 larrylap kernel: [<ffffffffa01f1055>]
ieee80211_hw_config+0x55/0x57 [mac80211]
Oct 2 17:22:52 larrylap kernel: [<ffffffffa01f6de7>]
ieee80211_scan_work+0xd1/0x196 [mac80211]
Oct 2 17:22:52 larrylap kernel: [<ffffffff802478ef>]
run_workqueue+0x103/0x20a
Oct 2 17:22:52 larrylap kernel: [<ffffffff8024789d>] ?
run_workqueue+0xb1/0x20a
Oct 2 17:22:52 larrylap kernel: [<ffffffffa01f6d16>] ?
ieee80211_scan_work+0x0/0x196 [mac80211]
Oct 2 17:22:52 larrylap kernel: [<ffffffff80247ad6>]
worker_thread+0xe0/0xf1
Oct 2 17:22:52 larrylap kernel: [<ffffffff8024b5e0>] ?
autoremove_wake_function+0x0/0x38
Oct 2 17:22:53 larrylap kernel: [<ffffffff802479f6>] ?
worker_thread+0x0/0xf1
Oct 2 17:22:53 larrylap kernel: [<ffffffff8024b284>] kthread+0x49/0x76
Oct 2 17:22:53 larrylap kernel: [<ffffffff8020d099>] child_rip+0xa/0x11
Oct 2 17:22:53 larrylap kernel: [<ffffffff80427834>] ?
_spin_unlock_irq+0x2b/0x30
Oct 2 17:22:53 larrylap kernel: [<ffffffff8020c6cf>] ?
restore_args+0x0/0x30
Oct 2 17:22:53 larrylap kernel: [<ffffffff8024b23b>] ? kthread+0x0/0x76
Oct 2 17:22:53 larrylap kernel: [<ffffffff8020d08f>] ?
child_rip+0x0/0x11
Oct 2 17:22:53 larrylap kernel:
Oct 2 17:22:53 larrylap kernel:
Oct 2 17:22:53 larrylap kernel: Code: 8b 4b 04 44 29 f1 39 d1 0f 42
ca 4d 85 ed 74 54 8b 45 cc 49 8d 55 40 89 c9 41 89 45 40 44 01 e0 89
42 04 48 8b 45 b8 48 89 42 08 <48> 8b 06 49 89 75 08 49 89 45 00 4c 89
68 08 4c 89 2e 0f b7 53
Oct 2 17:22:53 larrylap kernel: RIP [<ffffffffa024916e>]
p54_assign_address+0xff/0x162 [p54common]
Oct 2 17:22:53 larrylap kernel: RSP <ffff8800ba0ebd20>
Oct 2 17:22:53 larrylap kernel: CR2: 0000000000000000
Oct 2 17:22:53 larrylap kernel: ---[ end trace 4bd18aa5f2aeb5d8 ]---
Note, the "tainted" flag is false. No closed-source drivers have been
loaded.
The oops occurs in the following inline routine:
static inline void __skb_queue_after(struct sk_buff_head *list,
struct sk_buff *prev,
struct sk_buff *newsk)
{
__skb_insert(newsk, prev, prev->next, list);
}
and is called from p54_assign_addresses() in the following region:
if (skb) {
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct memrecord *range = (void *)info->driver_data;
range->start_addr = target_addr;
range->end_addr = target_addr + len;
range->dev = dev;
__skb_queue_after(&priv->tx_queue, target_skb, skb);
if (largest_hole < priv->rx_mtu + priv->headroom +
priv->tailroom +
sizeof(struct p54_control_hdr))
ieee80211_stop_queues(dev);
}
Larry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC][PATCH] p54: fix memory management
@ 2008-10-03 14:16 Chunkeey
2008-10-03 19:25 ` Christian Lamparter
0 siblings, 1 reply; 5+ messages in thread
From: Chunkeey @ 2008-10-03 14:16 UTC (permalink / raw)
To: Larry Finger, Christian Lamparter
Cc: Johannes Berg, Kalle Valo, linux-wireless, John W Linville
> Christian Lamparter wrote:
> > Well, since a lot of people have worked with the memory management of p54 driver
> > (or similar projects ;-) ), it's probably the best to give this _urgent_ thing a RFC round.
> >
> > The problem is that if multiple "control frames" are passed in a very short intervals to
> > the device's firmware (e.g: QoS and frequency are the best candidates)
> > the data might get corrupted. As p54_assign_address always put them into same
> > memory location and the device's firmware is too slow to pick the frames up,
> > before they're overwritten again.
> >
> > P.S: the code inside the #if 0 - #endif will go away in the final version,
> > but for now its very useful for debugging. So don't complain about it ;-).
>
> When the patch works, it seems to be OK; however, I got two oops and
> one system lockup (caps lock blinking at a 1 Hz rate). The first one
> is as follows:
>
> Oct 2 17:22:52 larrylap kernel: BUG: unable to handle kernel NULL
> pointer dereference at 0000000000000000
> Oct 2 17:22:52 larrylap kernel: IP: [<ffffffffa024916e>]
> p54_assign_address+0xff/0x162 [p54common]
> Oct 2 17:22:52 larrylap kernel: PGD 0
> Oct 2 17:22:52 larrylap kernel: Oops: 0000 [1] SMP
> Oct 2 17:22:52 larrylap kernel: CPU 0
> Oct 2 17:22:52 larrylap kernel: Modules linked in: snd_pcm_oss
> snd_mixer_oss snd_seq snd_seq_device af_packet sunrpc rfkill_input
> cpufreq_conservative cpufreq_userspace cpufreq_powersave powernow_k8
> fuse loop dm_mod ide_cd_mod cdrom arc4 ecb crypto_blkcipher p54usb b43
> snd_hda_intel rfkill amd74xx p54common snd_pcm led_class
> ide_pci_generic mac80211 snd_timer k8temp input_polldev snd battery ac
> button hwmon joydev ide_core forcedeth soundcore cfg80211 ssb
> snd_page_alloc serio_raw sg sd_mod ehci_hcd ohci_hcd usbcore edd fan
> thermal processor ext3 mbcache jbd ahci libata scsi_mod dock
> Oct 2 17:22:52 larrylap kernel: Pid: 2021, comm: p54usb Tainted: G
> M 2.6.27-rc8-wl #51
> Oct 2 17:22:52 larrylap kernel: RIP: 0010:[<ffffffffa024916e>]
> [<ffffffffa024916e>] p54_assign_address+0xff/0x162 [p54common]
> Oct 2 17:22:52 larrylap kernel: RSP: 0018:ffff8800ba0ebd20 EFLAGS:
> 00010006
> Oct 2 17:22:52 larrylap kernel: RAX: ffff8800ba0b04a0 RBX:
> ffff8800ba0b1d00 RCX: 000000000000009c
> Oct 2 17:22:52 larrylap kernel: RDX: ffff8800b9260f40 RSI:
> 0000000000000000 RDI: ffff8800b9260840
> Oct 2 17:22:52 larrylap kernel: RBP: ffff8800ba0ebd70 R08:
> 0000000000000002 R09: ffffffffa02490d0
> Oct 2 17:22:52 larrylap kernel: R10: ffff8800b9260f00 R11:
> ffff8800a7de2480 R12: 00000000000000c0
> Oct 2 17:22:52 larrylap kernel: R13: ffff8800b9260f00 R14:
> 0000000000024178 R15: ffff8800ba0b1d08
> Oct 2 17:22:52 larrylap kernel: FS: 00007f9a5f2fe6f0(0000)
> GS:ffffffff8069d680(0000) knlGS:00000000f7caf6c0
> Oct 2 17:22:52 larrylap kernel: CS: 0010 DS: 0018 ES: 0018 CR0:
> 000000008005003b
> Oct 2 17:22:52 larrylap kernel: CR2: 0000000000000000 CR3:
> 00000000b9cdc000 CR4: 00000000000006e0
> Oct 2 17:22:52 larrylap kernel: DR0: 0000000000000000 DR1:
> 0000000000000000 DR2: 0000000000000000
> Oct 2 17:22:52 larrylap kernel: DR3: 0000000000000000 DR6:
> 00000000ffff0ff0 DR7: 0000000000000400
> Oct 2 17:22:52 larrylap kernel: Process p54usb (pid: 2021, threadinfo
> ffff8800ba0ea000, task ffff8800b8a4c790)
> Oct 2 17:22:52 larrylap kernel: Stack: ffff8800a7e69e10
> ffff8800ba0b04a0 ffff8800ba0b1d20 0002020000000000
> Oct 2 17:22:52 larrylap kernel: 0000000000000286 ffff8800b9260f00
> ffff8800a7e69e10 ffff8800ba0b1d00
> Oct 2 17:22:52 larrylap kernel: ffff8800ba0b04a0 ffff8800ba0b04a0
> ffff8800ba0ebdb0 ffffffffa02499fb
> Oct 2 17:22:52 larrylap kernel: Call Trace:
> Oct 2 17:22:52 larrylap kernel: [<ffffffffa02499fb>]
> p54_set_vdcf+0xe1/0x104 [p54common]
> Oct 2 17:22:52 larrylap kernel: [<ffffffffa024a0fb>]
> p54_config+0x2b0/0x2ca [p54common]
> Oct 2 17:22:52 larrylap kernel: [<ffffffff802339a2>] ?
> finish_task_switch+0x0/0xb9
> Oct 2 17:22:52 larrylap kernel: [<ffffffffa01f1055>]
> ieee80211_hw_config+0x55/0x57 [mac80211]
> Oct 2 17:22:52 larrylap kernel: [<ffffffffa01f6de7>]
> ieee80211_scan_work+0xd1/0x196 [mac80211]
> Oct 2 17:22:52 larrylap kernel: [<ffffffff802478ef>]
> run_workqueue+0x103/0x20a
> Oct 2 17:22:52 larrylap kernel: [<ffffffff8024789d>] ?
> run_workqueue+0xb1/0x20a
> Oct 2 17:22:52 larrylap kernel: [<ffffffffa01f6d16>] ?
> ieee80211_scan_work+0x0/0x196 [mac80211]
> Oct 2 17:22:52 larrylap kernel: [<ffffffff80247ad6>]
> worker_thread+0xe0/0xf1
> Oct 2 17:22:52 larrylap kernel: [<ffffffff8024b5e0>] ?
> autoremove_wake_function+0x0/0x38
> Oct 2 17:22:53 larrylap kernel: [<ffffffff802479f6>] ?
> worker_thread+0x0/0xf1
> Oct 2 17:22:53 larrylap kernel: [<ffffffff8024b284>] kthread+0x49/0x76
> Oct 2 17:22:53 larrylap kernel: [<ffffffff8020d099>] child_rip+0xa/0x11
> Oct 2 17:22:53 larrylap kernel: [<ffffffff80427834>] ?
> _spin_unlock_irq+0x2b/0x30
> Oct 2 17:22:53 larrylap kernel: [<ffffffff8020c6cf>] ?
> restore_args+0x0/0x30
> Oct 2 17:22:53 larrylap kernel: [<ffffffff8024b23b>] ? kthread+0x0/0x76
> Oct 2 17:22:53 larrylap kernel: [<ffffffff8020d08f>] ?
> child_rip+0x0/0x11
> Oct 2 17:22:53 larrylap kernel:
> Oct 2 17:22:53 larrylap kernel:
> Oct 2 17:22:53 larrylap kernel: Code: 8b 4b 04 44 29 f1 39 d1 0f 42
> ca 4d 85 ed 74 54 8b 45 cc 49 8d 55 40 89 c9 41 89 45 40 44 01 e0 89
> 42 04 48 8b 45 b8 48 89 42 08 <48> 8b 06 49 89 75 08 49 89 45 00 4c 89
> 68 08 4c 89 2e 0f b7 53
> Oct 2 17:22:53 larrylap kernel: RIP [<ffffffffa024916e>]
> p54_assign_address+0xff/0x162 [p54common]
> Oct 2 17:22:53 larrylap kernel: RSP <ffff8800ba0ebd20>
> Oct 2 17:22:53 larrylap kernel: CR2: 0000000000000000
> Oct 2 17:22:53 larrylap kernel: ---[ end trace 4bd18aa5f2aeb5d8 ]---
>
> Note, the "tainted" flag is false. No closed-source drivers have been
> loaded.
>
> The oops occurs in the following inline routine:
>
> static inline void __skb_queue_after(struct sk_buff_head *list,
> struct sk_buff *prev,
> struct sk_buff *newsk)
> {
> __skb_insert(newsk, prev, prev->next, list);
> }
>
> and is called from p54_assign_addresses() in the following region:
>
> if (skb) {
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> struct memrecord *range = (void *)info->driver_data;
> range->start_addr = target_addr;
> range->end_addr = target_addr + len;
> range->dev = dev;
> __skb_queue_after(&priv->tx_queue, target_skb, skb);
> if (largest_hole < priv->rx_mtu + priv->headroom +
> priv->tailroom +
> sizeof(struct p54_control_hdr))
> ieee80211_stop_queues(dev);
> }
>
> Larry
Hmm, just a guess:
according to skbuff.h
the callback buffer in every skb is about;
char cb[48];
now, when we look at what mac80211 puts inside it
struct ieee80211_tx_info {
u32 flags;
u8 band;
s8 tx_rate_idx;
u8 antenna_sel_tx;
/* 1 byte hole => 8 bytes so far */
union {
struct {
struct ieee80211_vif *vif; // another 8 byte on 64bit cpus => 16
struct ieee80211_key_conf *hw_key; // + 8 bytes => 24
struct ieee80211_sta *sta; // + 8 bytes => 32
unsigned long jiffies; // + 8 bytes => 40
s8 rts_cts_rate_idx, alt_retry_rate_idx; // + 2
u8 retry_limit; // + 1
u8 icv_len; // + 1
u8 iv_len; // + 1
} control;
[...]
= 45 Bytes (without alignment, with it it's probably 48) out of 48...
If this is true, we have a serious problem on x64 since the memrecord
struct is about 8 bytes in the old code, but with this patch it's 16... well I am not sure, can I put
the extra ieee80211_hw* thing into skb->dev. It would be nice, but of course net_device isn't
exactly ieee80211_hw, as far as I can see.
But this won't solve the problem where the rest of the 8 bytes in memrecord should go.
I think we should mark p54 as broken for now, since it corrupts a rather huge chunk of the skb's data
structure.
Regards,
Chr
_________________________________________________________________________
In 5 Schritten zur eigenen Homepage. Jetzt Domain sichern und gestalten!
Nur 3,99 EUR/Monat! http://www.maildomain.web.de/?mc=021114
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC][PATCH] p54: fix memory management
2008-10-03 14:16 [RFC][PATCH] p54: fix memory management Chunkeey
@ 2008-10-03 19:25 ` Christian Lamparter
0 siblings, 0 replies; 5+ messages in thread
From: Christian Lamparter @ 2008-10-03 19:25 UTC (permalink / raw)
To: Larry Finger; +Cc: Johannes Berg, Kalle Valo, linux-wireless, John W Linville
On Friday 03 October 2008 16:16:32 Chunkeey@web.de wrote:
> >
> > Note, the "tainted" flag is false. No closed-source drivers have been
> > loaded.
> >
> > The oops occurs in the following inline routine:
> >
> > static inline void __skb_queue_after(struct sk_buff_head *list,
> > struct sk_buff *prev,
> > struct sk_buff *newsk)
> > {
> > __skb_insert(newsk, prev, prev->next, list);
> > }
> >
> > and is called from p54_assign_addresses() in the following region:
> >
> > if (skb) {
> > struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> > struct memrecord *range = (void *)info->driver_data;
> > range->start_addr = target_addr;
> > range->end_addr = target_addr + len;
> > range->dev = dev;
> > __skb_queue_after(&priv->tx_queue, target_skb, skb);
> > if (largest_hole < priv->rx_mtu + priv->headroom +
> > priv->tailroom +
> > sizeof(struct p54_control_hdr))
> > ieee80211_stop_queues(dev);
> > }
> >
> > Larry
>
> Hmm, just a guess:
>
> according to skbuff.h
> the callback buffer in every skb is about;
> char cb[48];
>
>
> now, when we look at what mac80211 puts inside it
> struct ieee80211_tx_info {
> u32 flags;
> u8 band;
> s8 tx_rate_idx;
> u8 antenna_sel_tx;
>
> /* 1 byte hole => 8 bytes so far */
>
> union {
> struct {
> struct ieee80211_vif *vif; // another 8 byte on 64bit cpus => 16
> struct ieee80211_key_conf *hw_key; // + 8 bytes => 24
> struct ieee80211_sta *sta; // + 8 bytes => 32
> unsigned long jiffies; // + 8 bytes => 40
> s8 rts_cts_rate_idx, alt_retry_rate_idx; // + 2
> u8 retry_limit; // + 1
> u8 icv_len; // + 1
> u8 iv_len; // + 1
> } control;
> [...]
>
> = 45 Bytes (without alignment, with it it's probably 48) out of 48...
> If this is true, we have a serious problem on x64 since the memrecord
> struct is about 8 bytes in the old code, but with this patch it's 16...
> well I am not sure, can I put the extra ieee80211_hw* thing into skb->dev.
> It would be nice, but of course net_device isn't exactly ieee80211_hw, as
> far as I can see.
Ahh, that's garbage. driver_data is a member of the union, so it has about 40
bytes which is plenty. sorry for the noise.
Regards,
Chr.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC][PATCH] p54: fix memory management
2008-10-03 2:43 ` Larry Finger
@ 2008-10-03 20:02 ` Christian Lamparter
0 siblings, 0 replies; 5+ messages in thread
From: Christian Lamparter @ 2008-10-03 20:02 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, John W Linville, Johannes Berg, Kalle Valo
On Friday 03 October 2008 04:43:00 Larry Finger wrote:
> Christian Lamparter wrote:
> Oct 2 17:22:52 larrylap kernel: BUG: unable to handle kernel NULL
> pointer dereference at 0000000000000000
[...]
> [...] Tainted: GM
> Note, the "tainted" flag is false. No closed-source drivers have been
> loaded.
>
more about the flags can be found in Documentation/oops-tracing.txt:
1: 'G' if all modules loaded have a GPL or compatible license, 'P' if
any proprietary module has been loaded. Modules without a
MODULE_LICENSE or with a MODULE_LICENSE that is not recognised by
insmod as GPL compatible are assumed to be proprietary.
[...]
5: 'M' if any processor has reported a Machine Check Exception,
' ' if no Machine Check Exceptions have occurred.
so:
- the "G" flag is perfectly OK! (P would be the "closed-source" drivers)
- however, the "M"(CE) flag is worryingly. The question is, if p54 causes it, or not?
As far as I know, when a MCE gets logged a short message should pop up
in the kernel's log and next you'll need extra tools like mcelog to see what's
going wrong.
Regards,
Chr
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-10-03 19:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-03 14:16 [RFC][PATCH] p54: fix memory management Chunkeey
2008-10-03 19:25 ` Christian Lamparter
-- strict thread matches above, loose matches on Subject: below --
2008-10-02 1:59 Christian Lamparter
2008-10-03 2:43 ` Larry Finger
2008-10-03 20:02 ` Christian Lamparter
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).