[RFC] Killing struct cmd_ctrl_node.wait_option

David Woodhouse dwmw2 at infradead.org
Thu Dec 13 13:48:54 EST 2007


I had this patch lined up last night, but now I'm not so sure.

I want command nodes to be freed by whoever submitted them -- i.e. do it
in __lbs_cmd() (and lbs_prepare_and_send_command() too for now, until
that dies). That way we can fix the races with priv->cur_cmd_retcode,
which as it stands can be overwritten by subsequent commands before the
thread waiting in __lbs_cmd() ever wakes up and looks at it.

The question is: how do we free the command structures for the
_asynchronous_ commands? We can't do it from the callback if we aren't
calling the callback on failure (which we aren't, right now).

The simple answer is that we actually start using the wait_option field,
which means I shouldn't remove it. If wait_option isn't set (indicating
an async command), the structure should be freed in cmdresp.c.

I think I prefer the alternative answer, which is that we start calling
the callbacks even in the failure case. Yes, it means the callback has
to start checking resp->result. But most functions will hopefully be
using the same callback function (lbs_cmd_copyback) anyway, so it's not
as if we have to add that in too many places.

commit f6a24cc26d0ad95c440846b15ec218e386994e4a
Author: David Woodhouse <dwmw2 at infradead.org>
Date:   Thu Dec 13 02:36:57 2007 -0500

    libertas: kill unused wait_option field in struct cmd_ctrl_node
    
    Signed-off-by: David Woodhouse <dwmw2 at infradead.org>

diff --git a/drivers/net/wireless/libertas/cmd.c b/drivers/net/wireless/libertas/cmd.c
index 171acc2..01052cf 100644
--- a/drivers/net/wireless/libertas/cmd.c
+++ b/drivers/net/wireless/libertas/cmd.c
@@ -17,7 +17,7 @@ static void cleanup_cmdnode(struct cmd_ctrl_node *ptempnode);
 static struct cmd_ctrl_node *lbs_get_cmd_ctrl_node(struct lbs_private *priv);
 static void lbs_set_cmd_ctrl_node(struct lbs_private *priv,
 		    struct cmd_ctrl_node *ptempnode,
-		    u16 wait_option, void *pdata_buf);
+		    void *pdata_buf);
 
 
 /**
@@ -1392,7 +1392,7 @@ int lbs_prepare_and_send_command(struct lbs_private *priv,
 		goto done;
 	}
 
-	lbs_set_cmd_ctrl_node(priv, cmdnode, wait_option, pdata_buf);
+	lbs_set_cmd_ctrl_node(priv, cmdnode, pdata_buf);
 
 	cmdptr = (struct cmd_ds_command *)cmdnode->cmdbuf;
 
@@ -1554,7 +1554,7 @@ int lbs_prepare_and_send_command(struct lbs_private *priv,
 	case CMD_802_11_INACTIVITY_TIMEOUT:
 		ret = lbs_cmd_802_11_inactivity_timeout(priv, cmdptr,
 							 cmd_action, pdata_buf);
-		lbs_set_cmd_ctrl_node(priv, cmdnode, 0, pdata_buf);
+		lbs_set_cmd_ctrl_node(priv, cmdnode, pdata_buf);
 		break;
 
 	case CMD_802_11_TPC_CFG:
@@ -1800,7 +1800,6 @@ static void cleanup_cmdnode(struct cmd_ctrl_node *cmdnode)
 		return;
 	cmdnode->cmdwaitqwoken = 1;
 	wake_up_interruptible(&cmdnode->cmdwait_q);
-	cmdnode->wait_option = 0;
 	cmdnode->pdata_buf = NULL;
 	cmdnode->callback = NULL;
 	cmdnode->callback_arg = 0;
@@ -1816,20 +1815,18 @@ static void cleanup_cmdnode(struct cmd_ctrl_node *cmdnode)
  *
  *  @param priv		A pointer to struct lbs_private structure
  *  @param ptempnode	A pointer to cmd_ctrl_node structure
- *  @param wait_option	wait option: wait response or not
  *  @param pdata_buf	A pointer to informaion buffer
  *  @return 		0 or -1
  */
 static void lbs_set_cmd_ctrl_node(struct lbs_private *priv,
 				  struct cmd_ctrl_node *ptempnode,
-				  u16 wait_option, void *pdata_buf)
+				  void *pdata_buf)
 {
 	lbs_deb_enter(LBS_DEB_HOST);
 
 	if (!ptempnode)
 		return;
 
-	ptempnode->wait_option = wait_option;
 	ptempnode->pdata_buf = pdata_buf;
 	ptempnode->callback = NULL;
 	ptempnode->callback_arg = 0;
@@ -2213,7 +2210,6 @@ int __lbs_cmd(struct lbs_private *priv, uint16_t command,
 		goto done;
 	}
 
-	cmdnode->wait_option = CMD_OPTION_WAITFORRSP;
 	cmdnode->callback = callback;
 	cmdnode->callback_arg = callback_arg;
 
diff --git a/drivers/net/wireless/libertas/hostcmd.h b/drivers/net/wireless/libertas/hostcmd.h
index aa4cea0..1b31250 100644
--- a/drivers/net/wireless/libertas/hostcmd.h
+++ b/drivers/net/wireless/libertas/hostcmd.h
@@ -74,8 +74,6 @@ struct cmd_header {
 
 struct cmd_ctrl_node {
 	struct list_head list;
-	/* wait for finish or not */
-	u16 wait_option;
 	/* command response */
 	void *pdata_buf;
 	int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *);

-- 
dwmw2




More information about the libertas-dev mailing list