* iscsi bugfixes and features for scsi-misc @ 2010-02-10 22:51 michaelc 2010-02-10 22:51 ` [PATCH 1/6] cxgb3i, bnx2i: remove uses of nipquad use %pi4 michaelc 2010-02-11 8:45 ` iscsi bugfixes and features for scsi-misc Boaz Harrosh 0 siblings, 2 replies; 8+ messages in thread From: michaelc @ 2010-02-10 22:51 UTC (permalink / raw) To: linux-scsi 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). ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/6] cxgb3i, bnx2i: remove uses of nipquad use %pi4 2010-02-10 22:51 iscsi bugfixes and features for scsi-misc michaelc @ 2010-02-10 22:51 ` michaelc 2010-02-10 22:51 ` [PATCH 2/6] cxgb3i: check for setup netdev michaelc 2010-02-11 8:45 ` iscsi bugfixes and features for scsi-misc Boaz Harrosh 1 sibling, 1 reply; 8+ messages in thread From: michaelc @ 2010-02-10 22:51 UTC (permalink / raw) To: linux-scsi; +Cc: Joe Perches, Andrew Morton, Mike Christie 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/6] cxgb3i: check for setup netdev 2010-02-10 22:51 ` [PATCH 1/6] cxgb3i, bnx2i: remove uses of nipquad use %pi4 michaelc @ 2010-02-10 22:51 ` michaelc 2010-02-10 22:51 ` [PATCH 3/6] libiscsi: reset cmd timer if cmds are making progress michaelc 0 siblings, 1 reply; 8+ messages in thread From: michaelc @ 2010-02-10 22:51 UTC (permalink / raw) To: linux-scsi; +Cc: Mike Christie 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/6] libiscsi: reset cmd timer if cmds are making progress 2010-02-10 22:51 ` [PATCH 2/6] cxgb3i: check for setup netdev michaelc @ 2010-02-10 22:51 ` michaelc 2010-02-10 22:51 ` [PATCH 4/6] bnx2i: set change_queue_depth function michaelc 0 siblings, 1 reply; 8+ messages in thread From: michaelc @ 2010-02-10 22:51 UTC (permalink / raw) To: linux-scsi; +Cc: Mike Christie 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/6] bnx2i: set change_queue_depth function 2010-02-10 22:51 ` [PATCH 3/6] libiscsi: reset cmd timer if cmds are making progress michaelc @ 2010-02-10 22:51 ` michaelc 2010-02-10 22:51 ` [PATCH 5/6] iscsi_tcp: wake xmit thread when killing session michaelc 0 siblings, 1 reply; 8+ messages in thread From: michaelc @ 2010-02-10 22:51 UTC (permalink / raw) To: linux-scsi; +Cc: Mike Christie 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/6] iscsi_tcp: wake xmit thread when killing session 2010-02-10 22:51 ` [PATCH 4/6] bnx2i: set change_queue_depth function michaelc @ 2010-02-10 22:51 ` michaelc 2010-02-10 22:51 ` [PATCH 6/6] qla4xxx: fix compile warning due to invalid extHwConfig michaelc 0 siblings, 1 reply; 8+ messages in thread From: michaelc @ 2010-02-10 22:51 UTC (permalink / raw) To: linux-scsi; +Cc: Mike Christie 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 6/6] qla4xxx: fix compile warning due to invalid extHwConfig 2010-02-10 22:51 ` [PATCH 5/6] iscsi_tcp: wake xmit thread when killing session michaelc @ 2010-02-10 22:51 ` michaelc 0 siblings, 0 replies; 8+ messages in thread From: michaelc @ 2010-02-10 22:51 UTC (permalink / raw) To: linux-scsi; +Cc: Mike Christie 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: iscsi bugfixes and features for scsi-misc 2010-02-10 22:51 iscsi bugfixes and features for scsi-misc michaelc 2010-02-10 22:51 ` [PATCH 1/6] cxgb3i, bnx2i: remove uses of nipquad use %pi4 michaelc @ 2010-02-11 8:45 ` Boaz Harrosh 1 sibling, 0 replies; 8+ messages in thread From: Boaz Harrosh @ 2010-02-11 8:45 UTC (permalink / raw) To: michaelc; +Cc: linux-scsi 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-02-11 8:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-02-10 22:51 iscsi bugfixes and features for scsi-misc michaelc 2010-02-10 22:51 ` [PATCH 1/6] cxgb3i, bnx2i: remove uses of nipquad use %pi4 michaelc 2010-02-10 22:51 ` [PATCH 2/6] cxgb3i: check for setup netdev michaelc 2010-02-10 22:51 ` [PATCH 3/6] libiscsi: reset cmd timer if cmds are making progress michaelc 2010-02-10 22:51 ` [PATCH 4/6] bnx2i: set change_queue_depth function michaelc 2010-02-10 22:51 ` [PATCH 5/6] iscsi_tcp: wake xmit thread when killing session michaelc 2010-02-10 22:51 ` [PATCH 6/6] qla4xxx: fix compile warning due to invalid extHwConfig michaelc 2010-02-11 8:45 ` iscsi bugfixes and features for scsi-misc Boaz Harrosh
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.