2005-12-28 21:27:54

by Jens Axboe

[permalink] [raw]
Subject: [PATCH] ipw2200 stack reduction

Hi,

Checking the stack usage of my kernel, showed that ipw2200 had a few bad
offenders. This is on i386 32-bit:

0x00002876 ipw_send_associate: 544
0x000028ee ipw_send_associate: 544
0x000027dc ipw_send_scan_request_ext: 520
0x00002864 ipw_set_sensitivity: 520
0x00005eac ipw_set_rsn_capa: 520

The reason is the host_cmd structure is large (500 bytes). All other
functions currently using ipw_send_cmd() suffer from the same problem.
This patch introduces ipw_send_cmd_simple() for commands with no data
transfer, and ipw_send_cmd_pdu() for commands with a data payload.

As an added bonus, the diffstat looks like this:

ipw2200.c | 285 +++++++++++++++++++++++++-------------------------------------
1 files changed, 115 insertions(+), 170 deletions(-)

and it shrinks the module a lot as well:

Before:

text data bss dec hex filename
75177 2472 44 77693 12f7d drivers/net/wireless/ipw2200.ko

After:

text data bss dec hex filename
65731 2472 44 68247 10a97 drivers/net/wireless/ipw2200.ko

(lsmod size is shrunk ~9kb, or about 11%).

Signed-off-by: Jens Axboe <[email protected]>

diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index 5e7c7e9..9ce659b 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -1870,7 +1870,7 @@ static char *get_cmd_string(u8 cmd)
}

#define HOST_COMPLETE_TIMEOUT HZ
-static int ipw_send_cmd(struct ipw_priv *priv, struct host_cmd *cmd)
+static int __ipw_send_cmd(struct ipw_priv *priv, struct host_cmd *cmd, int put)
{
int rc = 0;
unsigned long flags;
@@ -1880,6 +1880,8 @@ static int ipw_send_cmd(struct ipw_priv
IPW_ERROR("Failed to send %s: Already sending a command.\n",
get_cmd_string(cmd->cmd));
spin_unlock_irqrestore(&priv->lock, flags);
+ if (put)
+ kfree(cmd);
return -EAGAIN;
}

@@ -1934,69 +1936,92 @@ static int ipw_send_cmd(struct ipw_priv
goto exit;
}

- exit:
+exit:
if (priv->cmdlog) {
priv->cmdlog[priv->cmdlog_pos++].retcode = rc;
priv->cmdlog_pos %= priv->cmdlog_len;
}
+ if (put)
+ kfree(cmd);
return rc;
}

-static int ipw_send_host_complete(struct ipw_priv *priv)
+static struct host_cmd *ipw_host_cmd_get(u8 command, u8 len)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_HOST_COMPLETE,
- .len = 0
- };
+ struct host_cmd *cmd;
+
+ cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+ if (cmd) {
+ cmd->cmd = command;
+ cmd->len = len;
+ return cmd;
+ }
+
+ return NULL;
+}
+
+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, 1);

+ IPW_ERROR("Out of memory for cmd %x\n", command);
+ return -1;
+}
+
+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, 1);
+ }
+
+ IPW_ERROR("Out of memory for cmd %x\n", command);
+ return -1;
+}
+
+static int ipw_send_host_complete(struct ipw_priv *priv)
+{
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_simple(priv, IPW_CMD_HOST_COMPLETE);
}

static int ipw_send_system_config(struct ipw_priv *priv,
struct ipw_sys_config *config)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SYSTEM_CONFIG,
- .len = sizeof(*config)
- };
-
if (!priv || !config) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, config, sizeof(*config));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SYSTEM_CONFIG, sizeof(*config),
+ config);
}

static int ipw_send_ssid(struct ipw_priv *priv, u8 * ssid, int len)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SSID,
- .len = min(len, IW_ESSID_MAX_SIZE)
- };
-
if (!priv || !ssid) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, ssid, cmd.len);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SSID, min(len, IW_ESSID_MAX_SIZE),
+ ssid);
}

static int ipw_send_adapter_address(struct ipw_priv *priv, u8 * mac)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_ADAPTER_ADDRESS,
- .len = ETH_ALEN
- };
-
if (!priv || !mac) {
IPW_ERROR("Invalid args\n");
return -1;
@@ -2005,8 +2030,8 @@ static int ipw_send_adapter_address(stru
IPW_DEBUG_INFO("%s: Setting MAC to " MAC_FMT "\n",
priv->net_dev->name, MAC_ARG(mac));

- memcpy(cmd.param, mac, ETH_ALEN);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_ADAPTER_ADDRESS, ETH_ALEN,
+ mac);
}

/*
@@ -2065,51 +2090,40 @@ static void ipw_bg_scan_check(void *data
static int ipw_send_scan_request_ext(struct ipw_priv *priv,
struct ipw_scan_request_ext *request)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SCAN_REQUEST_EXT,
- .len = sizeof(*request)
- };
-
- memcpy(cmd.param, request, sizeof(*request));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SCAN_REQUEST_EXT,
+ sizeof(*request), request);
}

static int ipw_send_scan_abort(struct ipw_priv *priv)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SCAN_ABORT,
- .len = 0
- };
-
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_simple(priv, IPW_CMD_SCAN_ABORT);
}

static int ipw_set_sensitivity(struct ipw_priv *priv, u16 sens)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SENSITIVITY_CALIB,
- .len = sizeof(struct ipw_sensitivity_calib)
+ struct ipw_sensitivity_calib calib = {
+ .beacon_rssi_raw = sens,
};
- struct ipw_sensitivity_calib *calib = (struct ipw_sensitivity_calib *)
- &cmd.param;
- calib->beacon_rssi_raw = sens;
- return ipw_send_cmd(priv, &cmd);
+
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SENSITIVITY_CALIB, sizeof(calib),
+ &calib);
}

static int ipw_send_associate(struct ipw_priv *priv,
struct ipw_associate *associate)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_ASSOCIATE,
- .len = sizeof(*associate)
- };
-
struct ipw_associate tmp_associate;
+
+ if (!priv || !associate) {
+ IPW_ERROR("Invalid args\n");
+ return -1;
+ }
+
memcpy(&tmp_associate, associate, sizeof(*associate));
tmp_associate.policy_support =
cpu_to_le16(tmp_associate.policy_support);
@@ -2122,80 +2136,56 @@ static int ipw_send_associate(struct ipw
cpu_to_le16(tmp_associate.beacon_interval);
tmp_associate.atim_window = cpu_to_le16(tmp_associate.atim_window);

- if (!priv || !associate) {
- IPW_ERROR("Invalid args\n");
- return -1;
- }
-
- memcpy(cmd.param, &tmp_associate, sizeof(*associate));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_ASSOCIATE, sizeof(tmp_associate),
+ &tmp_associate);
}

static int ipw_send_supported_rates(struct ipw_priv *priv,
struct ipw_supported_rates *rates)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SUPPORTED_RATES,
- .len = sizeof(*rates)
- };
-
if (!priv || !rates) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, rates, sizeof(*rates));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SUPPORTED_RATES, sizeof(*rates),
+ rates);
}

static int ipw_set_random_seed(struct ipw_priv *priv)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SEED_NUMBER,
- .len = sizeof(u32)
- };
+ u32 val;

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- get_random_bytes(&cmd.param, sizeof(u32));
+ get_random_bytes(&val, sizeof(val));

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SEED_NUMBER, sizeof(val), &val);
}

static int ipw_send_card_disable(struct ipw_priv *priv, u32 phy_off)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_CARD_DISABLE,
- .len = sizeof(u32)
- };
-
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- *((u32 *) & cmd.param) = phy_off;
-
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_CARD_DISABLE, sizeof(phy_off),
+ &phy_off);
}

static int ipw_send_tx_power(struct ipw_priv *priv, struct ipw_tx_power *power)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_TX_POWER,
- .len = sizeof(*power)
- };
-
if (!priv || !power) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, power, sizeof(*power));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_TX_POWER, sizeof(*power),
+ power);
}

static int ipw_set_tx_power(struct ipw_priv *priv)
@@ -2247,18 +2237,14 @@ static int ipw_send_rts_threshold(struct
struct ipw_rts_threshold rts_threshold = {
.rts_threshold = rts,
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RTS_THRESHOLD,
- .len = sizeof(rts_threshold)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &rts_threshold, sizeof(rts_threshold));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RTS_THRESHOLD,
+ sizeof(rts_threshold), &rts_threshold);
}

static int ipw_send_frag_threshold(struct ipw_priv *priv, u16 frag)
@@ -2266,27 +2252,19 @@ static int ipw_send_frag_threshold(struc
struct ipw_frag_threshold frag_threshold = {
.frag_threshold = frag,
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_FRAG_THRESHOLD,
- .len = sizeof(frag_threshold)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &frag_threshold, sizeof(frag_threshold));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_FRAG_THRESHOLD,
+ sizeof(frag_threshold), &frag_threshold);
}

static int ipw_send_power_mode(struct ipw_priv *priv, u32 mode)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_POWER_MODE,
- .len = sizeof(u32)
- };
- u32 *param = (u32 *) (&cmd.param);
+ u32 param;

if (!priv) {
IPW_ERROR("Invalid args\n");
@@ -2297,17 +2275,18 @@ static int ipw_send_power_mode(struct ip
* level */
switch (mode) {
case IPW_POWER_BATTERY:
- *param = IPW_POWER_INDEX_3;
+ param = IPW_POWER_INDEX_3;
break;
case IPW_POWER_AC:
- *param = IPW_POWER_MODE_CAM;
+ param = IPW_POWER_MODE_CAM;
break;
default:
- *param = mode;
+ param = mode;
break;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_POWER_MODE, sizeof(param),
+ &param);
}

static int ipw_send_retry_limit(struct ipw_priv *priv, u8 slimit, u8 llimit)
@@ -2316,18 +2295,14 @@ static int ipw_send_retry_limit(struct i
.short_retry_limit = slimit,
.long_retry_limit = llimit
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RETRY_LIMIT,
- .len = sizeof(retry_limit)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &retry_limit, sizeof(retry_limit));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RETRY_LIMIT, sizeof(retry_limit),
+ &retry_limit);
}

/*
@@ -5672,54 +5647,44 @@ static void ipw_adhoc_create(struct ipw_

static void ipw_send_tgi_tx_key(struct ipw_priv *priv, int type, int index)
{
- struct ipw_tgi_tx_key *key;
- struct host_cmd cmd = {
- .cmd = IPW_CMD_TGI_TX_KEY,
- .len = sizeof(*key)
- };
+ struct ipw_tgi_tx_key key;

if (!(priv->ieee->sec.flags & (1 << index)))
return;

- key = (struct ipw_tgi_tx_key *)&cmd.param;
- key->key_id = index;
- memcpy(key->key, priv->ieee->sec.keys[index], SCM_TEMPORAL_KEY_LENGTH);
- key->security_type = type;
- key->station_index = 0; /* always 0 for BSS */
- key->flags = 0;
+ key.key_id = index;
+ memcpy(key.key, priv->ieee->sec.keys[index], SCM_TEMPORAL_KEY_LENGTH);
+ key.security_type = type;
+ key.station_index = 0; /* always 0 for BSS */
+ key.flags = 0;
/* 0 for new key; previous value of counter (after fatal error) */
- key->tx_counter[0] = 0;
- key->tx_counter[1] = 0;
+ key.tx_counter[0] = 0;
+ key.tx_counter[1] = 0;

- ipw_send_cmd(priv, &cmd);
+ ipw_send_cmd_pdu(priv, IPW_CMD_TGI_TX_KEY, sizeof(key), &key);
}

static void ipw_send_wep_keys(struct ipw_priv *priv, int type)
{
- struct ipw_wep_key *key;
+ struct ipw_wep_key key;
int i;
- struct host_cmd cmd = {
- .cmd = IPW_CMD_WEP_KEY,
- .len = sizeof(*key)
- };

- key = (struct ipw_wep_key *)&cmd.param;
- key->cmd_id = DINO_CMD_WEP_KEY;
- key->seq_num = 0;
+ key.cmd_id = DINO_CMD_WEP_KEY;
+ key.seq_num = 0;

/* Note: AES keys cannot be set for multiple times.
* Only set it at the first time. */
for (i = 0; i < 4; i++) {
- key->key_index = i | type;
+ key.key_index = i | type;
if (!(priv->ieee->sec.flags & (1 << i))) {
- key->key_size = 0;
+ key.key_size = 0;
continue;
}

- key->key_size = priv->ieee->sec.key_sizes[i];
- memcpy(key->key, priv->ieee->sec.keys[i], key->key_size);
+ key.key_size = priv->ieee->sec.key_sizes[i];
+ memcpy(key.key, priv->ieee->sec.keys[i], key.key_size);

- ipw_send_cmd(priv, &cmd);
+ ipw_send_cmd_pdu(priv, IPW_CMD_WEP_KEY, sizeof(key), &key);
}
}

@@ -6216,15 +6181,10 @@ void ipw_wpa_assoc_frame(struct ipw_priv
static int ipw_set_rsn_capa(struct ipw_priv *priv,
char *capabilities, int length)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RSN_CAPABILITIES,
- .len = length,
- };
-
IPW_DEBUG_HC("HOST_CMD_RSN_CAPABILITIES\n");

- memcpy(cmd.param, capabilities, length);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RSN_CAPABILITIES, length,
+ capabilities);
}

/*
@@ -7011,25 +6971,15 @@ static int ipw_handle_assoc_response(str
static int ipw_send_qos_params_command(struct ipw_priv *priv, struct ieee80211_qos_parameters
*qos_param)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_QOS_PARAMETERS,
- .len = (sizeof(struct ieee80211_qos_parameters) * 3)
- };
-
- memcpy(cmd.param, qos_param, sizeof(*qos_param) * 3);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_QOS_PARAMETERS, qos_param,
+ sizeof(*qos_param) * 3);
}

static int ipw_send_qos_info_command(struct ipw_priv *priv, struct ieee80211_qos_information_element
*qos_param)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_WME_INFO,
- .len = sizeof(*qos_param)
- };
-
- memcpy(cmd.param, qos_param, sizeof(*qos_param));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_WME_INFO, qos_param,
+ sizeof(*qos_param));
}

#endif /* CONFIG_IPW_QOS */

--
Jens Axboe


2005-12-29 08:48:52

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] ipw2200 stack reduction

Hi Jens,

On 12/28/05, Jens Axboe <[email protected]> wrote:
> -static int ipw_send_host_complete(struct ipw_priv *priv)
> +static struct host_cmd *ipw_host_cmd_get(u8 command, u8 len)
> {
> - struct host_cmd cmd = {
> - .cmd = IPW_CMD_HOST_COMPLETE,
> - .len = 0
> - };
> + struct host_cmd *cmd;
> +
> + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> + if (cmd) {
> + cmd->cmd = command;
> + cmd->len = len;
> + return cmd;
> + }
> +
> + return NULL;

A minor nitpick: you can always return cmd here.

Pekka

2005-12-29 08:58:36

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] ipw2200 stack reduction

On Thu, Dec 29 2005, Pekka Enberg wrote:
> Hi Jens,
>
> On 12/28/05, Jens Axboe <[email protected]> wrote:
> > -static int ipw_send_host_complete(struct ipw_priv *priv)
> > +static struct host_cmd *ipw_host_cmd_get(u8 command, u8 len)
> > {
> > - struct host_cmd cmd = {
> > - .cmd = IPW_CMD_HOST_COMPLETE,
> > - .len = 0
> > - };
> > + struct host_cmd *cmd;
> > +
> > + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> > + if (cmd) {
> > + cmd->cmd = command;
> > + cmd->len = len;
> > + return cmd;
> > + }
> > +
> > + return NULL;
>
> A minor nitpick: you can always return cmd here.

Yeah, I also forgot to kill the 'put' parameter to __ipw_cmd_send() as
well. Updated patch below.

---

Checking the stack usage of my kernel, showed that ipw2200 had a few bad
offenders. This is on i386 32-bit:

0x00002876 ipw_send_associate: 544
0x000028ee ipw_send_associate: 544
0x000027dc ipw_send_scan_request_ext: 520
0x00002864 ipw_set_sensitivity: 520
0x00005eac ipw_set_rsn_capa: 520

The reason is the host_cmd structure is large (500 bytes). All other
functions currently using ipw_send_cmd() suffer from the same problem.
This patch introduces ipw_send_cmd_simple() for commands with no data
transfer, and ipw_send_cmd_pdu() for commands with a data payload.

As an added bonus, the diffstat looks like this:

ipw2200.c | 285 +++++++++++++++++++++++++-------------------------------------
1 files changed, 115 insertions(+), 170 deletions(-)

and it shrinks the module a lot as well:

Before:

text data bss dec hex filename
75177 2472 44 77693 12f7d drivers/net/wireless/ipw2200.ko

After:

text data bss dec hex filename
65731 2472 44 68247 10a97 drivers/net/wireless/ipw2200.ko

(lsmod size is shrunk ~9kb, or about 11%).

Signed-off-by: Jens Axboe <[email protected]>

diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -1870,7 +1870,12 @@ static char *get_cmd_string(u8 cmd)
}

#define HOST_COMPLETE_TIMEOUT HZ
-static int ipw_send_cmd(struct ipw_priv *priv, struct host_cmd *cmd)
+
+/*
+ * 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;
unsigned long flags;
@@ -1880,7 +1885,8 @@ static int ipw_send_cmd(struct ipw_priv
IPW_ERROR("Failed to send %s: Already sending a command.\n",
get_cmd_string(cmd->cmd));
spin_unlock_irqrestore(&priv->lock, flags);
- return -EAGAIN;
+ rc = -EAGAIN;
+ goto out;
}

priv->status |= STATUS_HCMD_ACTIVE;
@@ -1934,69 +1940,91 @@ static int ipw_send_cmd(struct ipw_priv
goto exit;
}

- exit:
+exit:
if (priv->cmdlog) {
priv->cmdlog[priv->cmdlog_pos++].retcode = rc;
priv->cmdlog_pos %= priv->cmdlog_len;
}
+out:
+ kfree(cmd);
return rc;
}

-static int ipw_send_host_complete(struct ipw_priv *priv)
+static struct host_cmd *ipw_host_cmd_get(u8 command, u8 len)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_HOST_COMPLETE,
- .len = 0
- };
+ 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);
+
+ IPW_ERROR("Out of memory for cmd %x\n", command);
+ return -1;
+}
+
+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);
+ }
+
+ IPW_ERROR("Out of memory for cmd %x\n", command);
+ return -1;
+}
+
+static int ipw_send_host_complete(struct ipw_priv *priv)
+{
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_simple(priv, IPW_CMD_HOST_COMPLETE);
}

static int ipw_send_system_config(struct ipw_priv *priv,
struct ipw_sys_config *config)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SYSTEM_CONFIG,
- .len = sizeof(*config)
- };
-
if (!priv || !config) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, config, sizeof(*config));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SYSTEM_CONFIG, sizeof(*config),
+ config);
}

static int ipw_send_ssid(struct ipw_priv *priv, u8 * ssid, int len)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SSID,
- .len = min(len, IW_ESSID_MAX_SIZE)
- };
-
if (!priv || !ssid) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, ssid, cmd.len);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SSID, min(len, IW_ESSID_MAX_SIZE),
+ ssid);
}

static int ipw_send_adapter_address(struct ipw_priv *priv, u8 * mac)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_ADAPTER_ADDRESS,
- .len = ETH_ALEN
- };
-
if (!priv || !mac) {
IPW_ERROR("Invalid args\n");
return -1;
@@ -2005,8 +2033,8 @@ static int ipw_send_adapter_address(stru
IPW_DEBUG_INFO("%s: Setting MAC to " MAC_FMT "\n",
priv->net_dev->name, MAC_ARG(mac));

- memcpy(cmd.param, mac, ETH_ALEN);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_ADAPTER_ADDRESS, ETH_ALEN,
+ mac);
}

/*
@@ -2065,51 +2093,40 @@ static void ipw_bg_scan_check(void *data
static int ipw_send_scan_request_ext(struct ipw_priv *priv,
struct ipw_scan_request_ext *request)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SCAN_REQUEST_EXT,
- .len = sizeof(*request)
- };
-
- memcpy(cmd.param, request, sizeof(*request));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SCAN_REQUEST_EXT,
+ sizeof(*request), request);
}

static int ipw_send_scan_abort(struct ipw_priv *priv)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SCAN_ABORT,
- .len = 0
- };
-
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_simple(priv, IPW_CMD_SCAN_ABORT);
}

static int ipw_set_sensitivity(struct ipw_priv *priv, u16 sens)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SENSITIVITY_CALIB,
- .len = sizeof(struct ipw_sensitivity_calib)
+ struct ipw_sensitivity_calib calib = {
+ .beacon_rssi_raw = sens,
};
- struct ipw_sensitivity_calib *calib = (struct ipw_sensitivity_calib *)
- &cmd.param;
- calib->beacon_rssi_raw = sens;
- return ipw_send_cmd(priv, &cmd);
+
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SENSITIVITY_CALIB, sizeof(calib),
+ &calib);
}

static int ipw_send_associate(struct ipw_priv *priv,
struct ipw_associate *associate)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_ASSOCIATE,
- .len = sizeof(*associate)
- };
-
struct ipw_associate tmp_associate;
+
+ if (!priv || !associate) {
+ IPW_ERROR("Invalid args\n");
+ return -1;
+ }
+
memcpy(&tmp_associate, associate, sizeof(*associate));
tmp_associate.policy_support =
cpu_to_le16(tmp_associate.policy_support);
@@ -2122,80 +2139,56 @@ static int ipw_send_associate(struct ipw
cpu_to_le16(tmp_associate.beacon_interval);
tmp_associate.atim_window = cpu_to_le16(tmp_associate.atim_window);

- if (!priv || !associate) {
- IPW_ERROR("Invalid args\n");
- return -1;
- }
-
- memcpy(cmd.param, &tmp_associate, sizeof(*associate));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_ASSOCIATE, sizeof(tmp_associate),
+ &tmp_associate);
}

static int ipw_send_supported_rates(struct ipw_priv *priv,
struct ipw_supported_rates *rates)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SUPPORTED_RATES,
- .len = sizeof(*rates)
- };
-
if (!priv || !rates) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, rates, sizeof(*rates));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SUPPORTED_RATES, sizeof(*rates),
+ rates);
}

static int ipw_set_random_seed(struct ipw_priv *priv)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SEED_NUMBER,
- .len = sizeof(u32)
- };
+ u32 val;

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- get_random_bytes(&cmd.param, sizeof(u32));
+ get_random_bytes(&val, sizeof(val));

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SEED_NUMBER, sizeof(val), &val);
}

static int ipw_send_card_disable(struct ipw_priv *priv, u32 phy_off)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_CARD_DISABLE,
- .len = sizeof(u32)
- };
-
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- *((u32 *) & cmd.param) = phy_off;
-
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_CARD_DISABLE, sizeof(phy_off),
+ &phy_off);
}

static int ipw_send_tx_power(struct ipw_priv *priv, struct ipw_tx_power *power)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_TX_POWER,
- .len = sizeof(*power)
- };
-
if (!priv || !power) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, power, sizeof(*power));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_TX_POWER, sizeof(*power),
+ power);
}

static int ipw_set_tx_power(struct ipw_priv *priv)
@@ -2247,18 +2240,14 @@ static int ipw_send_rts_threshold(struct
struct ipw_rts_threshold rts_threshold = {
.rts_threshold = rts,
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RTS_THRESHOLD,
- .len = sizeof(rts_threshold)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &rts_threshold, sizeof(rts_threshold));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RTS_THRESHOLD,
+ sizeof(rts_threshold), &rts_threshold);
}

static int ipw_send_frag_threshold(struct ipw_priv *priv, u16 frag)
@@ -2266,27 +2255,19 @@ static int ipw_send_frag_threshold(struc
struct ipw_frag_threshold frag_threshold = {
.frag_threshold = frag,
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_FRAG_THRESHOLD,
- .len = sizeof(frag_threshold)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &frag_threshold, sizeof(frag_threshold));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_FRAG_THRESHOLD,
+ sizeof(frag_threshold), &frag_threshold);
}

static int ipw_send_power_mode(struct ipw_priv *priv, u32 mode)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_POWER_MODE,
- .len = sizeof(u32)
- };
- u32 *param = (u32 *) (&cmd.param);
+ u32 param;

if (!priv) {
IPW_ERROR("Invalid args\n");
@@ -2297,17 +2278,18 @@ static int ipw_send_power_mode(struct ip
* level */
switch (mode) {
case IPW_POWER_BATTERY:
- *param = IPW_POWER_INDEX_3;
+ param = IPW_POWER_INDEX_3;
break;
case IPW_POWER_AC:
- *param = IPW_POWER_MODE_CAM;
+ param = IPW_POWER_MODE_CAM;
break;
default:
- *param = mode;
+ param = mode;
break;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_POWER_MODE, sizeof(param),
+ &param);
}

static int ipw_send_retry_limit(struct ipw_priv *priv, u8 slimit, u8 llimit)
@@ -2316,18 +2298,14 @@ static int ipw_send_retry_limit(struct i
.short_retry_limit = slimit,
.long_retry_limit = llimit
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RETRY_LIMIT,
- .len = sizeof(retry_limit)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &retry_limit, sizeof(retry_limit));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RETRY_LIMIT, sizeof(retry_limit),
+ &retry_limit);
}

/*
@@ -5672,54 +5650,44 @@ static void ipw_adhoc_create(struct ipw_

static void ipw_send_tgi_tx_key(struct ipw_priv *priv, int type, int index)
{
- struct ipw_tgi_tx_key *key;
- struct host_cmd cmd = {
- .cmd = IPW_CMD_TGI_TX_KEY,
- .len = sizeof(*key)
- };
+ struct ipw_tgi_tx_key key;

if (!(priv->ieee->sec.flags & (1 << index)))
return;

- key = (struct ipw_tgi_tx_key *)&cmd.param;
- key->key_id = index;
- memcpy(key->key, priv->ieee->sec.keys[index], SCM_TEMPORAL_KEY_LENGTH);
- key->security_type = type;
- key->station_index = 0; /* always 0 for BSS */
- key->flags = 0;
+ key.key_id = index;
+ memcpy(key.key, priv->ieee->sec.keys[index], SCM_TEMPORAL_KEY_LENGTH);
+ key.security_type = type;
+ key.station_index = 0; /* always 0 for BSS */
+ key.flags = 0;
/* 0 for new key; previous value of counter (after fatal error) */
- key->tx_counter[0] = 0;
- key->tx_counter[1] = 0;
+ key.tx_counter[0] = 0;
+ key.tx_counter[1] = 0;

- ipw_send_cmd(priv, &cmd);
+ ipw_send_cmd_pdu(priv, IPW_CMD_TGI_TX_KEY, sizeof(key), &key);
}

static void ipw_send_wep_keys(struct ipw_priv *priv, int type)
{
- struct ipw_wep_key *key;
+ struct ipw_wep_key key;
int i;
- struct host_cmd cmd = {
- .cmd = IPW_CMD_WEP_KEY,
- .len = sizeof(*key)
- };

- key = (struct ipw_wep_key *)&cmd.param;
- key->cmd_id = DINO_CMD_WEP_KEY;
- key->seq_num = 0;
+ key.cmd_id = DINO_CMD_WEP_KEY;
+ key.seq_num = 0;

/* Note: AES keys cannot be set for multiple times.
* Only set it at the first time. */
for (i = 0; i < 4; i++) {
- key->key_index = i | type;
+ key.key_index = i | type;
if (!(priv->ieee->sec.flags & (1 << i))) {
- key->key_size = 0;
+ key.key_size = 0;
continue;
}

- key->key_size = priv->ieee->sec.key_sizes[i];
- memcpy(key->key, priv->ieee->sec.keys[i], key->key_size);
+ key.key_size = priv->ieee->sec.key_sizes[i];
+ memcpy(key.key, priv->ieee->sec.keys[i], key.key_size);

- ipw_send_cmd(priv, &cmd);
+ ipw_send_cmd_pdu(priv, IPW_CMD_WEP_KEY, sizeof(key), &key);
}
}

@@ -6216,15 +6184,10 @@ void ipw_wpa_assoc_frame(struct ipw_priv
static int ipw_set_rsn_capa(struct ipw_priv *priv,
char *capabilities, int length)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RSN_CAPABILITIES,
- .len = length,
- };
-
IPW_DEBUG_HC("HOST_CMD_RSN_CAPABILITIES\n");

- memcpy(cmd.param, capabilities, length);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RSN_CAPABILITIES, length,
+ capabilities);
}

/*
@@ -7011,25 +6974,15 @@ static int ipw_handle_assoc_response(str
static int ipw_send_qos_params_command(struct ipw_priv *priv, struct ieee80211_qos_parameters
*qos_param)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_QOS_PARAMETERS,
- .len = (sizeof(struct ieee80211_qos_parameters) * 3)
- };
-
- memcpy(cmd.param, qos_param, sizeof(*qos_param) * 3);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_QOS_PARAMETERS, qos_param,
+ sizeof(*qos_param) * 3);
}

static int ipw_send_qos_info_command(struct ipw_priv *priv, struct ieee80211_qos_information_element
*qos_param)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_WME_INFO,
- .len = sizeof(*qos_param)
- };
-
- memcpy(cmd.param, qos_param, sizeof(*qos_param));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_WME_INFO, qos_param,
+ sizeof(*qos_param));
}

#endif /* CONFIG_IPW_QOS */

--
Jens Axboe

2005-12-29 09:12:10

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] ipw2200 stack reduction

On Wed, 2005-12-28 at 22:29 +0100, Jens Axboe wrote:
> The reason is the host_cmd structure is large (500 bytes). All other
> functions currently using ipw_send_cmd() suffer from the same problem.
> This patch introduces ipw_send_cmd_simple() for commands with no data
> transfer, and ipw_send_cmd_pdu() for commands with a data payload.

Hi Jens,

Thanks for point this out and provide the patch. One comment:

> +static struct host_cmd *ipw_host_cmd_get(u8 command, u8 len)
> {
> ...
> + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);

This will still alloc 500 bytes for each command. I think if we want to
dynamically alloc the struct, we can split the struct to head and
payload and alloc the payload according to the real size.

Thanks,
-yi

2005-12-29 09:17:57

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] ipw2200 stack reduction

On Thu, Dec 29 2005, Zhu Yi wrote:
> On Wed, 2005-12-28 at 22:29 +0100, Jens Axboe wrote:
> > The reason is the host_cmd structure is large (500 bytes). All other
> > functions currently using ipw_send_cmd() suffer from the same problem.
> > This patch introduces ipw_send_cmd_simple() for commands with no data
> > transfer, and ipw_send_cmd_pdu() for commands with a data payload.
>
> Hi Jens,
>
> Thanks for point this out and provide the patch. One comment:
>
> > +static struct host_cmd *ipw_host_cmd_get(u8 command, u8 len)
> > {
> > ...
> > + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
>
> This will still alloc 500 bytes for each command. I think if we want to
> dynamically alloc the struct, we can split the struct to head and
> payload and alloc the payload according to the real size.

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.

--
Jens Axboe

2005-12-29 10:39:26

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] ipw2200 stack reduction

Hi,

On 12/29/05, Jens Axboe <[email protected]> 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?

Pekka

2005-12-29 11:12:32

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] ipw2200 stack reduction

On Thu, Dec 29 2005, Pekka Enberg wrote:
> Hi,
>
> On 12/29/05, Jens Axboe <[email protected]> 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 <[email protected]>

--- 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

2005-12-29 11:19:32

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] ipw2200 stack reduction

On Thu, Dec 29 2005, Jens Axboe wrote:
> 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...

Here it is, with updated description.

---

Checking the stack usage of my kernel, showed that ipw2200 had a few bad
offenders. This is on i386 32-bit:

0x00002876 ipw_send_associate: 544
0x000028ee ipw_send_associate: 544
0x000027dc ipw_send_scan_request_ext: 520
0x00002864 ipw_set_sensitivity: 520
0x00005eac ipw_set_rsn_capa: 520

The reason is the host_cmd structure is large (500 bytes). All other
functions currently using ipw_send_cmd() suffer from the same problem.
This patch introduces ipw_send_cmd_simple() for commands with no data
transfer, and ipw_send_cmd_pdu() for commands with a data payload and
makes the payload a pointer to the buffer passed in from the caller.

As an added bonus, the diffstat looks like this:

ipw2200.c | 260 +++++++++++++++++++++-----------------------------------------
ipw2200.h | 2
2 files changed, 92 insertions(+), 170 deletions(-)

and it shrinks the module a lot as well:

Before:

text data bss dec hex filename
75177 2472 44 77693 12f7d drivers/net/wireless/ipw2200.ko

After:

text data bss dec hex filename
61363 2488 44 63895 f997 drivers/net/wireless/ipw2200.ko

So about a ~18% reduction in module size.

Signed-off-by: Jens Axboe <[email protected]>


diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index 5e7c7e9..596db7b 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -1870,7 +1870,8 @@ static char *get_cmd_string(u8 cmd)
}

#define HOST_COMPLETE_TIMEOUT HZ
-static int ipw_send_cmd(struct ipw_priv *priv, struct host_cmd *cmd)
+
+static int __ipw_send_cmd(struct ipw_priv *priv, struct host_cmd *cmd)
{
int rc = 0;
unsigned long flags;
@@ -1899,7 +1900,7 @@ static int ipw_send_cmd(struct ipw_priv
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",
@@ -1934,7 +1935,7 @@ static int ipw_send_cmd(struct ipw_priv
goto exit;
}

- exit:
+exit:
if (priv->cmdlog) {
priv->cmdlog[priv->cmdlog_pos++].retcode = rc;
priv->cmdlog_pos %= priv->cmdlog_len;
@@ -1942,61 +1943,62 @@ static int ipw_send_cmd(struct ipw_priv
return rc;
}

-static int ipw_send_host_complete(struct ipw_priv *priv)
+static int ipw_send_cmd_simple(struct ipw_priv *priv, u8 command)
+{
+ struct host_cmd cmd = {
+ .cmd = command,
+ };
+
+ 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_CMD_HOST_COMPLETE,
- .len = 0
+ .cmd = command,
+ .len = len,
+ .param = data,
};

+ return __ipw_send_cmd(priv, &cmd);
+}
+
+static int ipw_send_host_complete(struct ipw_priv *priv)
+{
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_simple(priv, IPW_CMD_HOST_COMPLETE);
}

static int ipw_send_system_config(struct ipw_priv *priv,
struct ipw_sys_config *config)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SYSTEM_CONFIG,
- .len = sizeof(*config)
- };
-
if (!priv || !config) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, config, sizeof(*config));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SYSTEM_CONFIG, sizeof(*config),
+ config);
}

static int ipw_send_ssid(struct ipw_priv *priv, u8 * ssid, int len)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SSID,
- .len = min(len, IW_ESSID_MAX_SIZE)
- };
-
if (!priv || !ssid) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, ssid, cmd.len);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SSID, min(len, IW_ESSID_MAX_SIZE),
+ ssid);
}

static int ipw_send_adapter_address(struct ipw_priv *priv, u8 * mac)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_ADAPTER_ADDRESS,
- .len = ETH_ALEN
- };
-
if (!priv || !mac) {
IPW_ERROR("Invalid args\n");
return -1;
@@ -2005,8 +2007,8 @@ static int ipw_send_adapter_address(stru
IPW_DEBUG_INFO("%s: Setting MAC to " MAC_FMT "\n",
priv->net_dev->name, MAC_ARG(mac));

- memcpy(cmd.param, mac, ETH_ALEN);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_ADAPTER_ADDRESS, ETH_ALEN,
+ mac);
}

/*
@@ -2065,51 +2067,40 @@ static void ipw_bg_scan_check(void *data
static int ipw_send_scan_request_ext(struct ipw_priv *priv,
struct ipw_scan_request_ext *request)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SCAN_REQUEST_EXT,
- .len = sizeof(*request)
- };
-
- memcpy(cmd.param, request, sizeof(*request));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SCAN_REQUEST_EXT,
+ sizeof(*request), request);
}

static int ipw_send_scan_abort(struct ipw_priv *priv)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SCAN_ABORT,
- .len = 0
- };
-
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_simple(priv, IPW_CMD_SCAN_ABORT);
}

static int ipw_set_sensitivity(struct ipw_priv *priv, u16 sens)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SENSITIVITY_CALIB,
- .len = sizeof(struct ipw_sensitivity_calib)
+ struct ipw_sensitivity_calib calib = {
+ .beacon_rssi_raw = sens,
};
- struct ipw_sensitivity_calib *calib = (struct ipw_sensitivity_calib *)
- &cmd.param;
- calib->beacon_rssi_raw = sens;
- return ipw_send_cmd(priv, &cmd);
+
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SENSITIVITY_CALIB, sizeof(calib),
+ &calib);
}

static int ipw_send_associate(struct ipw_priv *priv,
struct ipw_associate *associate)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_ASSOCIATE,
- .len = sizeof(*associate)
- };
-
struct ipw_associate tmp_associate;
+
+ if (!priv || !associate) {
+ IPW_ERROR("Invalid args\n");
+ return -1;
+ }
+
memcpy(&tmp_associate, associate, sizeof(*associate));
tmp_associate.policy_support =
cpu_to_le16(tmp_associate.policy_support);
@@ -2122,80 +2113,56 @@ static int ipw_send_associate(struct ipw
cpu_to_le16(tmp_associate.beacon_interval);
tmp_associate.atim_window = cpu_to_le16(tmp_associate.atim_window);

- if (!priv || !associate) {
- IPW_ERROR("Invalid args\n");
- return -1;
- }
-
- memcpy(cmd.param, &tmp_associate, sizeof(*associate));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_ASSOCIATE, sizeof(tmp_associate),
+ &tmp_associate);
}

static int ipw_send_supported_rates(struct ipw_priv *priv,
struct ipw_supported_rates *rates)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SUPPORTED_RATES,
- .len = sizeof(*rates)
- };
-
if (!priv || !rates) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, rates, sizeof(*rates));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SUPPORTED_RATES, sizeof(*rates),
+ rates);
}

static int ipw_set_random_seed(struct ipw_priv *priv)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_SEED_NUMBER,
- .len = sizeof(u32)
- };
+ u32 val;

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- get_random_bytes(&cmd.param, sizeof(u32));
+ get_random_bytes(&val, sizeof(val));

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_SEED_NUMBER, sizeof(val), &val);
}

static int ipw_send_card_disable(struct ipw_priv *priv, u32 phy_off)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_CARD_DISABLE,
- .len = sizeof(u32)
- };
-
if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- *((u32 *) & cmd.param) = phy_off;
-
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_CARD_DISABLE, sizeof(phy_off),
+ &phy_off);
}

static int ipw_send_tx_power(struct ipw_priv *priv, struct ipw_tx_power *power)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_TX_POWER,
- .len = sizeof(*power)
- };
-
if (!priv || !power) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, power, sizeof(*power));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_TX_POWER, sizeof(*power),
+ power);
}

static int ipw_set_tx_power(struct ipw_priv *priv)
@@ -2247,18 +2214,14 @@ static int ipw_send_rts_threshold(struct
struct ipw_rts_threshold rts_threshold = {
.rts_threshold = rts,
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RTS_THRESHOLD,
- .len = sizeof(rts_threshold)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &rts_threshold, sizeof(rts_threshold));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RTS_THRESHOLD,
+ sizeof(rts_threshold), &rts_threshold);
}

static int ipw_send_frag_threshold(struct ipw_priv *priv, u16 frag)
@@ -2266,27 +2229,19 @@ static int ipw_send_frag_threshold(struc
struct ipw_frag_threshold frag_threshold = {
.frag_threshold = frag,
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_FRAG_THRESHOLD,
- .len = sizeof(frag_threshold)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &frag_threshold, sizeof(frag_threshold));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_FRAG_THRESHOLD,
+ sizeof(frag_threshold), &frag_threshold);
}

static int ipw_send_power_mode(struct ipw_priv *priv, u32 mode)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_POWER_MODE,
- .len = sizeof(u32)
- };
- u32 *param = (u32 *) (&cmd.param);
+ u32 param;

if (!priv) {
IPW_ERROR("Invalid args\n");
@@ -2297,17 +2252,18 @@ static int ipw_send_power_mode(struct ip
* level */
switch (mode) {
case IPW_POWER_BATTERY:
- *param = IPW_POWER_INDEX_3;
+ param = IPW_POWER_INDEX_3;
break;
case IPW_POWER_AC:
- *param = IPW_POWER_MODE_CAM;
+ param = IPW_POWER_MODE_CAM;
break;
default:
- *param = mode;
+ param = mode;
break;
}

- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_POWER_MODE, sizeof(param),
+ &param);
}

static int ipw_send_retry_limit(struct ipw_priv *priv, u8 slimit, u8 llimit)
@@ -2316,18 +2272,14 @@ static int ipw_send_retry_limit(struct i
.short_retry_limit = slimit,
.long_retry_limit = llimit
};
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RETRY_LIMIT,
- .len = sizeof(retry_limit)
- };

if (!priv) {
IPW_ERROR("Invalid args\n");
return -1;
}

- memcpy(cmd.param, &retry_limit, sizeof(retry_limit));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RETRY_LIMIT, sizeof(retry_limit),
+ &retry_limit);
}

/*
@@ -5672,54 +5624,44 @@ static void ipw_adhoc_create(struct ipw_

static void ipw_send_tgi_tx_key(struct ipw_priv *priv, int type, int index)
{
- struct ipw_tgi_tx_key *key;
- struct host_cmd cmd = {
- .cmd = IPW_CMD_TGI_TX_KEY,
- .len = sizeof(*key)
- };
+ struct ipw_tgi_tx_key key;

if (!(priv->ieee->sec.flags & (1 << index)))
return;

- key = (struct ipw_tgi_tx_key *)&cmd.param;
- key->key_id = index;
- memcpy(key->key, priv->ieee->sec.keys[index], SCM_TEMPORAL_KEY_LENGTH);
- key->security_type = type;
- key->station_index = 0; /* always 0 for BSS */
- key->flags = 0;
+ key.key_id = index;
+ memcpy(key.key, priv->ieee->sec.keys[index], SCM_TEMPORAL_KEY_LENGTH);
+ key.security_type = type;
+ key.station_index = 0; /* always 0 for BSS */
+ key.flags = 0;
/* 0 for new key; previous value of counter (after fatal error) */
- key->tx_counter[0] = 0;
- key->tx_counter[1] = 0;
+ key.tx_counter[0] = 0;
+ key.tx_counter[1] = 0;

- ipw_send_cmd(priv, &cmd);
+ ipw_send_cmd_pdu(priv, IPW_CMD_TGI_TX_KEY, sizeof(key), &key);
}

static void ipw_send_wep_keys(struct ipw_priv *priv, int type)
{
- struct ipw_wep_key *key;
+ struct ipw_wep_key key;
int i;
- struct host_cmd cmd = {
- .cmd = IPW_CMD_WEP_KEY,
- .len = sizeof(*key)
- };

- key = (struct ipw_wep_key *)&cmd.param;
- key->cmd_id = DINO_CMD_WEP_KEY;
- key->seq_num = 0;
+ key.cmd_id = DINO_CMD_WEP_KEY;
+ key.seq_num = 0;

/* Note: AES keys cannot be set for multiple times.
* Only set it at the first time. */
for (i = 0; i < 4; i++) {
- key->key_index = i | type;
+ key.key_index = i | type;
if (!(priv->ieee->sec.flags & (1 << i))) {
- key->key_size = 0;
+ key.key_size = 0;
continue;
}

- key->key_size = priv->ieee->sec.key_sizes[i];
- memcpy(key->key, priv->ieee->sec.keys[i], key->key_size);
+ key.key_size = priv->ieee->sec.key_sizes[i];
+ memcpy(key.key, priv->ieee->sec.keys[i], key.key_size);

- ipw_send_cmd(priv, &cmd);
+ ipw_send_cmd_pdu(priv, IPW_CMD_WEP_KEY, sizeof(key), &key);
}
}

@@ -6216,15 +6158,10 @@ void ipw_wpa_assoc_frame(struct ipw_priv
static int ipw_set_rsn_capa(struct ipw_priv *priv,
char *capabilities, int length)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_RSN_CAPABILITIES,
- .len = length,
- };
-
IPW_DEBUG_HC("HOST_CMD_RSN_CAPABILITIES\n");

- memcpy(cmd.param, capabilities, length);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_RSN_CAPABILITIES, length,
+ capabilities);
}

/*
@@ -7011,25 +6948,15 @@ static int ipw_handle_assoc_response(str
static int ipw_send_qos_params_command(struct ipw_priv *priv, struct ieee80211_qos_parameters
*qos_param)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_QOS_PARAMETERS,
- .len = (sizeof(struct ieee80211_qos_parameters) * 3)
- };
-
- memcpy(cmd.param, qos_param, sizeof(*qos_param) * 3);
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_QOS_PARAMETERS, qos_param,
+ sizeof(*qos_param) * 3);
}

static int ipw_send_qos_info_command(struct ipw_priv *priv, struct ieee80211_qos_information_element
*qos_param)
{
- struct host_cmd cmd = {
- .cmd = IPW_CMD_WME_INFO,
- .len = sizeof(*qos_param)
- };
-
- memcpy(cmd.param, qos_param, sizeof(*qos_param));
- return ipw_send_cmd(priv, &cmd);
+ return ipw_send_cmd_pdu(priv, IPW_CMD_WME_INFO, qos_param,
+ sizeof(*qos_param));
}

#endif /* CONFIG_IPW_QOS */
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

2005-12-30 05:28:16

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] ipw2200 stack reduction

On Thu, 2005-12-29 at 12:19 +0100, Jens Axboe wrote:
> On Thu, Dec 29 2005, Jens Axboe wrote:
> > 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...
>
> Here it is, with updated description.

I'm fine with the one. Thank you Jens!

-yi