iscsi bugfixes and features for scsi-misc

iscsi bugfixes and features for scsi-misc

[michaelc]

The following patches were made over scsi-misc plus this
patch
http://marc.info/?l=linux-scsi&m=126335810027706&w=2
which is not yet merged (that patch fixes a regression
in 2.6.33 and should go in there).

[PATCH 1/6] cxgb3i, bnx2i: remove uses of nipquad use %pi4

[michaelc]

From: Joe Perches <joe@perches.com>

Remove uses of NIPQUAD, use %pI4

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/bnx2i/bnx2i_iscsi.c     |    4 ++--
 drivers/scsi/cxgb3i/cxgb3i_iscsi.c   |    5 ++---
 drivers/scsi/cxgb3i/cxgb3i_offload.c |    7 ++++---
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index 33b2294..51709cb 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -1426,8 +1426,8 @@ static int bnx2i_conn_get_param(struct iscsi_cls_conn *cls_conn,
 		break;
 	case ISCSI_PARAM_CONN_ADDRESS:
 		if (bnx2i_conn->ep)
-			len = sprintf(buf, NIPQUAD_FMT "\n",
-				      NIPQUAD(bnx2i_conn->ep->cm_sk->dst_ip));
+			len = sprintf(buf, "%pI4\n",
+				      &bnx2i_conn->ep->cm_sk->dst_ip);
 		break;
 	default:
 		return iscsi_conn_get_param(cls_conn, param, buf);
diff --git a/drivers/scsi/cxgb3i/cxgb3i_iscsi.c b/drivers/scsi/cxgb3i/cxgb3i_iscsi.c
index 969c831..1fd89b2 100644
--- a/drivers/scsi/cxgb3i/cxgb3i_iscsi.c
+++ b/drivers/scsi/cxgb3i/cxgb3i_iscsi.c
@@ -591,8 +591,7 @@ static int cxgb3i_conn_bind(struct iscsi_cls_session *cls_session,
 	cxgb3i_conn_max_recv_dlength(conn);
 
 	spin_lock_bh(&conn->session->lock);
-	sprintf(conn->portal_address, NIPQUAD_FMT,
-		NIPQUAD(c3cn->daddr.sin_addr.s_addr));
+	sprintf(conn->portal_address, "%pI4", &c3cn->daddr.sin_addr.s_addr);
 	conn->portal_port = ntohs(c3cn->daddr.sin_port);
 	spin_unlock_bh(&conn->session->lock);
 
@@ -753,7 +752,7 @@ static int cxgb3i_host_get_param(struct Scsi_Host *shost,
 		__be32 addr;
 
 		addr = cxgb3i_get_private_ipv4addr(hba->ndev);
-		len = sprintf(buf, NIPQUAD_FMT, NIPQUAD(addr));
+		len = sprintf(buf, "%pI4", &addr);
 		break;
 	}
 	default:
diff --git a/drivers/scsi/cxgb3i/cxgb3i_offload.c b/drivers/scsi/cxgb3i/cxgb3i_offload.c
index 15a00e8..3e08c43 100644
--- a/drivers/scsi/cxgb3i/cxgb3i_offload.c
+++ b/drivers/scsi/cxgb3i/cxgb3i_offload.c
@@ -1675,10 +1675,11 @@ int cxgb3i_c3cn_connect(struct net_device *dev, struct s3_conn *c3cn,
 	} else
 		c3cn->saddr.sin_addr.s_addr = sipv4;
 
-	c3cn_conn_debug("c3cn 0x%p, %u.%u.%u.%u,%u-%u.%u.%u.%u,%u SYN_SENT.\n",
-			c3cn, NIPQUAD(c3cn->saddr.sin_addr.s_addr),
+	c3cn_conn_debug("c3cn 0x%p, %pI4,%u-%pI4,%u SYN_SENT.\n",
+			c3cn,
+			&c3cn->saddr.sin_addr.s_addr,
 			ntohs(c3cn->saddr.sin_port),
-			NIPQUAD(c3cn->daddr.sin_addr.s_addr),
+			&c3cn->daddr.sin_addr.s_addr,
 			ntohs(c3cn->daddr.sin_port));
 
 	c3cn_set_state(c3cn, C3CN_STATE_CONNECTING);
-- 
1.6.6

[PATCH 2/6] cxgb3i: check for setup netdev

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

If the netdev has not been setup when the host is, we
    will oops when the iscsi layer calls into the driver
    and a it tries to reference the netdev in hba->ndev.

    This can happen if the iscsi driver is loaded before
    ifup is done. This patch just adds a check, so we
    can gracefully fail the operation.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/cxgb3i/cxgb3i_iscsi.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/cxgb3i/cxgb3i_iscsi.c b/drivers/scsi/cxgb3i/cxgb3i_iscsi.c
index 1fd89b2..412853c 100644
--- a/drivers/scsi/cxgb3i/cxgb3i_iscsi.c
+++ b/drivers/scsi/cxgb3i/cxgb3i_iscsi.c
@@ -708,6 +708,12 @@ static int cxgb3i_host_set_param(struct Scsi_Host *shost,
 {
 	struct cxgb3i_hba *hba = iscsi_host_priv(shost);
 
+	if (!hba->ndev) {
+		shost_printk(KERN_ERR, shost, "Could not set host param. "
+			     "Netdev for host not set.\n");
+		return -ENODEV;
+	}
+
 	cxgb3i_api_debug("param %d, buf %s.\n", param, buf);
 
 	switch (param) {
@@ -738,6 +744,12 @@ static int cxgb3i_host_get_param(struct Scsi_Host *shost,
 	struct cxgb3i_hba *hba = iscsi_host_priv(shost);
 	int len = 0;
 
+	if (!hba->ndev) {
+		shost_printk(KERN_ERR, shost, "Could not set host param. "
+			     "Netdev for host not set.\n");
+		return -ENODEV;
+	}
+
 	cxgb3i_api_debug("hba %s, param %d.\n", hba->ndev->name, param);
 
 	switch (param) {
-- 
1.6.6

[PATCH 3/6] libiscsi: reset cmd timer if cmds are making progress

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

This patch resets the cmd timer if cmds started before
the timedout command are making progress. The idea is
that the cmd probably timed out because we are trying
to exeucte too many commands. If it turns out that the
device the IO timedout on was bad or the cmd just got
screwed up but other IO/devs were ok then we will
will figure this out when the cmds ahead of the timed
out one complete ok.

This also fixes a bug where we were sort of detecting
this by setting the last_timeout and last_xfer to the
same value when the task was allocated. That caught
the case where we never got to send any IO for it. However,
if the problem had started right before we started the
new task, then we were forced to wait an extra cmd
timeout seconds to start the scsi eh.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/libiscsi.c |   53 +++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index c28a712..703eb6a 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1919,10 +1919,11 @@ static int iscsi_has_ping_timed_out(struct iscsi_conn *conn)
 static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 {
 	enum blk_eh_timer_return rc = BLK_EH_NOT_HANDLED;
-	struct iscsi_task *task = NULL;
+	struct iscsi_task *task = NULL, *running_task;
 	struct iscsi_cls_session *cls_session;
 	struct iscsi_session *session;
 	struct iscsi_conn *conn;
+	int i;
 
 	cls_session = starget_to_session(scsi_target(sc->device));
 	session = cls_session->dd_data;
@@ -1947,8 +1948,15 @@ static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 	}
 
 	task = (struct iscsi_task *)sc->SCp.ptr;
-	if (!task)
+	if (!task) {
+		/*
+		 * Raced with completion. Just reset timer, and let it
+		 * complete normally
+		 */
+		rc = BLK_EH_RESET_TIMER;
 		goto done;
+	}
+
 	/*
 	 * If we have sent (at least queued to the network layer) a pdu or
 	 * recvd one for the task since the last timeout ask for
@@ -1956,10 +1964,10 @@ static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 	 * we can check if it is the task or connection when we send the
 	 * nop as a ping.
 	 */
-	if (time_after_eq(task->last_xfer, task->last_timeout)) {
+	if (time_after(task->last_xfer, task->last_timeout)) {
 		ISCSI_DBG_EH(session, "Command making progress. Asking "
 			     "scsi-ml for more time to complete. "
-			     "Last data recv at %lu. Last timeout was at "
+			     "Last data xfer at %lu. Last timeout was at "
 			     "%lu\n.", task->last_xfer, task->last_timeout);
 		task->have_checked_conn = false;
 		rc = BLK_EH_RESET_TIMER;
@@ -1977,6 +1985,43 @@ static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 		goto done;
 	}
 
+	for (i = 0; i < conn->session->cmds_max; i++) {
+		running_task = conn->session->cmds[i];
+		if (!running_task->sc || running_task == task ||
+		     running_task->state != ISCSI_TASK_RUNNING)
+			continue;
+
+		/*
+		 * Only check if cmds started before this one have made
+		 * progress, or this could never fail
+		 */
+		if (time_after(running_task->sc->jiffies_at_alloc,
+			       task->sc->jiffies_at_alloc))
+			continue;
+
+		if (time_after(running_task->last_xfer, task->last_timeout)) {
+			/*
+			 * This task has not made progress, but a task
+			 * started before us has transferred data since
+			 * we started/last-checked. We could be queueing
+			 * too many tasks or the LU is bad.
+			 *
+			 * If the device is bad the cmds ahead of us on
+			 * other devs will complete, and this loop will
+			 * eventually fail starting the scsi eh.
+			 */
+			ISCSI_DBG_EH(session, "Command has not made progress "
+				     "but commands ahead of it have. "
+				     "Asking scsi-ml for more time to "
+				     "complete. Our last xfer vs running task "
+				     "last xfer %lu/%lu. Last check %lu.\n",
+				     task->last_xfer, running_task->last_xfer,
+				     task->last_timeout);
+			rc = BLK_EH_RESET_TIMER;
+			goto done;
+		}
+	}
+
 	/* Assumes nop timeout is shorter than scsi cmd timeout */
 	if (task->have_checked_conn)
 		goto done;
-- 
1.6.6

[PATCH 4/6] bnx2i: set change_queue_depth function

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

No reason that we cannot set the change_queue_depth
function for bnx2i. We just forgot to when the
driver was created.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/bnx2i/bnx2i_iscsi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index 51709cb..1c4d121 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -1990,6 +1990,7 @@ static struct scsi_host_template bnx2i_host_template = {
 	.eh_abort_handler	= iscsi_eh_abort,
 	.eh_device_reset_handler = iscsi_eh_device_reset,
 	.eh_target_reset_handler = iscsi_eh_target_reset,
+	.change_queue_depth	= iscsi_change_queue_depth,
 	.can_queue		= 1024,
 	.max_sectors		= 127,
 	.cmd_per_lun		= 32,
-- 
1.6.6

[PATCH 5/6] iscsi_tcp: wake xmit thread when killing session

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

If the connection is bad, then the xmit thread could
end up waiting a long time (up to sendtmeo seconds) in
tcp_sendpage. This patch has us set the sk_error and
wake up the xmit thread so we can quickly fail.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/iscsi_tcp.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 517da3f..8a89ba9 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -584,9 +584,10 @@ static void iscsi_sw_tcp_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
 	struct iscsi_conn *conn = cls_conn->dd_data;
 	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
 	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
+	struct socket *sock = tcp_sw_conn->sock;
 
 	/* userspace may have goofed up and not bound us */
-	if (!tcp_sw_conn->sock)
+	if (!sock)
 		return;
 	/*
 	 * Make sure our recv side is stopped.
@@ -597,6 +598,11 @@ static void iscsi_sw_tcp_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
 	set_bit(ISCSI_SUSPEND_BIT, &conn->suspend_rx);
 	write_unlock_bh(&tcp_sw_conn->sock->sk->sk_callback_lock);
 
+	if (sock->sk->sk_sleep && waitqueue_active(sock->sk->sk_sleep)) {
+		sock->sk->sk_err = EIO;
+		wake_up_interruptible(sock->sk->sk_sleep);
+	}
+
 	iscsi_conn_stop(cls_conn, flag);
 	iscsi_sw_tcp_release_conn(conn);
 }
-- 
1.6.6

[PATCH 6/6] qla4xxx: fix compile warning due to invalid extHwConfig

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

If the nvram is invalid qla4xxx tries to set Asuint32_t
based on the card type. If the card type is not listed
then Asuint32_t is going to be gargabe. This just fixes
that if/elseif by adding a else to catch the case for
new hardware that might not be listed yet.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/qla4xxx/ql4_init.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
index af8c323..92329a4 100644
--- a/drivers/scsi/qla4xxx/ql4_init.c
+++ b/drivers/scsi/qla4xxx/ql4_init.c
@@ -844,10 +844,10 @@ static int qla4xxx_config_nvram(struct scsi_qla_host *ha)
 	DEBUG2(printk("scsi%ld: %s: Get EEProm parameters \n", ha->host_no,
 		      __func__));
 	if (ql4xxx_lock_flash(ha) != QLA_SUCCESS)
-		return (QLA_ERROR);
+		return QLA_ERROR;
 	if (ql4xxx_lock_nvram(ha) != QLA_SUCCESS) {
 		ql4xxx_unlock_flash(ha);
-		return (QLA_ERROR);
+		return QLA_ERROR;
 	}
 
 	/* Get EEPRom Parameters from NVRAM and validate */
@@ -858,20 +858,18 @@ static int qla4xxx_config_nvram(struct scsi_qla_host *ha)
 			rd_nvram_word(ha, eeprom_ext_hw_conf_offset(ha));
 		spin_unlock_irqrestore(&ha->hardware_lock, flags);
 	} else {
-		/*
-		 * QLogic adapters should always have a valid NVRAM.
-		 * If not valid, do not load.
-		 */
 		dev_warn(&ha->pdev->dev,
 			   "scsi%ld: %s: EEProm checksum invalid.  "
 			   "Please update your EEPROM\n", ha->host_no,
 			   __func__);
 
-		/* set defaults */
+		/* Attempt to set defaults */
 		if (is_qla4010(ha))
 			extHwConfig.Asuint32_t = 0x1912;
 		else if (is_qla4022(ha) | is_qla4032(ha))
 			extHwConfig.Asuint32_t = 0x0023;
+		else
+			return QLA_ERROR;
 	}
 	DEBUG(printk("scsi%ld: %s: Setting extHwConfig to 0xFFFF%04x\n",
 		     ha->host_no, __func__, extHwConfig.Asuint32_t));
@@ -884,7 +882,7 @@ static int qla4xxx_config_nvram(struct scsi_qla_host *ha)
 	ql4xxx_unlock_nvram(ha);
 	ql4xxx_unlock_flash(ha);
 
-	return (QLA_SUCCESS);
+	return QLA_SUCCESS;
 }
 
 static void qla4x00_pci_config(struct scsi_qla_host *ha)
-- 
1.6.6

Re: iscsi bugfixes and features for scsi-misc

[Boaz Harrosh]

On 02/11/2010 12:51 AM, michaelc@cs.wisc.edu wrote:
> The following patches were made over scsi-misc plus this
> patch
> http://marc.info/?l=linux-scsi&m=126335810027706&w=2

OK The above patch:
	PATCH 1/1] iscsi_tcp regression: remove bogus warn on in write path

Is it not in main line? It causes me grate pain that one please submit into
2.6.33-rc ASAP

Boaz
> which is not yet merged (that patch fixes a regression
> in 2.6.33 and should go in there).
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html