[svn-commits] sruffell: linux/trunk r8400 - /linux/trunk/drivers/dahdi/wcte12xp/base.c

[SVN commits to the Digium repositories]

Author: sruffell
Date: Tue Mar 23 16:56:02 2010
New Revision: 8400

URL: http://svnview.digium.com/svn/dahdi?view=rev&rev=8400
Log:
wcte12xp: Fix potential race on command handling.

If we timeout a command, don't free it right away unless we are the one who
removed it from whatever list it was on.  Also, increase the timeout (2 seconds
wasn't enough when the firmware for the VPMOCT was being loaded on a system with
4K stacks) and check the return value from t1_getreg in case there were timeouts.
DAHDI-560.

Modified:
    linux/trunk/drivers/dahdi/wcte12xp/base.c

Modified: linux/trunk/drivers/dahdi/wcte12xp/base.c
URL: http://svnview.digium.com/svn/dahdi/linux/trunk/drivers/dahdi/wcte12xp/base.c?view=diff&rev=8400&r1=8399&r2=8400
==============================================================================
--- linux/trunk/drivers/dahdi/wcte12xp/base.c (original)
+++ linux/trunk/drivers/dahdi/wcte12xp/base.c Tue Mar 23 16:56:02 2010
@@ -112,6 +112,7 @@
 	cmd = kmem_cache_alloc(cmd_cache, GFP_ATOMIC);
 	if (cmd) {
 		memset(cmd, 0, sizeof(*cmd));
+		init_completion(&cmd->complete);
 		INIT_LIST_HEAD(&cmd->node);
 	}
 	return cmd;
@@ -138,8 +139,6 @@
 static void submit_cmd(struct t1 *wc, struct command *cmd)
 {
 	unsigned long flags;
-	if (cmd->flags & (__CMD_RD | __CMD_PINS))
-		init_completion(&cmd->complete);
 	spin_lock_irqsave(&wc->cmd_list_lock, flags);
 	list_add_tail(&cmd->node, &wc->pending_cmds);
 	spin_unlock_irqrestore(&wc->cmd_list_lock, flags);
@@ -213,7 +212,7 @@
 		if (cmd->flags & (__CMD_WR | __CMD_LEDS)) {
 			/* Nobody is waiting on writes...so let's just
 			 * free them here. */
-			list_del(&cmd->node);
+			list_del_init(&cmd->node);
 			free_cmd(wc, cmd);
 		} else {
 			cmd->data |= readchunk[CMD_BYTE(cmd->cs_slot, 2, IS_VPM)];
@@ -577,17 +576,31 @@
 	cmd->data = 0x00;
 	cmd->flags = __CMD_RD;
 	submit_cmd(wc, cmd);
-	ret = wait_for_completion_timeout(&cmd->complete, HZ*2);
+	ret = wait_for_completion_timeout(&cmd->complete, HZ*10);
 	if (unlikely(!ret)) {
 		spin_lock_bh(&wc->cmd_list_lock);
-		list_del(&cmd->node);
-		spin_unlock_bh(&wc->cmd_list_lock);
-		if (printk_ratelimit()) {
-			dev_warn(&wc->vb.pdev->dev,
-				 "Timeout in %s\n", __func__);
-		}
-		free_cmd(wc, cmd);
-		return -EIO;
+		if (!list_empty(&cmd->node)) {
+			/* Since we've removed this command from the list, we
+			 * can go ahead and free it right away. */
+			list_del_init(&cmd->node);
+			spin_unlock_bh(&wc->cmd_list_lock);
+			if (printk_ratelimit()) {
+				dev_warn(&wc->vb.pdev->dev,
+					 "Timeout in %s\n", __func__);
+			}
+			free_cmd(wc, cmd);
+			return -EIO;
+		} else {
+			/* Looks like this command was removed from the list by
+			 * someone else already. Let's wait for them to complete
+			 * it so that we don't free up the memory. */
+			spin_unlock_bh(&wc->cmd_list_lock);
+			ret = wait_for_completion_timeout(&cmd->complete, HZ*2);
+			WARN_ON(!ret);
+			ret = cmd->data;
+			free_cmd(wc, cmd);
+			return ret;
+		}
 	}
 	ret = cmd->data;
 	free_cmd(wc, cmd);
@@ -623,7 +636,7 @@
 	ret = wait_for_completion_timeout(&cmd->complete, HZ*2);
 	if (unlikely(!ret)) {
 		spin_lock_bh(&wc->cmd_list_lock);
-		list_del(&cmd->node);
+		list_del_init(&cmd->node);
 		spin_unlock_bh(&wc->cmd_list_lock);
 		if (printk_ratelimit()) {
 			dev_warn(&wc->vb.pdev->dev,
@@ -678,7 +691,7 @@
 	spin_unlock_irqrestore(&wc->cmd_list_lock, flags);
 	while (!list_empty(&list)) {
 		cmd = list_entry(list.next, struct command, node);
-		list_del(&cmd->node);
+		list_del_init(&cmd->node);
 		free_cmd(wc, cmd);
 	}
 	kfree(wc);
@@ -1115,6 +1128,8 @@
 		case DAHDI_MAINT_LOCALLOOP:
 			t1xxp_clear_maint(span);
 			reg = t1_getreg(wc, LIM0);
+			if (reg < 0)
+				return -EIO;
 			t1_setreg(wc, LIM0, reg | LIM0_LL);
 			break;
 		case DAHDI_MAINT_REMOTELOOP:
@@ -1135,17 +1150,23 @@
 		case DAHDI_MAINT_LOCALLOOP:
 			t1xxp_clear_maint(span);
  			reg = t1_getreg(wc, LIM0);
- 			t1_setreg(wc, LIM0, reg | LIM0_LL);
+			if (reg < 0)
+				return -EIO;
+			t1_setreg(wc, LIM0, reg | LIM0_LL);
 			break;
  		case DAHDI_MAINT_NETWORKLINELOOP:
 			t1xxp_clear_maint(span);
  			reg = t1_getreg(wc, LIM1);
- 			t1_setreg(wc, LIM1, reg | LIM1_RL);
+			if (reg < 0)
+				return -EIO;
+			t1_setreg(wc, LIM1, reg | LIM1_RL);
  			break;
  		case DAHDI_MAINT_NETWORKPAYLOADLOOP:
 			t1xxp_clear_maint(span);
  			reg = t1_getreg(wc, LIM1);
- 			t1_setreg(wc, LIM1, reg | (LIM1_RL | LIM1_JATT));
+			if (reg < 0)
+				return -EIO;
+			t1_setreg(wc, LIM1, reg | (LIM1_RL | LIM1_JATT));
 			break;
 		case DAHDI_MAINT_LOOPUP:
 			t1xxp_clear_maint(span);
@@ -1175,10 +1196,14 @@
 
 	/* Turn off local loop */
 	reg = t1_getreg(wc, LIM0);
+	if (reg < 0)
+		return -EIO;
 	t1_setreg(wc, LIM0, reg & ~LIM0_LL);
 
 	/* Turn off remote loop & jitter attenuator */
 	reg = t1_getreg(wc, LIM1);
+	if (reg < 0)
+		return -EIO;
 	t1_setreg(wc, LIM1, reg & ~(LIM1_RL | LIM1_JATT));
 	return 0;
 }