Re: [PATCH] ipw2200 stack reduction

Jens Axboe
2005-12-29T11:12:32Z

On Thu, Dec 29 2005, Pekka Enberg wrote:
> Hi,
>
> On 12/29/05, Jens Axboe <axboe@suse.de> wrote:
> > Well you could do that if you wanted, but 500 bytes of dynamic
> > allocation is not a big issue. But it could be an optimization on top of
> > this patch, indeed. The downside is that you then have to do 2
> > allocations for each command, so whether it would be a win or not I
> > don't know.
>
> The allocation shouldn't make much difference but for the implicit
> memset() smaller size is a win maybe?

Good point. With the payload dynamic, the host_cmd is really small (only
8 bytes). If we put that back on the stack, we can just use the buffer
that the caller passes in. Net result: zero extra allocations.

Applied on top of the two other patches. Zhu, let me know if you just
want 1 patch instead. It is just one change, so...

Signed-off-by: Jens Axboe <axboe@suse.de>

--- linux-2.6.git/drivers/net/wireless/ipw2200.c~ 2005-12-29 11:46:46.000000000 +0100
+++ linux-2.6.git/drivers/net/wireless/ipw2200.c 2005-12-29 12:11:33.000000000 +0100
@@ -1871,10 +1871,6 @@

#define HOST_COMPLETE_TIMEOUT HZ

-/*
- * Note: this function frees the cmd, so it must not be touched by the callee
- * after submitting it.
- */
static int __ipw_send_cmd(struct ipw_priv *priv, struct host_cmd *cmd)
{
int rc = 0;
@@ -1885,8 +1881,7 @@
IPW_ERROR("Failed to send %s: Already sending a command.\n",
get_cmd_string(cmd->cmd));
spin_unlock_irqrestore(&priv->lock, flags);
- rc = -EAGAIN;
- goto out;
+ return -EAGAIN;
}

priv->status |= STATUS_HCMD_ACTIVE;
@@ -1905,7 +1900,7 @@
priv->status);
printk_buf(IPW_DL_HOST_COMMAND, (u8 *) cmd->param, cmd->len);

- rc = ipw_queue_tx_hcmd(priv, cmd->cmd, &cmd->param, cmd->len, 0);
+ rc = ipw_queue_tx_hcmd(priv, cmd->cmd, cmd->param, cmd->len, 0);
if (rc) {
priv->status &= ~STATUS_HCMD_ACTIVE;
IPW_ERROR("Failed to send %s: Reason %d\n",
@@ -1945,49 +1940,28 @@
priv->cmdlog[priv->cmdlog_pos++].retcode = rc;
priv->cmdlog_pos %= priv->cmdlog_len;
}
-out:
- kfree(cmd);
return rc;
}

-static struct host_cmd *ipw_host_cmd_get(u8 command, u8 len)
-{
- struct host_cmd *cmd;
-
- cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
- if (cmd) {
- cmd->cmd = command;
- cmd->len = len;
- }
-
- return cmd;
-}
-
static int ipw_send_cmd_simple(struct ipw_priv *priv, u8 command)
{
- struct host_cmd *cmd;
-
- cmd = ipw_host_cmd_get(command, 0);
- if (cmd)
- return __ipw_send_cmd(priv, cmd);
+ struct host_cmd cmd = {
+ .cmd = command,
+ };

- IPW_ERROR("Out of memory for cmd %x\n", command);
- return -1;
+ return __ipw_send_cmd(priv, &cmd);
}

static int ipw_send_cmd_pdu(struct ipw_priv *priv, u8 command, u8 len,
void *data)
{
- struct host_cmd *cmd;
-
- cmd = ipw_host_cmd_get(command, len);
- if (cmd) {
- memcpy(cmd->param, data, len);
- return __ipw_send_cmd(priv, cmd);
- }
+ struct host_cmd cmd = {
+ .cmd = command,
+ .len = len,
+ .param = data,
+ };

- IPW_ERROR("Out of memory for cmd %x\n", command);
- return -1;
+ return __ipw_send_cmd(priv, &cmd);
}

static int ipw_send_host_complete(struct ipw_priv *priv)
diff --git a/drivers/net/wireless/ipw2200.h b/drivers/net/wireless/ipw2200.h
index 1c98db0..dab1dcd 100644
--- a/drivers/net/wireless/ipw2200.h
+++ b/drivers/net/wireless/ipw2200.h
@@ -1860,7 +1860,7 @@ struct host_cmd {
u8 cmd;
u8 len;
u16 reserved;
- u32 param[TFD_CMD_IMMEDIATE_PAYLOAD_LENGTH];
+ u32 *param;
} __attribute__ ((packed));

struct ipw_cmd_log {

--
Jens Axboe