Make ib_srp better suited for H.A. purposes
Subsystems (3)
v4 cover Make ib_srp better suited for H.A. purposes
This patch series makes the ib_srp driver better suited for use in a H.A. setup because:
- multipathd is notified faster about transport layer failures.
- Transport layer failures reliably result in a reconnect.
- Switchover can be triggered explicitly by deleting an initiator device.
- Disconnecting from a target without unloading ib_srp is now possible.
Changes since v3:
- Restored the dev_loss_tmo and fast_io_fail_tmo sysfs attributes.
- Included a patch to fix an ib_srp crash that could be triggered by cable pulling.
Changes since v2:
- Addressed the v2 review comments.
- Dropped the patches that have already been merged.
- Dropped the patches for integration with multipathd.
- Dropped the micro-optimization of the IB completion handlers.
The individual patches are:
0001-ib_srp-Fix-a-race-condition.patch 0002-ib_srp-Enlarge-block-layer-timeout.patch 0003-ib_srp-Move-QP-state-check-into-srp_send_tsk_mgmt.patch 0004-ib_srp-Stop-queueing-if-QP-in-error.patch 0005-ib_srp-Eliminate-state-SRP_TARGET_CONNECTING.patch 0006-ib_srp-Suppress-superfluous-error-messages.patch 0007-ib_srp-Avoid-that-SCSI-error-handling-triggers-a-cra.patch 0008-ib_srp-Introduce-the-helper-function-srp_remove_targ.patch 0009-ib_srp-Eliminate-state-SRP_TARGET_DEAD.patch 0010-ib_srp-Keep-processing-commands-during-scsi_remove_h.patch 0011-ib_srp-Make-srp_disconnect_target-wait-for-IB-comple.patch 0012-ib_srp-Document-sysfs-attributes.patch 0013-srp_transport-Fix-atttribute-registration.patch 0014-srp_transport-Simplify-attribute-initialization-code.patch 0015-srp_transport-Document-sysfs-attributes.patch 0016-ib_srp-Allow-SRP-disconnect-through-sysfs.patch 0017-ib_srp-Introduce-a-temporary-variable-in-srp_remove_.patch 0018-ib_srp-Maintain-a-single-connection-per-I_T-nexus.patch 0019-srp_transport-Add-transport-layer-error-handling.patch 0020-ib_srp-Add-dev_loss_tmo-support.patch
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v4 1/20 ib_srp: Fix a race condition
Avoid that the scmnd->scsi_done(scmnd) call in srp_process_rsp() can trigger a crash by being invoked with scsi_done == NULL. That could happen if a reply is received during or after a command abort.
BUG: unable to handle kernel NULL pointer dereference at (null) Call Trace: <IRQ> [<ffffffffa04ab636>] ? srp_handle_recv+0x216/0x480 [ib_srp] [<ffffffffa04ab8ea>] srp_recv_completion+0x4a/0xb0 [ib_srp] [<ffffffffa032b497>] mlx4_ib_cq_comp+0x17/0x20 [mlx4_ib] [<ffffffffa02519f0>] mlx4_cq_completion+0x40/0x80 [mlx4_core] [<ffffffffa02529d3>] mlx4_eq_int+0x543/0x920 [mlx4_core] [<ffffffff81071acf>] ? local_clock+0x4f/0x60 [<ffffffffa0252dc4>] mlx4_msi_x_interrupt+0x14/0x20 [mlx4_core] [<ffffffff810b82b5>] handle_irq_event_percpu+0x75/0x240 [<ffffffff810b84ce>] handle_irq_event+0x4e/0x80 [<ffffffff810badf5>] handle_edge_irq+0x85/0x130 [<ffffffff81004355>] handle_irq+0x25/0x40 [<ffffffff814153cd>] do_IRQ+0x5d/0xe0 [<ffffffff8140b96c>] common_interrupt+0x6c/0x6c <EOI> Kernel panic - not syncing: Fatal exception in interrupt
Reference: http://marc.info/?l=linux-rdma&m=134314367801595
--- drivers/infiniband/ulp/srp/ib_srp.c | 82 ++++++++++++++++++++++++---------- 1 files changed, 58 insertions(+), 24 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +58 -24
@@ -586,24 +586,61 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd, scmnd->sc_data_direction); } -static void srp_remove_req (struct srp_target_port *target, -struct srp_request *req,s32 req_lim_delta) +/** +* srp_claim_req - Take ownership of the scmnd associated with a request. +* @target: SRP target port. +* @req: SRP request. +* @scmnd: If NULL, take ownership of @req->scmnd. If not NULL, only take +* ownership of @req->scmnd if it equals @scmnd. +* @req_lim_delta: target->req_lim_delta increment. +* +* Return value: +* Either NULL or a pointer to the SCSI command the caller became owner of. +*/ +static struct scsi_cmnd *srp_claim_req (struct srp_target_port *target, +struct srp_request *req, +struct scsi_cmnd *scmnd, +s32 req_lim_delta) {unsigned long flags; -srp_unmap_data (req->scmnd, target, req);spin_lock_irqsave (&target->lock, flags); target->req_lim += req_lim_delta; - req->scmnd =NULL ; +if (!scmnd) +swap (scmnd, req->scmnd); +else if (req->scmnd == scmnd) + req->scmnd =NULL ; +else + scmnd =NULL ; +spin_unlock_irqrestore (&target->lock, flags); + +return scmnd; +} + +/** +* srp_free_req() - Unmap data and add request to the free request list. +*/ +static void srp_free_req (struct srp_target_port *target, +struct srp_request *req,struct scsi_cmnd *scmnd) +{ +unsigned long flags; + +srp_unmap_data (scmnd, target, req); + +spin_lock_irqsave (&target->lock, flags);list_add_tail (&req->list, &target->free_reqs);spin_unlock_irqrestore (&target->lock, flags); }static void srp_reset_req (struct srp_target_port *target,struct srp_request *req) { - req->scmnd->result =DID_RESET <<16 ; - req->scmnd->scsi_done (req->scmnd); -srp_remove_req (target, req,0 ); +struct scsi_cmnd *scmnd = req->scmnd; + +if (srp_claim_req (target, req, scmnd,0 )) { + scmnd->result =DID_RESET <<16 ; + scmnd->scsi_done (scmnd); +srp_free_req (target, req, scmnd); + } }static int srp_reconnect_target (struct srp_target_port *target)
@@ -1073,11 +1110,14 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)complete (&target->tsk_mgmt_done); }else { req = &target->req_ring[rsp->tag]; - scmnd = req->scmnd; -if (!scmnd) + scmnd =srp_claim_req (target, req,NULL , +be32_to_cpu (rsp->req_lim_delta)); +if (!scmnd) {shost_printk (KERN_ERR , target->scsi_host,"Null scmnd for RSP w/tag %016llx\n" , (unsigned long long ) rsp->tag); +return ; + } scmnd->result = rsp->status;if (rsp->flags &SRP_RSP_FLAG_SNSVALID ) {
@@ -1092,7 +1132,8 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)else if (rsp->flags & (SRP_RSP_FLAG_DIOVER |SRP_RSP_FLAG_DIUNDER ))scsi_set_resid (scmnd,be32_to_cpu (rsp->data_in_res_cnt)); -srp_remove_req (target, req,be32_to_cpu (rsp->req_lim_delta)); +srp_free_req (target, req, scmnd); + scmnd->host_scribble =NULL ; scmnd->scsi_done (scmnd); }
@@ -1631,25 +1672,18 @@ static int srp_abort(struct scsi_cmnd *scmnd) {struct srp_target_port *target =host_to_target (scmnd->device->host);struct srp_request *req = (struct srp_request *) scmnd->host_scribble; -int ret =SUCCESS ;shost_printk (KERN_ERR , target->scsi_host,"SRP abort called\n" ); -if (!req || target->qp_in_error) +if (!req || target->qp_in_error || + !srp_claim_req (target, req, scmnd,0 ))return FAILED ; -if (srp_send_tsk_mgmt (target, req->index, scmnd->device->lun, -SRP_TSK_ABORT_TASK )) -return FAILED ; - -if (req->scmnd) { -if (!target->tsk_mgmt_status) { -srp_remove_req (target, req,0 ); - scmnd->result =DID_ABORT <<16 ; - }else - ret =FAILED ; - } +srp_send_tsk_mgmt (target, req->index, scmnd->device->lun, +SRP_TSK_ABORT_TASK ); +srp_free_req (target, req, scmnd); + scmnd->result =DID_ABORT <<16 ; -return ret; +return SUCCESS ; }static int srp_reset_device (struct scsi_cmnd *scmnd)
Signature · 7 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 01/20] ib_srp: Fix a race condition
Quoted 3 lines · Bart Van Assche
Avoid that the
scmnd->scsi_done(scmnd)call insrp_process_rsp()can trigger a crash by being invoked withscsi_done== NULL. That could happen if a reply is received during or after a command abort.
Please leave the call trace out of the commit message; it adds nothing.
drivers/infiniband/ulp/srp/ib_srp.c static void srp_unmap_data(struct scsi_cmnd *scmnd, · line 608
Quoted 10 lines
drivers/infiniband/ulp/srp/ib_srp.c +4
-1+static struct scsi_cmnd *srp_claim_req (struct srp_target_port *target, +struct srp_request *req, +struct scsi_cmnd *scmnd, +s32 req_lim_delta) {unsigned long flags; -srp_unmap_data (req->scmnd, target, req);spin_lock_irqsave (&target->lock, flags); target->req_lim += req_lim_delta;
I still think that adding credits back before putting the request on the free list is a crash waiting to happen. We don't check to see if the free request list is empty in srp_queuecommand(), so this creates a new race window where we get a response that gives us credits back, but we don't have a free request to back them.
Arguably, we need to check for that anyway to protect against buggy targets giving us back more credits than we have used...
Or am I missing something obvious?
Signature · 9 lines
Dave Dillow National Center for Computational Science Oak Ridge National Laboratory (865) 241-6602 office -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 1344914386.31833.45.camel-1q1vX8mYZiGLUyTwlgNVppKKF0rrzTr+@public.gmane.org
Re: [PATCH 01/20] ib_srp: Fix a race condition
Quoted 8 lines · David Dillow
Quoted 7 lines · Bart Van Assche
drivers/infiniband/ulp/srp/ib_srp.c +4
-0+static struct scsi_cmnd *srp_claim_req (struct srp_target_port *target, +struct srp_request *req, +struct scsi_cmnd *scmnd, +s32 req_lim_delta) {unsigned long flags;
Quoted 9 lines
drivers/infiniband/ulp/srp/ib_srp.c static void srp_unmap_data(struct scsi_cmnd *scmnd, · line 608
Quoted 3 lines
drivers/infiniband/ulp/srp/ib_srp.c +0
-1-srp_unmap_data (req->scmnd, target, req);spin_lock_irqsave (&target->lock, flags); target->req_lim += req_lim_delta;I still think that adding credits back before putting the request on the free list is a crash waiting to happen. We don't check to see if the free request list is empty in
srp_queuecommand(), so this creates a new race window where we get a response that gives us credits back, but we don't have a free request to back them.
I've just posted an updated patch in which the above comment has been addressed, and I've also pushed out a rebased and retested tree to
http://github.com/bvanassche/linux/tree/srp-ha.
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 1/20 ib_srp: Fix a race condition
In reply to a message not in the archiveAvoid that the scmnd->scsi_done(scmnd) call in srp_process_rsp() can trigger a crash by being invoked with scsi_done == NULL. That could happen if a reply is received during or after a command abort.
Reference: http://marc.info/?l=linux-rdma&m=134314367801595
--- drivers/infiniband/ulp/srp/ib_srp.c | 87 +++++++++++++++++++++++++---------- 1 files changed, 63 insertions(+), 24 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +63 -24
@@ -586,24 +586,62 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd, scmnd->sc_data_direction); } -static void srp_remove_req (struct srp_target_port *target, -struct srp_request *req,s32 req_lim_delta) +/** +* srp_claim_req - Take ownership of the scmnd associated with a request. +* @target: SRP target port. +* @req: SRP request. +* @scmnd: If NULL, take ownership of @req->scmnd. If not NULL, only take +* ownership of @req->scmnd if it equals @scmnd. +* +* Return value: +* Either NULL or a pointer to the SCSI command the caller became owner of. +*/ +static struct scsi_cmnd *srp_claim_req (struct srp_target_port *target, +struct srp_request *req, +struct scsi_cmnd *scmnd) +{ +unsigned long flags; + +spin_lock_irqsave (&target->lock, flags); +if (!scmnd) { + scmnd = req->scmnd; + req->scmnd =NULL ; + }else if (req->scmnd == scmnd) { + req->scmnd =NULL ; + }else { + scmnd =NULL ; + } +spin_unlock_irqrestore (&target->lock, flags); + +return scmnd; +} + +/** +* srp_free_req() - Unmap data and add request to the free request list. +*/ +static void srp_free_req (struct srp_target_port *target, +struct srp_request *req,struct scsi_cmnd *scmnd, +s32 req_lim_delta) {unsigned long flags; -srp_unmap_data (req->scmnd, target, req); +srp_unmap_data (scmnd, target, req); +spin_lock_irqsave (&target->lock, flags); target->req_lim += req_lim_delta; - req->scmnd =NULL ;list_add_tail (&req->list, &target->free_reqs);spin_unlock_irqrestore (&target->lock, flags); }static void srp_reset_req (struct srp_target_port *target,struct srp_request *req) { - req->scmnd->result =DID_RESET <<16 ; - req->scmnd->scsi_done (req->scmnd); -srp_remove_req (target, req,0 ); +struct scsi_cmnd *scmnd =srp_claim_req (target, req,NULL ); + +if (scmnd) { + scmnd->result =DID_RESET <<16 ; + scmnd->scsi_done (scmnd); +srp_free_req (target, req, scmnd,0 ); + } }static int srp_reconnect_target (struct srp_target_port *target)
@@ -1073,11 +1111,18 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)complete (&target->tsk_mgmt_done); }else { req = &target->req_ring[rsp->tag]; - scmnd = req->scmnd; -if (!scmnd) + scmnd =srp_claim_req (target, req,NULL ); +if (!scmnd) {shost_printk (KERN_ERR , target->scsi_host,"Null scmnd for RSP w/tag %016llx\n" , (unsigned long long ) rsp->tag); + +spin_lock_irqsave (&target->lock, flags); + target->req_lim +=be32_to_cpu (rsp->req_lim_delta); +spin_unlock_irqrestore (&target->lock, flags); + +return ; + } scmnd->result = rsp->status;if (rsp->flags &SRP_RSP_FLAG_SNSVALID ) {
@@ -1092,7 +1137,9 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)else if (rsp->flags & (SRP_RSP_FLAG_DIOVER |SRP_RSP_FLAG_DIUNDER ))scsi_set_resid (scmnd,be32_to_cpu (rsp->data_in_res_cnt)); -srp_remove_req (target, req,be32_to_cpu (rsp->req_lim_delta)); +srp_free_req (target, req, scmnd, +be32_to_cpu (rsp->req_lim_delta)); + scmnd->host_scribble =NULL ; scmnd->scsi_done (scmnd); }
@@ -1631,25 +1678,17 @@ static int srp_abort(struct scsi_cmnd *scmnd) {struct srp_target_port *target =host_to_target (scmnd->device->host);struct srp_request *req = (struct srp_request *) scmnd->host_scribble; -int ret =SUCCESS ;shost_printk (KERN_ERR , target->scsi_host,"SRP abort called\n" ); -if (!req || target->qp_in_error) +if (!req || target->qp_in_error || !srp_claim_req (target, req, scmnd))return FAILED ; -if (srp_send_tsk_mgmt (target, req->index, scmnd->device->lun, -SRP_TSK_ABORT_TASK )) -return FAILED ; - -if (req->scmnd) { -if (!target->tsk_mgmt_status) { -srp_remove_req (target, req,0 ); - scmnd->result =DID_ABORT <<16 ; - }else - ret =FAILED ; - } +srp_send_tsk_mgmt (target, req->index, scmnd->device->lun, +SRP_TSK_ABORT_TASK ); +srp_free_req (target, req, scmnd,0 ); + scmnd->result =DID_ABORT <<16 ; -return ret; +return SUCCESS ; }static int srp_reset_device (struct scsi_cmnd *scmnd)
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
David Dillow
Acked-byRe: [PATCH 01/20 v4b] ib_srp: Fix a race condition
Quoted 3 lines · Bart Van Assche
Avoid that the
scmnd->scsi_done(scmnd)call insrp_process_rsp()can trigger a crash by being invoked withscsi_done== NULL. That could happen if a reply is received during or after a command abort.
Thanks!
Signature · 9 lines
Dave Dillow National Center for Computational Science Oak Ridge National Laboratory (865) 241-6602 office -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 2/20 ib_srp: Enlarge block layer timeout
In reply to a message not in the archiveEnlarge the block layer timeout for disks such that it is above the InfiniBand transport layer timeout.
--- drivers/infiniband/ulp/srp/ib_srp.c | 45 +++++++++++++++++++++++++++++++++++ drivers/infiniband/ulp/srp/ib_srp.h | 2 + 2 files changed, 47 insertions(+), 0 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +45 -0
@@ -1413,6 +1413,33 @@ err:return -ENOMEM ; } +static uint32_t srp_compute_rq_tmo (struct ib_qp_attr *qp_attr,int attr_mask) +{ +uint64_t T_tr_ns, max_compl_time_ms; +uint32_t rq_tmo_jiffies; + +/* +* According to section 11.2.4.2 in the IBTA spec (Modify Queue Pair, +* table 91), both the QP timeout and the retry count have to be set +* for RC QP's during the RTR to RTS transition. +*/ +WARN_ON ((attr_mask & (IB_QP_TIMEOUT |IB_QP_RETRY_CNT )) != + (IB_QP_TIMEOUT |IB_QP_RETRY_CNT )); + +/* +* Set target->rq_tmo_jiffies to one second more than the largest time +* it can take before an error completion is generated. See also +* C9-140..142 in the IBTA spec for more information about how to +* convert the QP Local ACK Timeout value to nanoseconds. +*/ + T_tr_ns =4096 * (1ULL << qp_attr->timeout); + max_compl_time_ms = qp_attr->retry_cnt *4 * T_tr_ns; +do_div (max_compl_time_ms,NSEC_PER_MSEC ); + rq_tmo_jiffies =msecs_to_jiffies (max_compl_time_ms +1000 ); + +return rq_tmo_jiffies; +} +static void srp_cm_rep_handler (struct ib_cm_id *cm_id,struct srp_login_rsp *lrsp,struct srp_target_port *target)
@@ -1472,6 +1499,8 @@ static void srp_cm_rep_handler(struct ib_cm_id *cm_id,if (ret) goto error_free; + target->rq_tmo_jiffies =srp_compute_rq_tmo (qp_attr, attr_mask); + ret =ib_modify_qp (target->qp, qp_attr, attr_mask);if (ret) goto error_free;
@@ -1723,6 +1752,21 @@ static int srp_reset_host(struct scsi_cmnd *scmnd)return ret; } +static int srp_slave_configure (struct scsi_device *sdev) +{ +struct Scsi_Host *shost = sdev->host; +struct srp_target_port *target =host_to_target (shost); +struct request_queue *q = sdev->request_queue; +unsigned long timeout; + +if (sdev->type ==TYPE_DISK ) { + timeout =max_t (unsigned,30 *HZ , target->rq_tmo_jiffies); +blk_queue_rq_timeout (q, timeout); + } + +return 0 ; +} +static ssize_t show_id_ext (struct device *dev,struct device_attribute *attr,char *buf) {
@@ -1855,6 +1899,7 @@ static struct scsi_host_template srp_template = { .module = THIS_MODULE, .name = "InfiniBand SRP initiator", .proc_name = DRV_NAME, + .slave_configure = srp_slave_configure, .info = srp_target_info, .queuecommand = srp_queuecommand, .eh_abort_handler = srp_abort,
drivers/infiniband/ulp/srp/ib_srp.h +2 -0
@@ -163,6 +163,8 @@ struct srp_target_port {struct ib_sa_query *path_query;int path_query_id; +u32 rq_tmo_jiffies; +struct ib_cm_id *cm_id;int max_ti_iu_len;
Signature · 7 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 3/20 ib_srp: Move QP state check into srp_send_tsk_mgmt()
In reply to a message not in the archiveTest the QP state inside srp_send_tsk_mgmt() instead of letting each caller perform that test.
--- drivers/infiniband/ulp/srp/ib_srp.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +3 -5
@@ -1660,7 +1660,8 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,struct srp_tsk_mgmt *tsk_mgmt;if (target->state ==SRP_TARGET_DEAD || - target->state ==SRP_TARGET_REMOVED ) + target->state ==SRP_TARGET_REMOVED || + target->qp_in_error)return -1 ;init_completion (&target->tsk_mgmt_done);
@@ -1704,8 +1705,7 @@ static int srp_abort(struct scsi_cmnd *scmnd)shost_printk (KERN_ERR , target->scsi_host,"SRP abort called\n" ); -if (!req || target->qp_in_error || - !srp_claim_req (target, req, scmnd,0 )) +if (!req || !srp_claim_req (target, req, scmnd,0 ))return FAILED ;srp_send_tsk_mgmt (target, req->index, scmnd->device->lun,SRP_TSK_ABORT_TASK );
@@ -1722,8 +1722,6 @@ static int srp_reset_device(struct scsi_cmnd *scmnd)shost_printk (KERN_ERR , target->scsi_host,"SRP reset_device called\n" ); -if (target->qp_in_error) -return FAILED ;if (srp_send_tsk_mgmt (target,SRP_TAG_NO_REQ , scmnd->device->lun,SRP_TSK_LUN_RESET ))return FAILED ;
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 4/20 ib_srp: Stop queueing if QP in error
In reply to a message not in the archiveFail SCSI commands if the QP between initiator and target is in the error state. For connections monitored by the multipath software the result is that multipathd doesn't have to wait until the SCSI timeout has expired to find out that a path has failed. Also, introduce the function srp_handle_qp_err(), change the type of qp_in_error from int into bool and move the initialization of that variable from srp_reconnect_target() to srp_connect_target().
--- drivers/infiniband/ulp/srp/ib_srp.c | 40 +++++++++++++++++++--------------- drivers/infiniband/ulp/srp/ib_srp.h | 2 +- 2 files changed, 23 insertions(+), 19 deletions(-)
(quoted diff) +0 -0
b/drivers/infiniband/ulp/srp/ib_srp.c
index f333705..28a044e 100644
drivers/infiniband/ulp/srp/ib_srp.c +0 -0
@@ -515,6 +515,8 @@ static int srp_connect_target(struct srp_target_port
*target)
int retries = 3;
int ret;
+ target->qp_in_error = false;
+
ret = srp_lookup_path(target);
if (ret)
return ret;
@@ -685,7 +687,6 @@ static int srp_reconnect_target(struct srp_target_port *target)
for (i = 0; i < SRP_SQ_SIZE; ++i)
list_add(&target->tx_ring[i]->list, &target->free_tx);
- target->qp_in_error = 0;
ret = srp_connect_target(target);
if (ret)
goto err;
@@ -1256,6 +1257,15 @@ static void srp_handle_recv(struct srp_target_port *target, struct ib_wc *wc)
PFX "Recv failed with error code %d\n", res);
}
+static void srp_handle_qp_err(enum ib_wc_status wc_status,
+ enum ib_wc_opcode wc_opcode,
+ struct srp_target_port *target)
+{
+ shost_printk(KERN_ERR, target->scsi_host, PFX "failed %s status %d\n",
+ wc_opcode & IB_WC_RECV ? "receive" : "send", wc_status);
+ target->qp_in_error = true;
+}
+
static void srp_recv_completion(struct ib_cq *cq, void *target_ptr) {
struct srp_target_port *target = target_ptr;
@@ -1263,15 +1273,12 @@ static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)
ib_req_notify_cq(cq, IB_CQ_NEXT_COMP);
while (ib_poll_cq(cq, 1, &wc) > 0) {
- if (wc.status) {
- shost_printk(KERN_ERR, target->scsi_host,
- PFX "failed receive status %d\n",
- wc.status);
- target->qp_in_error = 1;
+ if (likely(wc.status == IB_WC_SUCCESS)) {
+ srp_handle_recv(target, &wc);
+ } else {
+ srp_handle_qp_err(wc.status, wc.opcode, target);
break;
}
-
- srp_handle_recv(target, &wc);
}
}
@@ -1282,16 +1289,13 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr)
struct srp_iu *iu;
while (ib_poll_cq(cq, 1, &wc) > 0) {
- if (wc.status) {
- shost_printk(KERN_ERR, target->scsi_host,
- PFX "failed send status %d\n",
- wc.status);
- target->qp_in_error = 1;
+ if (likely(wc.status == IB_WC_SUCCESS)) {
+ iu = (struct srp_iu *) (uintptr_t) wc.wr_id;
+ list_add(&iu->list, &target->free_tx);
+ } else {
+ srp_handle_qp_err(wc.status, wc.opcode, target);
break;
}
-
- iu = (struct srp_iu *) (uintptr_t) wc.wr_id;
- list_add(&iu->list, &target->free_tx);
}
}
@@ -1309,7 +1313,8 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
goto err;
if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED) {
+ target->state == SRP_TARGET_REMOVED ||
+ target->qp_in_error) { scmnd->result = DID_BAD_TARGET << 16; scmnd->scsi_done(scmnd);
return 0;
@@ -2269,7 +2274,6 @@ static ssize_t srp_create_target(struct device *dev,
if (ret)
goto err_free_ib;
- target->qp_in_error = 0;
ret = srp_connect_target(target);
if (ret) {
shost_printk(KERN_ERR, target->scsi_host,
(quoted diff) +0 -0
b/drivers/infiniband/ulp/srp/ib_srp.h
index e3a6304..f0daeb3 100644
drivers/infiniband/ulp/srp/ib_srp.h +1 -1
@@ -180,7 +180,7 @@ struct srp_target_port {struct list_head list;struct completion done;int status; -int qp_in_error; +bool qp_in_error;struct completion tsk_mgmt_done;u8 tsk_mgmt_status;
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 5/20 ib_srp: Eliminate state SRP_TARGET_CONNECTING
In reply to a message not in the archiveBlock the SCSI host while reconnecting instead of representing the reconnection activity as a distinct SRP target state. This allows to eliminate the target state SRP_TARGET_CONNECTING.
--- drivers/infiniband/ulp/srp/ib_srp.c | 16 ++++++++-------- drivers/infiniband/ulp/srp/ib_srp.h | 1 - 2 files changed, 8 insertions(+), 9 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +8 -8
@@ -647,13 +647,16 @@ static void srp_reset_req(struct srp_target_port *target, struct srp_request *restatic int srp_reconnect_target (struct srp_target_port *target) { +struct Scsi_Host *shost = target->scsi_host;struct ib_qp_attr qp_attr;struct ib_wc wc;int i, ret; -if (!srp_change_state (target,SRP_TARGET_LIVE ,SRP_TARGET_CONNECTING )) +if (target->state !=SRP_TARGET_LIVE )return -EAGAIN ; +scsi_target_block (&shost->shost_gendev); +srp_disconnect_target (target);/* * Now get a new local CM ID so that we avoid confusing the
@@ -691,12 +694,13 @@ static int srp_reconnect_target(struct srp_target_port *target)if (ret) goto err; -if (!srp_change_state (target,SRP_TARGET_CONNECTING ,SRP_TARGET_LIVE )) - ret = -EAGAIN ; +scsi_target_unblock (&shost->shost_gendev,SDEV_RUNNING );return ret; err: +scsi_target_unblock (&shost->shost_gendev,SDEV_TRANSPORT_OFFLINE ); +shost_printk (KERN_ERR , target->scsi_host,PFX "reconnect failed (%d), removing target port.\n" , ret);
@@ -710,7 +714,7 @@ err:* the flush_scheduled_work() in srp_remove_one(). */ spin_lock_irq (&target->lock); -if (target->state ==SRP_TARGET_CONNECTING ) { +if (target->state ==SRP_TARGET_LIVE ) { target->state =SRP_TARGET_DEAD ;INIT_WORK (&target->work, srp_remove_work);queue_work (ib_wq, &target->work);
@@ -1309,9 +1313,6 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)unsigned long flags;int len; -if (target->state ==SRP_TARGET_CONNECTING ) - goto err; -if (target->state ==SRP_TARGET_DEAD || target->state ==SRP_TARGET_REMOVED || target->qp_in_error) {
@@ -1376,7 +1377,6 @@ err_iu: err_unlock:spin_unlock_irqrestore (&target->lock, flags); -err:return SCSI_MLQUEUE_HOST_BUSY ; }
drivers/infiniband/ulp/srp/ib_srp.h +0 -1
@@ -80,7 +80,6 @@ enum {enum srp_target_state {SRP_TARGET_LIVE , -SRP_TARGET_CONNECTING ,SRP_TARGET_DEAD ,SRP_TARGET_REMOVED };
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 6/20 ib_srp: Suppress superfluous error messages
In reply to a message not in the archiveKeep track of the connection state. Only report QP errors while connected. Only invoke ib_send_cm_dreq() when connected such that invoking srp_disconnect_target() after having received a DREQ does not cause an error message to be printed.
--- drivers/infiniband/ulp/srp/ib_srp.c | 43 +++++++++++++++++++++++++++------- drivers/infiniband/ulp/srp/ib_srp.h | 1 + 2 files changed, 35 insertions(+), 9 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +34 -9
@@ -428,17 +428,34 @@ static int srp_send_req(struct srp_target_port *target)return status; } +static bool srp_change_conn_state (struct srp_target_port *target, +bool connected) +{ +bool changed = false; + +spin_lock_irq (&target->lock); +if (target->connected != connected) { + target->connected = connected; + changed = true; + } +spin_unlock_irq (&target->lock); + +return changed; +} +static void srp_disconnect_target (struct srp_target_port *target) { -/* XXX should send SRP_I_LOGOUT request */ +if (srp_change_conn_state (target, false)) { +/* XXX should send SRP_I_LOGOUT request */ -init_completion (&target->done); -if (ib_send_cm_dreq (target->cm_id,NULL ,0 )) { -shost_printk (KERN_DEBUG , target->scsi_host, -PFX "Sending CM DREQ failed\n" ); -return ; +init_completion (&target->done); +if (ib_send_cm_dreq (target->cm_id,NULL ,0 )) { +shost_printk (KERN_DEBUG , target->scsi_host, +PFX "Sending CM DREQ failed\n" ); + }else { +wait_for_completion (&target->done); + } } -wait_for_completion (&target->done); } static bool srp_change_state(struct srp_target_port *target,
@@ -515,6 +532,8 @@ static int srp_connect_target(struct srp_target_port *target)int retries =3 ;int ret; +WARN_ON (target->connected); + target->qp_in_error = false; ret =srp_lookup_path (target);
@@ -536,6 +555,7 @@ static int srp_connect_target(struct srp_target_port *target)*/ switch (target->status) {case 0 : +srp_change_conn_state (target, true);return 0 ;case SRP_PORT_REDIRECT :
@@ -1265,8 +1285,11 @@ static void srp_handle_qp_err(enum ib_wc_status wc_status, enum ib_wc_opcode wc_opcode, struct srp_target_port *target) { - shost_printk(KERN_ERR, target->scsi_host, PFX "failed %s status %d\n", - wc_opcode & IB_WC_RECV ? "receive" : "send", wc_status); +if (target->connected) +shost_printk (KERN_ERR , target->scsi_host, +PFX "failed %s status %d\n" , + wc_opcode &IB_WC_RECV ?"receive" :"send" , + wc_status); target->qp_in_error = true; }
@@ -1627,6 +1650,7 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)case IB_CM_DREQ_RECEIVED :shost_printk (KERN_WARNING , target->scsi_host,PFX "DREQ received - connection closed\n" ); +srp_change_conn_state (target, false);if (ib_send_cm_drep (cm_id,NULL ,0 ))shost_printk (KERN_ERR , target->scsi_host,PFX "Sending CM DREP failed\n" );
@@ -1941,6 +1965,7 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)spin_unlock (&host->target_lock); target->state =SRP_TARGET_LIVE ; + target->connected = false;scsi_scan_target (&target->scsi_host->shost_gendev,0 , target->scsi_id,SCAN_WILD_CARD ,0 );
drivers/infiniband/ulp/srp/ib_srp.h +1 -0
@@ -163,6 +163,7 @@ struct srp_target_port {int path_query_id;u32 rq_tmo_jiffies; +bool connected;struct ib_cm_id *cm_id;
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 7/20 ib_srp: Avoid that SCSI error handling triggers a crash
In reply to a message not in the archiveSending any data over a queue pair associated with a closed connection is wrong. The HCA will send the data anyway and such data may be sent to another system to a queue pair that is in use. The data will get processed and a response will be sent back. That can result in ib_srp complaining about "Null scmnd for RSP ..." followed by a kernel oops.
--- drivers/infiniband/ulp/srp/ib_srp.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +2 -0
@@ -1338,6 +1338,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)if (target->state ==SRP_TARGET_DEAD || target->state ==SRP_TARGET_REMOVED || + !target->connected || target->qp_in_error) { scmnd->result =DID_BAD_TARGET <<16 ; scmnd->scsi_done (scmnd);
@@ -1690,6 +1691,7 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,if (target->state ==SRP_TARGET_DEAD || target->state ==SRP_TARGET_REMOVED || + !target->connected || target->qp_in_error)return -1 ;
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 8/20 ib_srp: Introduce the helper function, srp_remove_target()
In reply to a message not in the archive--- drivers/infiniband/ulp/srp/ib_srp.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +12 -7
@@ -506,6 +506,17 @@ static void srp_del_scsi_host_attr(struct Scsi_Host *shost)device_remove_file (&shost->shost_dev, *attr); } +static void srp_remove_target (struct srp_target_port *target) +{ +srp_del_scsi_host_attr (target->scsi_host); +srp_remove_host (target->scsi_host); +scsi_remove_host (target->scsi_host); +ib_destroy_cm_id (target->cm_id); +srp_free_target_ib (target); +srp_free_req_data (target); +scsi_host_put (target->scsi_host); +} +static void srp_remove_work (struct work_struct *work) { struct srp_target_port *target =
@@ -518,13 +529,7 @@ static void srp_remove_work(struct work_struct *work)list_del (&target->list);spin_unlock (&target->srp_host->target_lock); -srp_del_scsi_host_attr (target->scsi_host); -srp_remove_host (target->scsi_host); -scsi_remove_host (target->scsi_host); -ib_destroy_cm_id (target->cm_id); -srp_free_target_ib (target); -srp_free_req_data (target); -scsi_host_put (target->scsi_host); +srp_remove_target (target); }static int srp_connect_target (struct srp_target_port *target)
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 9/20 ib_srp: Eliminate state SRP_TARGET_DEAD
In reply to a message not in the archiveOnly queue removal work after having changed the target state into SRP_TARGET_REMOVED and not if that state was already equal to SRP_TARGET_REMOVED. That allows to remove the state SRP_TARGET_DEAD. Add a call to srp_disconnect_target() in srp_remove_target() - due to previous changes it is now safe to invoke that last function even if the IB connection has already been disconnected. This change allows to replace the target removal code in srp_remove_one() by an (indirect) call to srp_remove_target(). Rename srp_target_port.work into srp_target_port.remove_work to reflect its usage.
--- drivers/infiniband/ulp/srp/ib_srp.c | 89 ++++++++++++---------------------- drivers/infiniband/ulp/srp/ib_srp.h | 5 +- 2 files changed, 34 insertions(+), 60 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +32 -57
@@ -428,6 +428,23 @@ static int srp_send_req(struct srp_target_port *target)return status; } +static bool srp_queue_remove_work (struct srp_target_port *target) +{ +bool changed = false; + +spin_lock_irq (&target->lock); +if (target->state !=SRP_TARGET_REMOVED ) { + target->state =SRP_TARGET_REMOVED ; + changed = true; + } +spin_unlock_irq (&target->lock); + +if (changed) +queue_work (system_long_wq, &target->remove_work); + +return changed; +} +static bool srp_change_conn_state (struct srp_target_port *target,bool connected) {
@@ -458,21 +475,6 @@ static void srp_disconnect_target(struct srp_target_port *target) } } -static bool srp_change_state (struct srp_target_port *target, -enum srp_target_state old, -enum srp_target_state new) -{ -bool changed = false; - -spin_lock_irq (&target->lock); -if (target->state == old) { - target->state = new; - changed = true; - } -spin_unlock_irq (&target->lock); -return changed; -} -static void srp_free_req_data (struct srp_target_port *target) {struct ib_device *ibdev = target->srp_host->srp_dev->dev;
@@ -508,9 +510,12 @@ static void srp_del_scsi_host_attr(struct Scsi_Host *shost)static void srp_remove_target (struct srp_target_port *target) { +WARN_ON (target->state !=SRP_TARGET_REMOVED ); +srp_del_scsi_host_attr (target->scsi_host);srp_remove_host (target->scsi_host);scsi_remove_host (target->scsi_host); +srp_disconnect_target (target);ib_destroy_cm_id (target->cm_id);srp_free_target_ib (target);srp_free_req_data (target);
@@ -520,10 +525,9 @@ static void srp_remove_target(struct srp_target_port *target)static void srp_remove_work (struct work_struct *work) {struct srp_target_port *target = - container_of(work, struct srp_target_port, work); + container_of(work, struct srp_target_port, remove_work); -if (!srp_change_state (target,SRP_TARGET_DEAD ,SRP_TARGET_REMOVED )) -return ; +WARN_ON (target->state !=SRP_TARGET_REMOVED );spin_lock (&target->srp_host->target_lock);list_del (&target->list);
@@ -734,17 +738,8 @@ err:* However, we have to defer the real removal because we * are in the context of the SCSI error handler now, which * will deadlock if we call scsi_remove_host(). -* -* Schedule our work inside the lock to avoid a race with -* the flush_scheduled_work() in srp_remove_one(). */ -spin_lock_irq (&target->lock); -if (target->state ==SRP_TARGET_LIVE ) { - target->state =SRP_TARGET_DEAD ; -INIT_WORK (&target->work, srp_remove_work); -queue_work (ib_wq, &target->work); - } -spin_unlock_irq (&target->lock); +srp_queue_remove_work (target);return ret; }
@@ -1341,8 +1336,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)unsigned long flags;int len; -if (target->state ==SRP_TARGET_DEAD || - target->state ==SRP_TARGET_REMOVED || +if (target->state ==SRP_TARGET_REMOVED || !target->connected || target->qp_in_error) { scmnd->result =DID_BAD_TARGET <<16 ;
@@ -1694,8 +1688,7 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,struct srp_iu *iu;struct srp_tsk_mgmt *tsk_mgmt; -if (target->state ==SRP_TARGET_DEAD || - target->state ==SRP_TARGET_REMOVED || +if (target->state ==SRP_TARGET_REMOVED || !target->connected || target->qp_in_error)return -1 ;
@@ -2262,6 +2255,7 @@ static ssize_t srp_create_target(struct device *dev,sizeof (struct srp_indirect_buf ) + target->cmd_sg_cnt *sizeof (struct srp_direct_buf ); +INIT_WORK (&target->remove_work, srp_remove_work);spin_lock_init (&target->lock);INIT_LIST_HEAD (&target->free_tx);INIT_LIST_HEAD (&target->free_reqs);
@@ -2495,8 +2489,7 @@ static void srp_remove_one(struct ib_device *device) {struct srp_device *srp_dev;struct srp_host *host, *tmp_host; -LIST_HEAD (target_list); -struct srp_target_port *target, *tmp_target; +struct srp_target_port *target; srp_dev =ib_get_client_data (device, &srp_client);
@@ -2509,35 +2502,17 @@ static void srp_remove_one(struct ib_device *device)wait_for_completion (&host->released);/* -* Mark all target ports as removed, so we stop queueing -* commands and don't try to reconnect. +* Remove all target ports. */ spin_lock (&host->target_lock); -list_for_each_entry (target, &host->target_list, list) { -spin_lock_irq (&target->lock); - target->state =SRP_TARGET_REMOVED ; -spin_unlock_irq (&target->lock); - } +list_for_each_entry (target, &host->target_list, list) +srp_queue_remove_work (target);spin_unlock (&host->target_lock);/* -* Wait for any reconnection tasks that may have -* started before we marked our target ports as -* removed, and any target port removal tasks. +* Wait for target port removal tasks. */ -flush_workqueue (ib_wq); - -list_for_each_entry_safe (target, tmp_target, - &host->target_list, list) { -srp_del_scsi_host_attr (target->scsi_host); -srp_remove_host (target->scsi_host); -scsi_remove_host (target->scsi_host); -srp_disconnect_target (target); -ib_destroy_cm_id (target->cm_id); -srp_free_target_ib (target); -srp_free_req_data (target); -scsi_host_put (target->scsi_host); - } +flush_workqueue (system_long_wq);kfree (host); }
drivers/infiniband/ulp/srp/ib_srp.h +2 -3
@@ -80,8 +80,7 @@ enum {enum srp_target_state {SRP_TARGET_LIVE , -SRP_TARGET_DEAD , -SRP_TARGET_REMOVED +SRP_TARGET_REMOVED , };enum srp_iu_type {
@@ -175,7 +174,7 @@ struct srp_target_port {struct srp_iu *rx_ring[SRP_RQ_SIZE ];struct srp_request req_ring[SRP_CMD_SQ_SIZE ]; -struct work_struct work; +struct work_struct remove_work;struct list_head list;struct completion done;
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 10/20 ib_srp: Keep processing commands during scsi_remove_host()
In reply to a message not in the archiveSome SCSI upper layer drivers, e.g. sd, issue SCSI commands from inside scsi_remove_host() (see also the sd_shutdown() call in sd_remove()). Make sure that these commands have a chance to reach the SCSI device.
--- drivers/infiniband/ulp/srp/ib_srp.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +2 -6
@@ -1336,9 +1336,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)unsigned long flags;int len; -if (target->state ==SRP_TARGET_REMOVED || - !target->connected || - target->qp_in_error) { +if (!target->connected || target->qp_in_error) { scmnd->result =DID_BAD_TARGET <<16 ; scmnd->scsi_done (scmnd);return 0 ;
@@ -1688,9 +1686,7 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,struct srp_iu *iu;struct srp_tsk_mgmt *tsk_mgmt; -if (target->state ==SRP_TARGET_REMOVED || - !target->connected || - target->qp_in_error) +if (!target->connected || target->qp_in_error)return -1 ;init_completion (&target->tsk_mgmt_done);
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 11/20 ib_srp: Make srp_disconnect_target() wait for IB completions
In reply to a message not in the archiveModify srp_disconnect_target() such that it waits until it is sure that no new IB completions will be received anymore.
--- drivers/infiniband/ulp/srp/ib_srp.c | 104 ++++++++++++++++++++++++++++++----- drivers/infiniband/ulp/srp/ib_srp.h | 6 ++ 2 files changed, 95 insertions(+), 15 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +89 -15
@@ -40,7 +40,7 @@#include <linux/parser.h> #include <linux/random.h> #include <linux/jiffies.h> - +#include <linux/delay.h> #include <linux/atomic.h> #include <scsi/scsi.h>
@@ -229,14 +229,16 @@ static int srp_create_target_ib(struct srp_target_port *target)return -ENOMEM ; target->recv_cq =ib_create_cq (target->srp_host->srp_dev->dev, - srp_recv_completion,NULL , target,SRP_RQ_SIZE ,0 ); + srp_recv_completion,NULL , target, +SRP_RQ_SIZE +1 ,0 );if (IS_ERR (target->recv_cq)) { ret =PTR_ERR (target->recv_cq); goto err; } target->send_cq =ib_create_cq (target->srp_host->srp_dev->dev, - srp_send_completion,NULL , target,SRP_SQ_SIZE ,0 ); + srp_send_completion,NULL , target, +SRP_SQ_SIZE +1 ,0 );if (IS_ERR (target->send_cq)) { ret =PTR_ERR (target->send_cq); goto err_recv_cq;
@@ -245,8 +247,8 @@ static int srp_create_target_ib(struct srp_target_port *target)ib_req_notify_cq (target->recv_cq,IB_CQ_NEXT_COMP ); init_attr->event_handler = srp_qp_event; - init_attr->cap.max_send_wr =SRP_SQ_SIZE ; - init_attr->cap.max_recv_wr =SRP_RQ_SIZE ; + init_attr->cap.max_send_wr =SRP_SQ_SIZE +1 ; + init_attr->cap.max_recv_wr =SRP_RQ_SIZE +1 ; init_attr->cap.max_recv_sge =1 ; init_attr->cap.max_send_sge =1 ; init_attr->sq_sig_type =IB_SIGNAL_ALL_WR ;
@@ -460,11 +462,69 @@ static bool srp_change_conn_state(struct srp_target_port *target,return changed; } +static void srp_wait_last_recv_wqe (struct srp_target_port *target) +{ +static struct ib_recv_wr wr = { + .wr_id =SRP_LAST_RECV , + }; +struct ib_recv_wr *bad_wr; +int ret; + +if (target->last_recv_wqe) +return ; + + ret =ib_post_recv (target->qp, &wr, &bad_wr); +if (ret <0 ) { +shost_printk (KERN_ERR , target->scsi_host, +"ib_post_recv() failed (%d)\n" , ret); +return ; + } + + ret =wait_event_timeout (target->qp_wq, target->last_recv_wqe, + target->rq_tmo_jiffies); +WARN (ret <=0 ,"Timeout while waiting for last recv WQE (ret = %d)\n" , + ret); +} + +static void srp_wait_last_send_wqe (struct srp_target_port *target) +{ +static struct ib_send_wr wr = { + .wr_id =SRP_LAST_SEND , + }; +struct ib_send_wr *bad_wr; +unsigned long deadline = jiffies + target->rq_tmo_jiffies; +int ret; + +if (target->last_send_wqe) +return ; + + ret =ib_post_send (target->qp, &wr, &bad_wr); +if (ret <0 ) { +shost_printk (KERN_ERR , target->scsi_host, +"ib_post_send() failed (%d)\n" , ret); +return ; + } + +while (!target->last_send_wqe &&time_before (jiffies, deadline)) { +srp_send_completion (target->send_cq, target); +msleep (20 ); + } + +WARN_ON (!target->last_send_wqe); +} +static void srp_disconnect_target (struct srp_target_port *target) { +static struct ib_qp_attr qp_attr = { + .qp_state =IB_QPS_ERR + }; +int ret; +if (srp_change_conn_state (target, false)) {/* XXX should send SRP_I_LOGOUT request */ +BUG_ON (!target->cm_id); +init_completion (&target->done);if (ib_send_cm_dreq (target->cm_id,NULL ,0 )) { shost_printk(KERN_DEBUG, target->scsi_host,
@@ -473,6 +533,20 @@ static void srp_disconnect_target(struct srp_target_port *target)wait_for_completion (&target->done); } } + +if (target->cm_id) { +ib_destroy_cm_id (target->cm_id); + target->cm_id =NULL ; + } + +if (target->qp) { + ret =ib_modify_qp (target->qp, &qp_attr,IB_QP_STATE ); +WARN (ret !=0 ,"ib_modify_qp() failed: %d\n" , ret); + +srp_wait_last_recv_wqe (target); + +srp_wait_last_send_wqe (target); + } }static void srp_free_req_data (struct srp_target_port *target)
@@ -516,7 +590,6 @@ static void srp_remove_target(struct srp_target_port *target)srp_remove_host (target->scsi_host);scsi_remove_host (target->scsi_host);srp_disconnect_target (target); -ib_destroy_cm_id (target->cm_id);srp_free_target_ib (target);srp_free_req_data (target);scsi_host_put (target->scsi_host);
@@ -544,6 +617,8 @@ static int srp_connect_target(struct srp_target_port *target)WARN_ON (target->connected); target->qp_in_error = false; + target->last_recv_wqe = false; + target->last_send_wqe = false; ret =srp_lookup_path (target);if (ret)
@@ -678,7 +753,6 @@ static int srp_reconnect_target(struct srp_target_port *target) {struct Scsi_Host *shost = target->scsi_host;struct ib_qp_attr qp_attr; -struct ib_wc wc;int i, ret;if (target->state !=SRP_TARGET_LIVE )
@@ -704,11 +778,6 @@ static int srp_reconnect_target(struct srp_target_port *target)if (ret) goto err; -while (ib_poll_cq (target->recv_cq,1 , &wc) >0 ) - ;/* nothing */ -while (ib_poll_cq (target->send_cq,1 , &wc) >0 ) - ;/* nothing */ -for (i =0 ; i <SRP_CMD_SQ_SIZE ; ++i) {struct srp_request *req = &target->req_ring[i];if (req->scmnd)
@@ -1285,7 +1354,7 @@ static void srp_handle_qp_err(enum ib_wc_status wc_status, enum ib_wc_opcode wc_opcode, struct srp_target_port *target) { -if (target->connected) +if (target->connected && !target->qp_in_error) shost_printk(KERN_ERR, target->scsi_host, PFX "failed %s status %d\n", wc_opcode & IB_WC_RECV ? "receive" : "send",
@@ -1303,8 +1372,11 @@ static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)if (likely (wc.status ==IB_WC_SUCCESS )) {srp_handle_recv (target, &wc); }else { +if (wc.wr_id ==SRP_LAST_RECV ) { + target->last_recv_wqe = true; +wake_up (&target->qp_wq); + }srp_handle_qp_err (wc.status, wc.opcode, target); -break ; } } }
@@ -1320,8 +1392,9 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr) iu = (struct srp_iu *) (uintptr_t ) wc.wr_id;list_add (&iu->list, &target->free_tx); }else { +if (wc.wr_id ==SRP_LAST_SEND ) + target->last_send_wqe = true;srp_handle_qp_err (wc.status, wc.opcode, target); -break ; } } }
@@ -2255,6 +2328,7 @@ static ssize_t srp_create_target(struct device *dev,spin_lock_init (&target->lock);INIT_LIST_HEAD (&target->free_tx);INIT_LIST_HEAD (&target->free_reqs); +init_waitqueue_head (&target->qp_wq);for (i =0 ; i <SRP_CMD_SQ_SIZE ; ++i) {struct srp_request *req = &target->req_ring[i];
drivers/infiniband/ulp/srp/ib_srp.h +6 -0
@@ -76,6 +76,9 @@ enum {SRP_MAP_ALLOW_FMR =0 ,SRP_MAP_NO_FMR =1 , + +SRP_LAST_RECV =0 , + SRP_LAST_SEND = 0, }; enum srp_target_state {
@@ -180,6 +183,9 @@ struct srp_target_port {struct completion done;int status;bool qp_in_error; +bool last_recv_wqe; +bool last_send_wqe; +wait_queue_head_t qp_wq;struct completion tsk_mgmt_done;u8 tsk_mgmt_status;
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 11/20] ib_srp: Make srp_disconnect_target() wait for IB completions
Hi Bart,
we've triggered the WARN_ON() in srp_wait_last_send_wqe() by connecting to a disabled SCST SRP target.
I would remove that one.
Cheers,
Sebastian
Quoted 97 lines · Bart Van Assche
Modify
srp_disconnect_target()such that it waits until it is sure that no new IB completions will be received anymore.Signed-off-by Bart Van Assche <[email protected]>Cc David Dillow <[email protected]>Cc Roland Dreier <[email protected]>--- drivers/infiniband/ulp/srp/ib_srp.c | 104 ++++++++++++++++++++++++++++++----- drivers/infiniband/ulp/srp/ib_srp.h | 6 ++ 2 files changed, 95 insertions(+), 15 deletions(-)drivers/infiniband/ulp/srp/ib_srp.c +46
-5@@ -40,7 +40,7 @@#include <linux/parser.h> #include <linux/random.h> #include <linux/jiffies.h> - +#include <linux/delay.h> #include <linux/atomic.h> #include <scsi/scsi.h> @@ -229,14 +229,16 @@ static int srp_create_target_ib(struct srp_target_port *target)return -ENOMEM ; target->recv_cq =ib_create_cq (target->srp_host->srp_dev->dev, - srp_recv_completion,NULL , target,SRP_RQ_SIZE ,0 ); + srp_recv_completion,NULL , target, +SRP_RQ_SIZE +1 ,0 );if (IS_ERR (target->recv_cq)) { ret =PTR_ERR (target->recv_cq); goto err; } target->send_cq =ib_create_cq (target->srp_host->srp_dev->dev, - srp_send_completion,NULL , target,SRP_SQ_SIZE ,0 ); + srp_send_completion,NULL , target, +SRP_SQ_SIZE +1 ,0 );if (IS_ERR (target->send_cq)) { ret =PTR_ERR (target->send_cq); goto err_recv_cq;@@ -245,8 +247,8 @@ static int srp_create_target_ib(struct srp_target_port *target)ib_req_notify_cq (target->recv_cq,IB_CQ_NEXT_COMP ); init_attr->event_handler = srp_qp_event; - init_attr->cap.max_send_wr =SRP_SQ_SIZE ; - init_attr->cap.max_recv_wr =SRP_RQ_SIZE ; + init_attr->cap.max_send_wr =SRP_SQ_SIZE +1 ; + init_attr->cap.max_recv_wr =SRP_RQ_SIZE +1 ; init_attr->cap.max_recv_sge =1 ; init_attr->cap.max_send_sge =1 ; init_attr->sq_sig_type =IB_SIGNAL_ALL_WR ;@@ -460,11 +462,69 @@ static bool srp_change_conn_state(struct srp_target_port *target,return changed; } +static void srp_wait_last_recv_wqe (struct srp_target_port *target) +{ +static struct ib_recv_wr wr = { + .wr_id =SRP_LAST_RECV , + }; +struct ib_recv_wr *bad_wr; +int ret; + +if (target->last_recv_wqe) +return ; + + ret =ib_post_recv (target->qp, &wr, &bad_wr); +if (ret <0 ) { +shost_printk (KERN_ERR , target->scsi_host, +"ib_post_recv() failed (%d)\n" , ret); +return ; + } + + ret =wait_event_timeout (target->qp_wq, target->last_recv_wqe, + target->rq_tmo_jiffies); +WARN (ret <=0 ,"Timeout while waiting for last recv WQE (ret = %d)\n" , + ret); +} + +static void srp_wait_last_send_wqe (struct srp_target_port *target) +{ +static struct ib_send_wr wr = { + .wr_id =SRP_LAST_SEND , + }; +struct ib_send_wr *bad_wr; +unsigned long deadline = jiffies + target->rq_tmo_jiffies; +int ret; + +if (target->last_send_wqe) +return ; + + ret =ib_post_send (target->qp, &wr, &bad_wr); +if (ret <0 ) { + shost_printk(KERN_ERR, target->scsi_host,
drivers/infiniband/ulp/srp/ib_srp.c static bool srp_change_conn_state(struct srp_target_port *target, · line 513
Quoted 10 lines
drivers/infiniband/ulp/srp/ib_srp.c +10
-0+ "ib_post_send() failed (%d)\n", ret); +return ; + } + +while (!target->last_send_wqe &&time_before (jiffies, deadline)) { +srp_send_completion (target->send_cq, target); +msleep (20 ); + } + +WARN_ON (!target->last_send_wqe);
<-- here it is - remove it
Quoted 131 lines
drivers/infiniband/ulp/srp/ib_srp.c +33
-10+} +static void srp_disconnect_target (struct srp_target_port *target) { +static struct ib_qp_attr qp_attr = { + .qp_state =IB_QPS_ERR + }; +int ret; +if (srp_change_conn_state (target, false)) {/* XXX should send SRP_I_LOGOUT request */ +BUG_ON (!target->cm_id); +init_completion (&target->done);if (ib_send_cm_dreq (target->cm_id,NULL ,0 )) { shost_printk(KERN_DEBUG, target->scsi_host,@@ -473,6 +533,20 @@ static void srp_disconnect_target(struct srp_target_port *target)wait_for_completion (&target->done); } } + +if (target->cm_id) { +ib_destroy_cm_id (target->cm_id); + target->cm_id =NULL ; + } + +if (target->qp) { + ret =ib_modify_qp (target->qp, &qp_attr,IB_QP_STATE ); +WARN (ret !=0 ,"ib_modify_qp() failed: %d\n" , ret); + +srp_wait_last_recv_wqe (target); + +srp_wait_last_send_wqe (target); + } }static void srp_free_req_data (struct srp_target_port *target)@@ -516,7 +590,6 @@ static void srp_remove_target(struct srp_target_port *target)srp_remove_host (target->scsi_host);scsi_remove_host (target->scsi_host);srp_disconnect_target (target); -ib_destroy_cm_id (target->cm_id);srp_free_target_ib (target);srp_free_req_data (target);scsi_host_put (target->scsi_host);@@ -544,6 +617,8 @@ static int srp_connect_target(struct srp_target_port *target)WARN_ON (target->connected); target->qp_in_error = false; + target->last_recv_wqe = false; + target->last_send_wqe = false; ret =srp_lookup_path (target);if (ret)@@ -678,7 +753,6 @@ static int srp_reconnect_target(struct srp_target_port *target) {struct Scsi_Host *shost = target->scsi_host;struct ib_qp_attr qp_attr; -struct ib_wc wc;int i, ret;if (target->state !=SRP_TARGET_LIVE )@@ -704,11 +778,6 @@ static int srp_reconnect_target(struct srp_target_port *target)if (ret) goto err; -while (ib_poll_cq (target->recv_cq,1 , &wc) >0 ) - ;/* nothing */ -while (ib_poll_cq (target->send_cq,1 , &wc) >0 ) - ;/* nothing */ -for (i =0 ; i <SRP_CMD_SQ_SIZE ; ++i) {struct srp_request *req = &target->req_ring[i];if (req->scmnd)@@ -1285,7 +1354,7 @@ static void srp_handle_qp_err(enum ib_wc_status wc_status, enum ib_wc_opcode wc_opcode, struct srp_target_port *target) { -if (target->connected) +if (target->connected && !target->qp_in_error) shost_printk(KERN_ERR, target->scsi_host, PFX "failed %s status %d\n", wc_opcode & IB_WC_RECV ? "receive" : "send",@@ -1303,8 +1372,11 @@ static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)if (likely (wc.status ==IB_WC_SUCCESS )) {srp_handle_recv (target, &wc); }else { +if (wc.wr_id ==SRP_LAST_RECV ) { + target->last_recv_wqe = true; +wake_up (&target->qp_wq); + }srp_handle_qp_err (wc.status, wc.opcode, target); -break ; } } }@@ -1320,8 +1392,9 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr) iu = (struct srp_iu *) (uintptr_t ) wc.wr_id;list_add (&iu->list, &target->free_tx); }else { +if (wc.wr_id ==SRP_LAST_SEND ) + target->last_send_wqe = true;srp_handle_qp_err (wc.status, wc.opcode, target); -break ; } } }@@ -2255,6 +2328,7 @@ static ssize_t srp_create_target(struct device *dev,spin_lock_init (&target->lock);INIT_LIST_HEAD (&target->free_tx);INIT_LIST_HEAD (&target->free_reqs); +init_waitqueue_head (&target->qp_wq);for (i =0 ; i <SRP_CMD_SQ_SIZE ; ++i) {struct srp_request *req = &target->req_ring[i];drivers/infiniband/ulp/srp/ib_srp.h +3
-0@@ -76,6 +76,9 @@ enum {SRP_MAP_ALLOW_FMR =0 ,SRP_MAP_NO_FMR =1 , + +SRP_LAST_RECV =0 , + SRP_LAST_SEND = 0, }; enum srp_target_state {
drivers/infiniband/ulp/srp/ib_srp.h struct srp_target_port { · line 191
Quoted 9 lines
drivers/infiniband/ulp/srp/ib_srp.h +3
-0@@ -180,6 +183,9 @@ struct srp_target_port {struct completion done;int status;bool qp_in_error; +bool last_recv_wqe; +bool last_send_wqe; +wait_queue_head_t qp_wq;struct completion tsk_mgmt_done;u8 tsk_mgmt_status;
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 11/20] ib_srp: Make srp_disconnect_target() wait for IB completions
Quoted 7 lines · Sebastian Riemer
we've triggered the
WARN_ON()insrp_wait_last_send_wqe()by connecting to a disabled SCST SRP target.I would remove that one.
[ ... ]
Quoted 8 lines
drivers/infiniband/ulp/srp/ib_srp.c static bool srp_change_conn_state(struct srp_target_port *target, · line 513
Quoted 6 lines
drivers/infiniband/ulp/srp/ib_srp.c +6
-0+while (!target->last_send_wqe &&time_before (jiffies, deadline)) { +srp_send_completion (target->send_cq, target); +msleep (20 ); + } + +WARN_ON (!target->last_send_wqe);<-- here it is - remove it
But why was that WARN_ON() statement hit ? srp_wait_last_send_wqe() is invoked after the QP has been transitioned into the error state. It is the responsibility of the HCA to generate an error completion for any work queued on a QP that is in the error state. If that WARN_ON() statement has been hit that means that it took more than the RC timeout before the HCA finished processing earlier queued work and generated an error completion. That's not really something I had expected.
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 11/20] ib_srp: Make srp_disconnect_target() wait for IB completions
Hi Bart,
I'll try to explain, as Sebastian is on vacation.
Quoted 17 lines · Bart Van Assche
Quoted 7 lines · Sebastian Riemer
we've triggered the
WARN_ON()insrp_wait_last_send_wqe()by connecting to a disabled SCST SRP target.I would remove that one.
[ ... ]
Quoted 8 lines
drivers/infiniband/ulp/srp/ib_srp.c static bool srp_change_conn_state(struct srp_target_port *target, · line 513
Quoted 6 lines
drivers/infiniband/ulp/srp/ib_srp.c +6
-0+while (!target->last_send_wqe &&time_before (jiffies, deadline)) { +srp_send_completion (target->send_cq, target); +msleep (20 ); + } + +WARN_ON (!target->last_send_wqe);<-- here it is - remove it
Quoted 7 lines
But why was that
WARN_ON()statement hit ?srp_wait_last_send_wqe()is invoked after the QP has been transitioned into the error state. It is the responsibility of the HCA to generate an error completion for any work queued on a QP that is in the error state. If thatWARN_ON()statement has been hit that means that it took more than the RC timeout before the HCA finished processing earlier queued work and generated an error completion. That's not really something I had expected.
That occurs usually when releasing multiple targets at the same time. A typical situation is unloading kernel module ib_srp.ko immediately, which leads to tearing down every Infiniband connection. But it doesn't occur always, which makes it hard for us to test.
Example of kernel trace:
WARNING: at drivers/infiniband/ulp/srp/ib_srp.c:529 srp_disconnect_target+0x317/0x320 [ib_srp]() Hardware name: H8DGU Modules linked in:
rdma_ucm rdma_cm iw_cm ib_addr ib_ipoib ib_uverbs ib_umad ib_srp scsi_transport_srp scsi_tgt ib_cm ib_sa loop ib_mthca psmouse ib_mad amd64_edac_mod edac_core i2c_piix4 evdev serio_raw edac_mce_amd ib_core tpm_tis tpm tpm_bios processor button thermal_sys sg hid_cherry sd_mod crc_t10dif usb_storage ahci libahci libata scsi_mod [last unloaded: scsi_wait_scan]
Pid: 101, comm: kworker/1:1 Tainted: G W 3.2.8-pserver #1
Call Trace: [<ffffffff81048dbb>] ? warn_slowpath_common+0x7b/0xc0 [<ffffffffa00a9b07>] ? srp_disconnect_target+0x317/0x320 [ib_srp] [<ffffffff8106a640>] ? wake_up_bit+0x40/0x40 [<ffffffffa00ab0bf>] ? srp_remove_work+0x13f/0x1c0 [ib_srp] [<ffffffffa00aaf80>] ? srp_free_req_data+0xd0/0xd0 [ib_srp] [<ffffffff81063383>] ? process_one_work+0x113/0x470 [<ffffffff81065c73>] ? worker_thread+0x163/0x3e0 [<ffffffff81065b10>] ? manage_workers+0x200/0x200 [<ffffffff81065b10>] ? manage_workers+0x200/0x200 [<ffffffff8106a126>] ? kthread+0x96/0xa0 [<ffffffff8165f674>] ? kernel_thread_helper+0x4/0x10 [<ffffffff8106a090>] ? kthread_worker_fn+0x180/0x180 [<ffffffff8165f670>] ? gs_change+0x13/0x13
Regards,
Dongsu
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 12/20 ib_srp: Document sysfs attributes
In reply to a message not in the archiveDocument the sysfs attributes of the SRP initiator (ib_srp) according to the rules specified in Documentation/ABI/README.
--- Documentation/ABI/stable/sysfs-driver-ib_srp | 156 ++++++++++++++++++++++++++ 1 files changed, 156 insertions(+), 0 deletions(-) create mode 100644 Documentation/ABI/stable/sysfs-driver-ib_srp
Documentation/ABI/stable/sysfs-driver-ib_srp new file +156 -0
@@ -0,0 +1,156 @@ +What: /sys/class/infiniband_srp/srp-<hca>-<port_number>/add_target +Date: January 2, 2006 +KernelVersion: 2.6.15 +Contact: [email protected] +Description: Interface for making ib_srp connect to a new target. + One can request ib_srp to connect to a new target by writing + a comma-separated list of login parameters to this sysfs + attribute. The supported parameters are: + * id_ext, a 16-digit hexadecimal number specifying the eight + byte identifier extension in the 16-byte SRP target port + identifier. The target port identifier is sent by ib_srp + to the target in the SRP_LOGIN_REQ request. + * ioc_guid, a 16-digit hexadecimal number specifying the eight + byte I/O controller GUID portion of the 16-byte target port + identifier. + * dgid, a 32-digit hexadecimal number specifying the + destination GID. + * pkey, a four-digit hexadecimal number specifying the + InfiniBand partition key. + * service_id, a 16-digit hexadecimal number specifying the + InfiniBand service ID used to establish communication with + the SRP target. How to find out the value of the service ID + is specified in the documentation of the SRP target. + * max_sect, a decimal number specifying the maximum number of + 512-byte sectors to be transferred via a single SCSI command. + * max_cmd_per_lun, a decimal number specifying the maximum + number of outstanding commands for a single LUN. + * io_class, a hexadecimal number specifying the SRP I/O class. + Must be either 0xff00 (rev 10) or 0x0100 (rev 16a). The I/O + class defines the format of the SRP initiator and target + port identifiers. + * initiator_ext, a 16-digit hexadecimal number specifying the + identifier extension portion of the SRP initiator port + identifier. This data is sent by the initiator to the target + in the SRP_LOGIN_REQ request. + * cmd_sg_entries, a number in the range 1..255 that specifies + the maximum number of data buffer descriptors stored in the + SRP_CMD information unit itself. With allow_ext_sg=0 the + parameter cmd_sg_entries defines the maximum S/G list length + for a single SRP_CMD, and commands whose S/G list length + exceeds this limit after S/G list collapsing will fail. + * allow_ext_sg, whether ib_srp is allowed to include a partial + memory descriptor list in an SRP_CMD instead of the entire + list. If a partial memory descriptor list has been included + in an SRP_CMD the remaining memory descriptors are + communicated from initiator to target via an additional RDMA + transfer. Setting allow_ext_sg to 1 increases the maximum + amount of data that can be transferred between initiator and + target via a single SCSI command. Since not all SRP target + implementations support partial memory descriptor lists the + default value for this option is 0. + * sg_tablesize, a number in the range 1..2048 specifying the + maximum S/G list length the SCSI layer is allowed to pass to + ib_srp. Specifying a value that exceeds cmd_sg_entries is + only safe with partial memory descriptor list support enabled + (allow_ext_sg=1). + +What: /sys/class/infiniband_srp/srp-<hca>-<port_number>/ibdev +Date: January 2, 2006 +KernelVersion: 2.6.15 +Contact: [email protected] +Description: HCA name (<hca>). + +What: /sys/class/infiniband_srp/srp-<hca>-<port_number>/port +Date: January 2, 2006 +KernelVersion: 2.6.15 +Contact: [email protected] +Description: HCA port number (<port_number>). + +What: /sys/class/scsi_host/host<n>/allow_ext_sg +Date: May 19, 2011 +KernelVersion: 2.6.39 +Contact: [email protected] +Description: Whether ib_srp is allowed to include a partial memory + descriptor list in an SRP_CMD when communicating with an SRP + target. + +What: /sys/class/scsi_host/host<n>/cmd_sg_entries +Date: May 19, 2011 +KernelVersion: 2.6.39 +Contact: [email protected] +Description: Maximum number of data buffer descriptors that may be sent to + the target in a single SRP_CMD request. + +What: /sys/class/scsi_host/host<n>/dgid +Date: June 17, 2006 +KernelVersion: 2.6.17 +Contact: [email protected] +Description: InfiniBand destination GID used for communication with the SRP + target. Differs from orig_dgid if port redirection has happened. + +What: /sys/class/scsi_host/host<n>/id_ext +Date: June 17, 2006 +KernelVersion: 2.6.17 +Contact: [email protected] +Description: Eight-byte identifier extension portion of the 16-byte target + port identifier. + +What: /sys/class/scsi_host/host<n>/ioc_guid +Date: June 17, 2006 +KernelVersion: 2.6.17 +Contact: [email protected] +Description: Eight-byte I/O controller GUID portion of the 16-byte target + port identifier. + +What: /sys/class/scsi_host/host<n>/local_ib_device +Date: November 29, 2006 +KernelVersion: 2.6.19 +Contact: [email protected] +Description: Name of the InfiniBand HCA used for communicating with the + SRP target. + +What: /sys/class/scsi_host/host<n>/local_ib_port +Date: November 29, 2006 +KernelVersion: 2.6.19 +Contact: [email protected] +Description: Number of the HCA port used for communicating with the + SRP target. + +What: /sys/class/scsi_host/host<n>/orig_dgid +Date: June 17, 2006 +KernelVersion: 2.6.17 +Contact: [email protected] +Description: InfiniBand destination GID specified in the parameters + written to the add_target sysfs attribute. + +What: /sys/class/scsi_host/host<n>/pkey +Date: June 17, 2006 +KernelVersion: 2.6.17 +Contact: [email protected] +Description: A 16-bit number representing the InfiniBand partition key used + for communication with the SRP target. + +What: /sys/class/scsi_host/host<n>/req_lim +Date: October 20, 2010 +KernelVersion: 2.6.36 +Contact: [email protected] +Description: Number of requests ib_srp can send to the target before it has + to wait for more credits. For more information see also the + SRP credit algorithm in the SRP specification. + +What: /sys/class/scsi_host/host<n>/service_id +Date: June 17, 2006 +KernelVersion: 2.6.17 +Contact: [email protected] +Description: InfiniBand service ID used for establishing communication with + the SRP target. + +What: /sys/class/scsi_host/host<n>/zero_req_lim +Date: September 20, 2006 +KernelVersion: 2.6.18 +Contact: [email protected] +Description: Number of times the initiator had to wait before sending a + request to the target because it ran out of credits. For more + information see also the SRP credit algorithm in the SRP + specification.
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 13/20 srp_transport: Fix atttribute registration
In reply to a message not in the archiveRegister transport attributes after the attribute array has been set up instead of before. The current code can trigger a race condition because the code reading the attribute array can run on another thread than the code that initialized that array. Make sure that any code reading the attribute array will see all values written into that array.
--- drivers/scsi/scsi_transport_srp.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/scsi/scsi_transport_srp.c +2 -1
@@ -324,13 +324,14 @@ srp_attach_transport(struct srp_function_template *ft) i->rport_attr_cont.ac.attrs = &i->rport_attrs[0 ]; i->rport_attr_cont.ac.class = &srp_rport_class.class; i->rport_attr_cont.ac.match = srp_rport_match; -transport_container_register (&i->rport_attr_cont); count =0 ;SETUP_RPORT_ATTRIBUTE_RD (port_id);SETUP_RPORT_ATTRIBUTE_RD (roles); i->rport_attrs[count] =NULL ; +transport_container_register (&i->rport_attr_cont); + i->f = ft;return &i->t;
Signature · 7 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 15/20 srp_transport: Document sysfs attributes
In reply to a message not in the archive--- Documentation/ABI/stable/sysfs-transport-srp | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) create mode 100644 Documentation/ABI/stable/sysfs-transport-srp
Documentation/ABI/stable/sysfs-transport-srp new file +12 -0
@@ -0,0 +1,12 @@ +What: /sys/class/srp_remote_ports/port-<h>:<n>/port_id +Date: June 27, 2007 +KernelVersion: 2.6.24 +Contact: [email protected] +Description: 16-byte local SRP port identifier in hexadecimal format. An + example: 4c:49:4e:55:58:20:56:49:4f:00:00:00:00:00:00:00. + +What: /sys/class/srp_remote_ports/port-<h>:<n>/roles +Date: June 27, 2007 +KernelVersion: 2.6.24 +Contact: [email protected] +Description: Role of the remote port. Either "SRP Initiator" or "SRP Target".
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 16/20 ib_srp: Allow SRP disconnect through sysfs
In reply to a message not in the archiveMake it possible to disconnect the IB RC connection used by the SRP protocol to communicate with a target.
Let the SRP transport layer create a sysfs "delete" attribute for initiator drivers that support this functionality.
--- Documentation/ABI/stable/sysfs-transport-srp | 7 +++++++ drivers/infiniband/ulp/srp/ib_srp.c | 10 ++++++++++ drivers/scsi/scsi_transport_srp.c | 22 +++++++++++++++++++++- include/scsi/scsi_transport_srp.h | 8 ++++++++ 4 files changed, 46 insertions(+), 1 deletions(-)
Documentation/ABI/stable/sysfs-transport-srp +7 -0
@@ -1,3 +1,10 @@ +What: /sys/class/srp_remote_ports/port-<h>:<n>/delete +Date: June 1, 2012 +KernelVersion: 3.7 +Contact: [email protected], [email protected] +Description: Instructs an SRP initiator to disconnect from a target and to + remove all LUNs imported from that target. + What: /sys/class/srp_remote_ports/port-<h>:<n>/port_id Date: June 27, 2007 KernelVersion: 2.6.24
drivers/infiniband/ulp/srp/ib_srp.c +10 -0
@@ -609,6 +609,13 @@ static void srp_remove_work(struct work_struct *work)srp_remove_target (target); } +static void srp_rport_delete (struct srp_rport *rport) +{ +struct srp_target_port *target = rport->lld_data; + +srp_queue_remove_work (target); +} +static int srp_connect_target (struct srp_target_port *target) {int retries =3 ;
@@ -2029,6 +2036,8 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)return PTR_ERR (rport); } + rport->lld_data = target; +spin_lock (&host->target_lock);list_add_tail (&target->list, &host->target_list);spin_unlock (&host->target_lock);
@@ -2596,6 +2605,7 @@ static void srp_remove_one(struct ib_device *device) }static struct srp_function_template ib_srp_transport_functions = { + .rport_delete = srp_rport_delete, };static int __init srp_init_module (void )
drivers/scsi/scsi_transport_srp.c +21 -1
@@ -38,7 +38,7 @@ struct srp_host_attrs {#define to_srp_host_attrs (host) ((struct srp_host_attrs *)(host)->shost_data)#define SRP_HOST_ATTRS 0 -#define SRP_RPORT_ATTRS 2 +#define SRP_RPORT_ATTRS 3struct srp_internal {struct scsi_transport_template t;
@@ -116,6 +116,24 @@ show_srp_rport_roles(struct device *dev, struct device_attribute *attr, static DEVICE_ATTR(roles, S_IRUGO, show_srp_rport_roles, NULL); +static ssize_t store_srp_rport_delete (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t count) +{ +struct srp_rport *rport =transport_class_to_srp_rport (dev); +struct Scsi_Host *shost =dev_to_shost (dev); +struct srp_internal *i =to_srp_internal (shost->transportt); + +if (i->f->rport_delete) { + i->f->rport_delete (rport); +return count; + }else { +return -ENOSYS ; + } +} + +static DEVICE_ATTR(delete, S_IWUSR, NULL, store_srp_rport_delete); +static void srp_rport_release (struct device *dev) {struct srp_rport *rport =dev_to_rport (dev);
@@ -309,6 +327,8 @@ srp_attach_transport(struct srp_function_template *ft) count =0 ; i->rport_attrs[count++] = &dev_attr_port_id; i->rport_attrs[count++] = &dev_attr_roles; +if (ft->rport_delete) + i->rport_attrs[count++] = &dev_attr_delete; i->rport_attrs[count++] =NULL ;BUG_ON (count >ARRAY_SIZE (i->rport_attrs));
include/scsi/scsi_transport_srp.h +8 -0
@@ -14,13 +14,21 @@ struct srp_rport_identifiers { };struct srp_rport { +/* for initiator and target drivers */ +struct device dev;u8 port_id[16 ];u8 roles; + +/* for initiator drivers */ + +void *lld_data;/* LLD private data */ };struct srp_function_template { +/* for initiator drivers */ +void (*rport_delete)(struct srp_rport *rport);/* for target drivers */ int (* tsk_mgmt_response)(struct Scsi_Host *,u64 ,u64 ,int );int (* it_nexus_response)(struct Scsi_Host *,u64 ,int );
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 17/20 ib_srp: Introduce a temporary variable in srp_remove_target()
In reply to a message not in the archive--- drivers/infiniband/ulp/srp/ib_srp.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +6 -4
@@ -584,15 +584,17 @@ static void srp_del_scsi_host_attr(struct Scsi_Host *shost)static void srp_remove_target (struct srp_target_port *target) { +struct Scsi_Host *shost = target->scsi_host; +WARN_ON (target->state !=SRP_TARGET_REMOVED ); -srp_del_scsi_host_attr (target->scsi_host); -srp_remove_host (target->scsi_host); -scsi_remove_host (target->scsi_host); +srp_del_scsi_host_attr (shost); +srp_remove_host (shost); +scsi_remove_host (shost);srp_disconnect_target (target);srp_free_target_ib (target);srp_free_req_data (target); -scsi_host_put (target->scsi_host); +scsi_host_put (shost); }static void srp_remove_work (struct work_struct *work)
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 18/20 ib_srp: Maintain a single connection per I_T nexus
In reply to a message not in the archiveThe sysfs attribute 'add_target' may be used to relogin to a target. An SRP target that receives a second login request from an initiator will disconnect the previous connection. So before trying to reconnect, check whether another connection to the same SRP target identifier already exists. If so, remove the target port. Add a target to the target list before connecting instead of after such that this algorithm has a chance to work.
--- drivers/infiniband/ulp/srp/ib_srp.c | 120 +++++++++++++++++++++++++++++------ drivers/infiniband/ulp/srp/ib_srp.h | 11 +++ 2 files changed, 110 insertions(+), 21 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +99 -21
@@ -430,6 +430,24 @@ static int srp_send_req(struct srp_target_port *target)return status; } +static bool srp_change_state (struct srp_target_port *target, +enum srp_target_state old, +enum srp_target_state new) +{ +bool changed = false; + +BUG_ON (old ==SRP_TARGET_REMOVED ); + +spin_lock_irq (&target->lock); +if (target->state == old) { + target->state = new; + changed = true; + } +spin_unlock_irq (&target->lock); + +return changed; +} +static bool srp_queue_remove_work (struct srp_target_port *target) {bool changed = false;
@@ -588,10 +606,15 @@ static void srp_remove_target(struct srp_target_port *target)WARN_ON (target->state !=SRP_TARGET_REMOVED ); -srp_del_scsi_host_attr (shost); -srp_remove_host (shost); -scsi_remove_host (shost); +mutex_lock (&target->mutex); +if (target->scsi_host_added) { +srp_del_scsi_host_attr (shost); +srp_remove_host (shost); +scsi_remove_host (shost); + } +mutex_unlock (&target->mutex);srp_disconnect_target (target); +srp_free_target_ib (target);srp_free_req_data (target);scsi_host_put (shost);
@@ -618,6 +641,30 @@ static void srp_rport_delete(struct srp_rport *rport)srp_queue_remove_work (target); } +/** +* srp_conn_unique() - Check whether the connection to a target is unique. +*/ +static bool srp_conn_unique (struct srp_host *host, +struct srp_target_port *target) +{ +struct srp_target_port *t; +bool ret = true; + +spin_lock (&host->target_lock); +list_for_each_entry (t, &host->target_list, list) { +if (t != target && + target->id_ext == t->id_ext && + target->ioc_guid == t->ioc_guid && + target->initiator_ext == t->initiator_ext) { + ret = false; +break ; + } + } +spin_unlock (&host->target_lock); + +return ret; +} +static int srp_connect_target (struct srp_target_port *target) {int retries =3 ;
@@ -764,8 +811,18 @@ static int srp_reconnect_target(struct srp_target_port *target)struct ib_qp_attr qp_attr;int i, ret; -if (target->state !=SRP_TARGET_LIVE ) +if (!srp_conn_unique (target->srp_host, target) && +srp_queue_remove_work (target)) { +shost_printk (KERN_INFO , target->scsi_host, +PFX "Deleting SCSI host because obsolete\n" ); +return -ENXIO ; + } + +if (target->state ==SRP_TARGET_REMOVED ) { +shost_printk (KERN_DEBUG , target->scsi_host, +PFX "Removal already scheduled\n" );return -EAGAIN ; + }scsi_target_block (&shost->shost_gendev);
@@ -1740,6 +1797,9 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)shost_printk (KERN_ERR , target->scsi_host,PFX "connection closed\n" ); +if (!srp_conn_unique (target->srp_host, target)) +srp_queue_remove_work (target); + comp =1 ; target->status =0 ;break ;
@@ -2040,16 +2100,6 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target) rport->lld_data = target; -spin_lock (&host->target_lock); -list_add_tail (&target->list, &host->target_list); -spin_unlock (&host->target_lock); - - target->state =SRP_TARGET_LIVE ; - target->connected = false; - -scsi_scan_target (&target->scsi_host->shost_gendev, -0 , target->scsi_id,SCAN_WILD_CARD ,0 ); -return 0 ; }
@@ -2335,6 +2385,7 @@ static ssize_t srp_create_target(struct device *dev,sizeof (struct srp_indirect_buf ) + target->cmd_sg_cnt *sizeof (struct srp_direct_buf ); +mutex_init (&target->mutex);INIT_WORK (&target->remove_work, srp_remove_work);spin_lock_init (&target->lock);INIT_LIST_HEAD (&target->free_tx);
@@ -2381,24 +2432,51 @@ static ssize_t srp_create_target(struct device *dev,if (ret) goto err_free_ib; + target->connected = false; + target->rq_tmo_jiffies =1 *HZ ; + target->state =SRP_TARGET_CONNECTING ; + target->scsi_host_added = false; + +spin_lock (&host->target_lock); +list_add_tail (&target->list, &host->target_list); +spin_unlock (&host->target_lock); + +mutex_lock (&target->mutex); +if (!srp_change_state (target,SRP_TARGET_CONNECTING ,SRP_TARGET_LIVE )) { + ret = -ENOENT ; + goto err_unlock_remove; + } + ret =srp_connect_target (target);if (ret) {shost_printk (KERN_ERR , target->scsi_host,PFX "Connection failed\n" ); - goto err_cm_id; + goto err_unlock_remove; } ret =srp_add_target (host, target); + target->scsi_host_added = ret ==0 ; + +WARN_ON (!scsi_host_get (target_host)); +mutex_unlock (&target->mutex); + +scsi_scan_target (&target->scsi_host->shost_gendev,0 , target->scsi_id, +SCAN_WILD_CARD ,0 ); + +scsi_host_put (target_host); +if (ret) - goto err_disconnect; + goto err_remove;return count; -err_disconnect: -srp_disconnect_target (target); - -err_cm_id: -ib_destroy_cm_id (target->cm_id); +err_unlock_remove: +mutex_unlock (&target->mutex); +err_remove: +if (srp_queue_remove_work (target)) +shost_printk (KERN_INFO , target->scsi_host, +PFX "deleting SCSI host (ret = %d).\n" , ret); +return ret; err_free_ib:srp_free_target_ib (target);
drivers/infiniband/ulp/srp/ib_srp.h +11 -0
@@ -81,7 +81,16 @@ enum { SRP_LAST_SEND = 0, }; +/** +* enum srp_target_state - State of an SRP target. +* @SRP_TARGET_CONNECTING: IB connection being established and SCSI host being +* added. +* @SRP_TARGET_LIVE: IB RC connection has been established. +* @SRP_TARGET_REMOVED: IB RC connection is about to be closed and SCSI host +* removal is pending. +*/ enum srp_target_state { +SRP_TARGET_CONNECTING ,SRP_TARGET_LIVE ,SRP_TARGET_REMOVED , };
@@ -166,6 +175,7 @@ struct srp_target_port {u32 rq_tmo_jiffies;bool connected; +bool scsi_host_added;struct ib_cm_id *cm_id;
@@ -186,6 +196,7 @@ struct srp_target_port {bool last_recv_wqe;bool last_send_wqe;wait_queue_head_t qp_wq; +struct mutex mutex;struct completion tsk_mgmt_done;u8 tsk_mgmt_status;
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 19/20 srp_transport: Add transport layer error handling
In reply to a message not in the archiveAdd the necessary functions in the SRP transport module to allow an SRP initiator driver to implement transport error handling. This includes:
- Support for implementing fast_io_fail_tmo, the time that should elapse after having detected a transport layer problem and before failing I/O.
- Support for implementing dev_loss_tmo, the time that should elapse after having detected a transport layer problem and before removing a remote port.
--- Documentation/ABI/stable/sysfs-transport-srp | 17 +++ drivers/scsi/scsi_transport_srp.c | 199 +++++++++++++++++++++++++- include/scsi/scsi_transport_srp.h | 11 ++- 3 files changed, 224 insertions(+), 3 deletions(-)
Documentation/ABI/stable/sysfs-transport-srp +17 -0
@@ -5,6 +5,23 @@ Contact: [email protected], [email protected] Description: Instructs an SRP initiator to disconnect from a target and to remove all LUNs imported from that target. +What: /sys/class/srp_remote_ports/port-<h>:<n>/dev_loss_tmo +Date: January 1, 2012 +KernelVersion: 3.7 +Contact: [email protected], [email protected] +Description: Number of seconds the SCSI layer will wait after a transport + layer error has been observed before removing a target port. + Zero means immediate removal. + +What: /sys/class/srp_remote_ports/port-<h>:<n>/fast_io_fail_tmo +Date: January 1, 2012 +KernelVersion: 3.7 +Contact: [email protected], [email protected] +Description: Number of seconds the SCSI layer will wait after a transport + layer error has been observed before failing I/O. Zero means + immediate removal. A negative value will disable this + behavior. + What: /sys/class/srp_remote_ports/port-<h>:<n>/port_id Date: June 27, 2007 KernelVersion: 2.6.24
drivers/scsi/scsi_transport_srp.c +197 -2
242 lines not on this page · raw message
include/scsi/scsi_transport_srp.h +10 -1
@@ -23,11 +23,17 @@ struct srp_rport {/* for initiator drivers */ -void *lld_data;/* LLD private data */ +void *lld_data;/* LLD private data */ + +int fast_io_fail_tmo; +unsigned dev_loss_tmo; +struct delayed_work fast_io_fail_work; +struct delayed_work dev_loss_work; };struct srp_function_template {/* for initiator drivers */ +void (*terminate_rport_io)(struct srp_rport *rport);void (*rport_delete)(struct srp_rport *rport);/* for target drivers */ int (* tsk_mgmt_response)(struct Scsi_Host *,u64 ,u64 ,int );
@@ -41,6 +47,9 @@ extern void srp_release_transport(struct scsi_transport_template *);extern struct srp_rport *srp_rport_add (struct Scsi_Host *,struct srp_rport_identifiers *);extern void srp_rport_del (struct srp_rport *); +extern void srp_start_tl_fail_timers (struct srp_rport *rport); +extern void srp_stop_tl_fail_timers (struct srp_rport *rport); +extern void srp_stop_rport (struct srp_rport *rport);extern void srp_remove_host (struct Scsi_Host *);
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v4 20/20 ib_srp: Add dev_loss_tmo support
In reply to a message not in the archiveRemove an SRP host if either dev_loss_tmo expired or the target closed the IB connection.
--- drivers/infiniband/ulp/srp/ib_srp.c | 53 +++++++++++++++++++++++++++++----- drivers/infiniband/ulp/srp/ib_srp.h | 2 + 2 files changed, 47 insertions(+), 8 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +45 -8
@@ -603,17 +603,23 @@ static void srp_del_scsi_host_attr(struct Scsi_Host *shost)static void srp_remove_target (struct srp_target_port *target) {struct Scsi_Host *shost = target->scsi_host; +struct srp_rport *rport = target->rport; +bool scsi_host_added = false;WARN_ON (target->state !=SRP_TARGET_REMOVED );mutex_lock (&target->mutex); -if (target->scsi_host_added) { +swap (target->scsi_host_added, scsi_host_added); +mutex_unlock (&target->mutex); + +if (scsi_host_added) {srp_del_scsi_host_attr (shost); +srp_stop_rport (rport);srp_remove_host (shost);scsi_remove_host (shost); } -mutex_unlock (&target->mutex);srp_disconnect_target (target); +cancel_work_sync (&target->tl_err_work);srp_free_target_ib (target);srp_free_req_data (target);
@@ -824,9 +830,19 @@ static int srp_reconnect_target(struct srp_target_port *target)return -EAGAIN ; } -scsi_target_block (&shost->shost_gendev); +mutex_lock (&target->mutex); +if (target->scsi_host_added) +scsi_target_block (&shost->shost_gendev); +mutex_unlock (&target->mutex);srp_disconnect_target (target); +cancel_work_sync (&target->tl_err_work); + +mutex_lock (&target->mutex); +if (target->scsi_host_added) +srp_stop_tl_fail_timers (target->rport); +mutex_unlock (&target->mutex); +/* * Now get a new local CM ID so that we avoid confusing the * target in case things are really fouled up.
@@ -858,12 +874,16 @@ static int srp_reconnect_target(struct srp_target_port *target)if (ret) goto err; +mutex_lock (&target->mutex);scsi_target_unblock (&shost->shost_gendev,SDEV_RUNNING ); +mutex_unlock (&target->mutex);return ret; err: +mutex_lock (&target->mutex);scsi_target_unblock (&shost->shost_gendev,SDEV_TRANSPORT_OFFLINE ); +mutex_unlock (&target->mutex);shost_printk (KERN_ERR , target->scsi_host,PFX "reconnect failed (%d), removing target port.\n" , ret);
@@ -1416,15 +1436,32 @@ static void srp_handle_recv(struct srp_target_port *target, struct ib_wc *wc) PFX "Recv failed with error code %d\n", res); } +/** +* srp_tl_err_work - Start the transport layer failure timers. +*/ +static void srp_tl_err_work (struct work_struct *work) +{ +struct srp_target_port *target; + + target = container_of(work, struct srp_target_port, tl_err_work); + +mutex_lock (&target->mutex); +if (target->scsi_host_added) +srp_start_tl_fail_timers (target->rport); +mutex_unlock (&target->mutex); +} +static void srp_handle_qp_err (enum ib_wc_status wc_status,enum ib_wc_opcode wc_opcode,struct srp_target_port *target) { -if (target->connected && !target->qp_in_error) +if (target->connected && !target->qp_in_error) {shost_printk (KERN_ERR , target->scsi_host,PFX "failed %s status %d\n" , wc_opcode &IB_WC_RECV ?"receive" :"send" , wc_status); +queue_work (system_long_wq, &target->tl_err_work); + } target->qp_in_error = true; }
@@ -1791,15 +1828,13 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)if (ib_send_cm_drep (cm_id,NULL ,0 ))shost_printk (KERN_ERR , target->scsi_host,PFX "Sending CM DREP failed\n" ); +queue_work (system_long_wq, &target->tl_err_work);break ;case IB_CM_TIMEWAIT_EXIT :shost_printk (KERN_ERR , target->scsi_host,PFX "connection closed\n" ); - -if (!srp_conn_unique (target->srp_host, target)) -srp_queue_remove_work (target); - +srp_queue_remove_work (target); comp =1 ; target->status =0 ;break ;
@@ -2099,6 +2134,7 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target) } rport->lld_data = target; + target->rport = rport;return 0 ; }
@@ -2386,6 +2422,7 @@ static ssize_t srp_create_target(struct device *dev, target->cmd_sg_cnt *sizeof (struct srp_direct_buf );mutex_init (&target->mutex); +INIT_WORK (&target->tl_err_work, srp_tl_err_work);INIT_WORK (&target->remove_work, srp_remove_work);spin_lock_init (&target->lock);INIT_LIST_HEAD (&target->free_tx);
drivers/infiniband/ulp/srp/ib_srp.h +2 -0
@@ -164,6 +164,7 @@ struct srp_target_port {u16 io_class;struct srp_host *srp_host;struct Scsi_Host *scsi_host; +struct srp_rport *rport;char target_name[32 ];unsigned int scsi_id;unsigned int sg_tablesize;
@@ -187,6 +188,7 @@ struct srp_target_port {struct srp_iu *rx_ring[SRP_RQ_SIZE ];struct srp_request req_ring[SRP_CMD_SQ_SIZE ]; +struct work_struct tl_err_work;struct work_struct remove_work;struct list_head list;
Signature · 6 lines
1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bart,
while testing ib_srp based on your srp-ha,
we sometimes hit kernel crashes with the call trace below.
How to reproduce:
0. Kernel 3.2.15 with SCST v4193 on the target,
Kernel 3.2.8 with ib_srp-ha on the initiator.
1. Configure 500+ vdisks on target, and get initiator connected.
2. Exchange data intensively, which works well.
3. (On initiator) delete SRP remote port occasionally, e.g.
# echo "1" > /sys/class/srp_remote_ports/port-6\:1/delete And configure again the SRP target. 4. (On target) disable Infiniband interface, and enable it again. 5. Repeat 3 and 4.
Then the initiator's kernel suddenly crashes. (but not always)
Do you have any idea why?
Thanks in advance,
Dongsu
---------------------------------------------------------------
BUG: unable to handle kernel paging request at 0000000000010001 IP: [<ffffffff8139ec55>] strnlen+0x5/0x40 PGD 212fea067 PUD 2162f8067 PMD 0 Oops: 0000 [#1] SMP CPU 0 Modules linked in: ib_srp scsi_transport_srp scsi_tgt rdma_ucm rdma_cm
iw_cm ib_addr ib_ipoib ib_cm ib_sa ib_uverbs ib_umad loop psmouse serio_raw evdev i2c_piix4 tpm_tis tpm tpm_bios ib_mthca sg ib_mad processor amd64_edac_mod edac_core thermal_sys edac_mce_amd ib_core button sd_mod crc_t10dif hid_cherry usb_storage ahci libahci libata scsi_mod [last unloaded: scsi_wait_scan]
Pid: 2311, comm: kworker/0:2 Not tainted 3.2.8 #1 Supermicro H8DGU/H8DGU
RIP: 0010:[<ffffffff8139ec55>] [<ffffffff8139ec55>] strnlen+0x5/0x40 RSP: 0018:ffff880215fe3c28 EFLAGS: 00010086 RAX: ffffffff81915991 RBX: ffffffff81ba5497 RCX: ffffffffff0a0004 RDX: 0000000000010001 RSI: ffffffffffffffff RDI: 0000000000010001 RBP: ffffffff81ba5860 R08: 000000000000fffb R09: 0000000000000001 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000010001 R13: ffffffffffffffff R14: 00000000ffffffff R15: 0000000000000000 FS: 00007faafb63e700(0000) GS:ffff880217c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b CR2: 0000000000010001 CR3: 0000000212f87000 CR4: 00000000000006f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process kworker/0:2 (pid: 2311, threadinfo ffff880215fe2000, task ffff88020f2ce540) Stack: ffffffff813a023c 0000000000000000 ffffffff81ba5497 ffffffffa0131d82 ffffffffa0131d80 ffff880215fe3da0 ffffffff81ba5860 ffff880215fe3c90 ffffffff813a142d 0000000000000016 ffffffff81ba5460 0000000000000400 Call Trace: [<ffffffff813a023c>] ? string+0x4c/0xe0 [<ffffffff813a142d>] ? vsnprintf+0x1ed/0x5b0 [<ffffffffa0131900>] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [<ffffffff813a18a9>] ? vscnprintf+0x9/0x20 [<ffffffff81049b7f>] ? vprintk+0xaf/0x440 [<ffffffff810f3cc0>] ? next_online_pgdat+0x20/0x50 [<ffffffff810f3d20>] ? next_zone+0x30/0x40 [<ffffffff810f4c60>] ? refresh_cpu_vm_stats+0xf0/0x160 [<ffffffffa0131900>] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [<ffffffff816533b6>] ? printk+0x40/0x4a [<ffffffffa013192d>] ? rport_dev_loss_timedout+0x2d/0xa0
[scsi_transport_srp]
[<ffffffff81063383>] ? process_one_work+0x113/0x470 [<ffffffff81065c73>] ? worker_thread+0x163/0x3e0 [<ffffffff81065b10>] ? manage_workers+0x200/0x200 [<ffffffff81065b10>] ? manage_workers+0x200/0x200 [<ffffffff8106a126>] ? kthread+0x96/0xa0 [<ffffffff8165f674>] ? kernel_thread_helper+0x4/0x10 [<ffffffff8106a090>] ? kthread_worker_fn+0x180/0x180 [<ffffffff8165f670>] ? gs_change+0x13/0x13
Code: 1f 80 00 00 00 00 31 c0 80 3f 00 48 89 fa 74 14 66 0f 1f 44 00 00
48 ff c2 80 3a 00 75 f8 48 89 d0 48 29 f8 f3 c3 48 85 f6 74 27 <80> 3f 00 74 22 48 ff ce 48 89 f8 eb 0e 66 0f 1f 44 00 00 48 ff RIP [<ffffffff8139ec55>] strnlen+0x5/0x40
RSP <ffff880215fe3c28>
CR2: 0000000000010001
---[ end trace d55b61cd78c54a0a ]--- BUG: unable to handle kernel paging request at fffffffffffffff8
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
maintainerQuoted 10 lines · Dongsu Park
while testing
ib_srpbased on your srp-ha,
we sometimes hit kernel crashes with the call trace below.How to reproduce:
0. Kernel 3.2.15 with SCST v4193 on the target,
Kernel 3.2.8 with ib_srp-ha on the initiator.
1. Configure 500+ vdisks on target, and get initiator connected.
2. Exchange data intensively, which works well.
3. (On initiator) delete SRP remote port occasionally, e.g.
Quoted 8 lines
# echo "1" > /sys/class/srp_remote_ports/port-6\:1/delete And configure again the SRP target. 4. (On target) disable Infiniband interface, and enable it again. 5. Repeat 3 and 4.Then the initiator's kernel suddenly crashes. (but not always)
Do you have any idea why?
Hello Dongsu,
That's unfortunate. I've just finished running the above test 1000 times on my test setup. The test ran perfectly - login succeeded every time, the test finished in the expected time, no kernel crash did occur and no memory was leaked. I've been running my test with kernel 3.6-rc3 instead of kernel 3.2.8 though. Can you repeat your test with kernel 3.6-rc3 on the initiator system instead of kernel 3.2.8 ? The 3.6-rc3 kernel contains multiple patches that improve robustness with regard to SCSI device removal.
Thanks,
Bart.
Hi Bart,
Quoted 22 lines · Bart Van Assche
Quoted 10 lines · Dongsu Park
while testing
ib_srpbased on your srp-ha,
we sometimes hit kernel crashes with the call trace below.How to reproduce:
0. Kernel 3.2.15 with SCST v4193 on the target,
Kernel 3.2.8 with ib_srp-ha on the initiator.
1. Configure 500+ vdisks on target, and get initiator connected.
2. Exchange data intensively, which works well.
3. (On initiator) delete SRP remote port occasionally, e.g.Quoted 8 lines
# echo "1" > /sys/class/srp_remote_ports/port-6\:1/delete And configure again the SRP target. 4. (On target) disable Infiniband interface, and enable it again. 5. Repeat 3 and 4.Then the initiator's kernel suddenly crashes. (but not always)
Do you have any idea why?
Hello Dongsu,
Quoted 8 lines
That's unfortunate. I've just finished running the above test 1000 times on my test setup. The test ran perfectly - login succeeded every time, the test finished in the expected time, no kernel crash did occur and no memory was leaked. I've been running my test with kernel 3.6-rc3 instead of kernel 3.2.8 though. Can you repeat your test with kernel 3.6-rc3 on the initiator system instead of kernel 3.2.8 ? The 3.6-rc3 kernel contains multiple patches that improve robustness with regard to SCSI device removal.
Ok, when I get a chance to set up a new test system with kernel 3.6-rc3, I'll do a new test and let you know.
By the way, as long as I've observed today, the crash occurs only if rport_dev_loss_timedout() is called. It means, without device loss, a simple rport_delete does not make any crash.
Is that probably because arguments to pr_err() are accessing to invalid addresses?
drivers/scsi/scsi_transport_srp.c:275
pr_err("SRP transport: dev_loss_tmo (%ds) expired - removing %s.\n",
rport->dev_loss_tmo, dev_name(&rport->dev));
Cheers,
Dongsu
Bart Van Assche
maintainerQuoted 11 lines · Dongsu Park
By the way, as long as I've observed today, the crash occurs only if
rport_dev_loss_timedout()is called. It means, without device loss, a simplerport_deletedoes not make any crash.Is that probably because arguments to
pr_err()are accessing to invalid addresses?
drivers/scsi/scsi_transport_srp.c:275pr_err("SRP transport:
dev_loss_tmo(%ds) expired - removing %s.\n",
rport->dev_loss_tmo,dev_name(&rport->dev));
It's not clear to me how that could happen. The dev_loss timer is stopped before the rport device is removed. See also the srp_stop_rport() call (which stops the dev_loss timer) and srp_remove_host() call (which removes the rport) in srp_remove_target().
Bart.
Quoted 1 line · Bart Van Assche
[ ... ]
Hello Dave,
More than six weeks have elapsed since I posted version four of this patch series. It would be appreciated if you could tell me when review comments for this patch series will be posted. I'd also like to remind you that some time ago you asked other people to wait with posting more ib_srp patches until this patch series is upstream [1, 2].
Thanks,
Bart.
[1] David Dillow, Re: [PATCH 1/1] ib_srp: Infiniband srp fast failover patch, May 29, 2012
(http://www.mail-archive.com/[email protected]/msg11801.html).
[2] David Dillow, Re: [PATCH] srp: convert SRP_RQ_SHIFT into a module parameter, May 29, 2012
(http://www.mail-archive.com/[email protected]/msg11802.html).
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 10 lines · Bart Van Assche
Quoted 1 line · Bart Van Assche
[ ... ]
Hello Dave,
More than six weeks have elapsed since I posted version four of this patch series. It would be appreciated if you could tell me when review comments for this patch series will be posted. I'd also like to remind you that some time ago you asked other people to wait with posting more
ib_srppatches until this patch series is upstream [1, 2].
Yes, it has taken me far more time than I expected to get to these. I am in the middle of fiscal-year-end thrash, and will attend to the SRP backlog next week.
Signature · 4 lines
Dave Dillow National Center for Computational Science Oak Ridge National Laboratory (865) 241-6602 office
A message that is not in the archive (sent off-list or not yet received): 1348705896.26028.3.camel-1q1vX8mYZiGLUyTwlgNVppKKF0rrzTr+@public.gmane.org
Quoted 12 lines · David Dillow
Quoted 10 lines · Bart Van Assche
Quoted 1 line · Bart Van Assche
[ ... ]
Hello Dave,
More than six weeks have elapsed since I posted version four of this patch series. It would be appreciated if you could tell me when review comments for this patch series will be posted. I'd also like to remind you that some time ago you asked other people to wait with posting more
ib_srppatches until this patch series is upstream [1, 2].
Quoted 3 lines
Yes, it has taken me far more time than I expected to get to these. I am in the middle of fiscal-year-end thrash, and will attend to the SRP backlog next week.
(replying to an e-mail of one month ago)
Hello Dave,
Will you have time next week to review the srp-ha patch series ?
Thanks,
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 17 lines · Bart Van Assche
Quoted 12 lines · David Dillow
Quoted 10 lines · Bart Van Assche
Quoted 1 line · Bart Van Assche
[ ... ]
Hello Dave,
More than six weeks have elapsed since I posted version four of this patch series. It would be appreciated if you could tell me when review comments for this patch series will be posted. I'd also like to remind you that some time ago you asked other people to wait with posting more
ib_srppatches until this patch series is upstream [1, 2].Quoted 3 lines
Yes, it has taken me far more time than I expected to get to these. I am in the middle of fiscal-year-end thrash, and will attend to the SRP backlog next week.
Quoted 5 lines
(replying to an e-mail of one month ago)
Hello Dave,
Will you have time next week to review the srp-ha patch series ?
I just pushed my first patch for your review; fingers crossed to build some momentum.
Signature · 9 lines
Dave Dillow National Center for Computational Science Oak Ridge National Laboratory (865) 241-6602 office -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
maintainerPATCH 14/20 srp_transport: Simplify attribute initialization code
In reply to Bart Van AsscheEliminate the private_rport_attrs[] array and the SETUP_*() macros used to set up that array since the information in that array duplicates the information in the static device attributes. Also, verify whether SRP_RPORT_ATTRS is large enough since it is easy to forget to update that macro when adding new attributes.
--- drivers/scsi/scsi_transport_srp.c | 26 ++++---------------------- 1 files changed, 4 insertions(+), 22 deletions(-)
drivers/scsi/scsi_transport_srp.c +4 -22
@@ -47,7 +47,6 @@ struct srp_internal {struct device_attribute *host_attrs[SRP_HOST_ATTRS +1 ];struct device_attribute *rport_attrs[SRP_RPORT_ATTRS +1 ]; -struct device_attribute private_rport_attrs[SRP_RPORT_ATTRS ];struct transport_container rport_attr_cont; };
@@ -72,24 +71,6 @@ static DECLARE_TRANSPORT_CLASS(srp_host_class, "srp_host", srp_host_setup, static DECLARE_TRANSPORT_CLASS(srp_rport_class, "srp_remote_ports", NULL, NULL, NULL); -#define SETUP_TEMPLATE (attrb, field, perm, test, ro_test, ro_perm) \ - i->private_##attrb[count] = dev_attr_##field; \ - i->private_##attrb[count].attr.mode = perm; \ - if (ro_test) { \ - i->private_##attrb[count].attr.mode = ro_perm; \ - i->private_##attrb[count].store = NULL; \ - } \ - i->attrb[count] = &i->private_##attrb[count]; \ - if (test) \ - count++ - -#define SETUP_RPORT_ATTRIBUTE_RD (field) \ - SETUP_TEMPLATE(rport_attrs, field, S_IRUGO, 1, 0, 0) - -#define SETUP_RPORT_ATTRIBUTE_RW (field) \ - SETUP_TEMPLATE(rport_attrs, field, S_IRUGO | S_IWUSR, \ - 1, 1, S_IRUGO) -#define SRP_PID (p) \ (p)->port_id[0], (p)->port_id[1], (p)->port_id[2], (p)->port_id[3], \ (p)->port_id[4], (p)->port_id[5], (p)->port_id[6], (p)->port_id[7], \
@@ -326,9 +307,10 @@ srp_attach_transport(struct srp_function_template *ft) i->rport_attr_cont.ac.match = srp_rport_match; count =0 ; -SETUP_RPORT_ATTRIBUTE_RD (port_id); -SETUP_RPORT_ATTRIBUTE_RD (roles); - i->rport_attrs[count] =NULL ; + i->rport_attrs[count++] = &dev_attr_port_id; + i->rport_attrs[count++] = &dev_attr_roles; + i->rport_attrs[count++] =NULL ; +BUG_ON (count >ARRAY_SIZE (i->rport_attrs));transport_container_register (&i->rport_attr_cont);
Signature · 1 line
1.7.7
Bart Van Assche
maintainerQuoted 1 line · Bart Van Assche
[ ... ]
The patch series is also available on top of 3.6-rc1 here:
http://github.com/bvanassche/linux/tree/srp-ha
Bart.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi Bart,
I have tested this series, all issues relating to hard removing targets are resolved.
Thanks!
Joseph.
Quoted 11 lines · Bart Van Assche
Quoted 1 line · Bart Van Assche
[ ... ]
The patch series is also available on top of 3.6-rc1 here:
http://github.com/bvanassche/linux/tree/srp-haBart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 6 lines
CTO | Orion Virtualisation Solutions | www.orionvm.com.au Phone: 1300 56 99 52 | Mobile: 0428 754 846 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html