Re: rc9 + orinoco WPA patchset: BUG: scheduling while atomic loading firmware with PCMCIA adapter

[Andrey Borzenkov]

On Friday 10 October 2008, Dave wrote:
> Thanks for identifying the problem. The Agere case looks good - a few suggestions for the Symbol case though:

Updated patch attached

>  
> But you absolutely have to kfree(pda) here.

Yes, I forgot; sorry.

> Dan Williams wrote:
> > maybe you should not use priv->pda_size but
> > #define SYMBOL_PDA_SIZE 256 and use that for the hermes_read_pda()
> > length just to ensure the patched code is functionally the same as
> > before the patch.
> 
> Using fw->pda_size should be fine. The value comes from a const static, set to 0x100 for Symbol.
> 

Actually it allocated 512 byte before. Should it probably be set to 0x200?
Subject: [PATCH] orinoco: reduce stack usage in firmware download path
From: Andrey Borzenkov <arvidjaar@xxxxxxx>

orinoco_dl_firmware and symbol_dl_mage allocate large local
variables (1K); at least orinoco fails with panic or hung
kernel if 4K stacks is enabled.

Allocate large buffers dynamically at run time.

Tested-By: Andrey Borzenkov <arvidjaar@xxxxxxx> for Agere case
Signed-off-by: Andrey Borzenkov <arvidjaar@xxxxxxx>

---

 drivers/net/wireless/orinoco.c |   40 ++++++++++++++++++++++++++++------------
 1 files changed, 28 insertions(+), 12 deletions(-)


diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c
index 9a2fcc0..e225fec 100644
--- a/drivers/net/wireless/orinoco.c
+++ b/drivers/net/wireless/orinoco.c
@@ -458,7 +458,7 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 		    int ap)
 {
 	/* Plug Data Area (PDA) */
-	__le16 pda[512] = { 0 };
+	__le16 *pda;
 
 	hermes_t *hw = &priv->hw;
 	const struct firmware *fw_entry;
@@ -467,7 +467,11 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 	const unsigned char *end;
 	const char *firmware;
 	struct net_device *dev = priv->ndev;
-	int err;
+	int err = 0;
+
+	pda = kzalloc(fw->pda_size, GFP_KERNEL);
+	if (!pda)
+		return -ENOMEM;
 
 	if (ap)
 		firmware = fw->ap_fw;
@@ -478,17 +482,17 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 	       dev->name, firmware);
 
 	/* Read current plug data */
-	err = hermes_read_pda(hw, pda, fw->pda_addr,
-			      min_t(u16, fw->pda_size, sizeof(pda)), 0);
+	err = hermes_read_pda(hw, pda, fw->pda_addr, fw->pda_size, 0);
 	printk(KERN_DEBUG "%s: Read PDA returned %d\n", dev->name, err);
 	if (err)
-		return err;
+		goto free;
 
 	err = request_firmware(&fw_entry, firmware, priv->dev);
 	if (err) {
 		printk(KERN_ERR "%s: Cannot find firmware %s\n",
 		       dev->name, firmware);
-		return -ENOENT;
+		err = -ENOENT;
+		goto free;
 	}
 
 	hdr = (const struct orinoco_fw_header *) fw_entry->data;
@@ -532,6 +536,9 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 
 abort:
 	release_firmware(fw_entry);
+
+free:
+	kfree(pda);
 	return err;
 }
 
@@ -549,12 +556,12 @@ symbol_dl_image(struct orinoco_private *priv, const struct fw_info *fw,
 		int secondary)
 {
 	hermes_t *hw = &priv->hw;
-	int ret;
+	int ret = 0;
 	const unsigned char *ptr;
 	const unsigned char *first_block;
 
 	/* Plug Data Area (PDA) */
-	__le16 pda[256];
+	__le16 *pda = NULL;
 
 	/* Binary block begins after the 0x1A marker */
 	ptr = image;
@@ -563,28 +570,33 @@ symbol_dl_image(struct orinoco_private *priv, const struct fw_info *fw,
 
 	/* Read the PDA from EEPROM */
 	if (secondary) {
-		ret = hermes_read_pda(hw, pda, fw->pda_addr, sizeof(pda), 1);
+		pda = kzalloc(fw->pda_size, GFP_KERNEL);
+		if (!pda)
+			return -ENOMEM;
+
+		ret = hermes_read_pda(hw, pda, fw->pda_addr, fw->pda_size, 1);
 		if (ret)
-			return ret;
+			goto free;
 	}
 
 	/* Stop the firmware, so that it can be safely rewritten */
 	if (priv->stop_fw) {
 		ret = priv->stop_fw(priv, 1);
 		if (ret)
-			return ret;
+			goto free;
 	}
 
 	/* Program the adapter with new firmware */
 	ret = hermes_program(hw, first_block, end);
 	if (ret)
-		return ret;
+		goto free;
 
 	/* Write the PDA to the adapter */
 	if (secondary) {
 		size_t len = hermes_blocks_length(first_block);
 		ptr = first_block + len;
 		ret = hermes_apply_pda(hw, ptr, pda);
+		kfree(pda);
 		if (ret)
 			return ret;
 	}
@@ -608,6 +620,10 @@ symbol_dl_image(struct orinoco_private *priv, const struct fw_info *fw,
 		return -ENODEV;
 
 	return 0;
+
+free:
+	kfree(pda);
+	return ret;
 }

Attachment: signature.asc
Description: This is a digitally signed message part.