Make ib_srp better suited for H.A. purposes
Subsystems (3)
v3 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:
- Switchover can be triggered explicitly by deleting an initiator device.
- Disconnecting from a target without unloading ib_srp becomes possible.
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-Enlarge-block-layer-timeout.patch 0002-ib_srp-Introduce-srp_handle_qp_err.patch 0003-ib_srp-Micro-optimize-srp_queuecommand.patch 0004-ib_srp-Suppress-superfluous-error-messages.patch 0005-ib_srp-Avoid-that-error-handling-triggers-a-crash.patch 0006-ib_srp-Introduce-the-helper-function-srp_remove_targ.patch 0007-ib_srp-Eliminate-state-SRP_TARGET_DEAD.patch 0008-ib_srp-Fix-race-triggered-by-module-removal.patch 0009-srp_transport-Fix-atttribute-registration.patch 0010-srp_transport-Simplify-attribute-initialization-code.patch 0011-srp_transport-Document-sysfs-attributes.patch 0012-ib_srp-Document-sysfs-attributes.patch 0013-ib_srp-Allow-SRP-disconnect-through-sysfs.patch 0014-ib_srp-Introduce-temporary-variable-in-srp_remove_ta.patch 0015-ib_srp-Maintain-a-single-connection-per-I_T-nexus.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
PATCH v3 1/15 ib_srp: Enlarge block layer timeout
Enlarge the block layer timeout for disks such that it is above the InfiniBand transport layer timeout. This is necessary to avoid that an SRP response is received after the SCSI layer has already killed the associated SCSI command.
--- 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
@@ -1372,6 +1372,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..C9-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)
@@ -1431,6 +1458,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;
@@ -1689,6 +1718,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) {
@@ -1821,6 +1865,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
Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
drivers/infiniband/ulp/srp/ib_srp.c err: · line 1386
Quoted 13 lines · Bart Van Assche
Enlarge the block layer timeout for disks such that it is above the InfiniBand transport layer timeout. This is necessary to avoid that an SRP response is received after the SCSI layer has already killed the associated SCSI command.
drivers/infiniband/ulp/srp/ib_srp.c +8
-0+ +/* +* 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 ));
I still don't like this useless splat when it could be written in a more safe manner, but not enough to kick it back again.
I haven't chewed on the rest yet, but would like to see this one at least in 3.4 if possible.
Signature · 8 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): [email protected]
Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
Quoted 4 lines · Dave Dillow
[ ... ]
I haven't chewed on the rest yet, but would like to see this one at least in 3.4 if possible.
Hi Dave,
If you have further comments about any of the patches in this series, these are welcome. The 3.5 merge window isn't that far away anymore.
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 01/15] ib_srp: Enlarge block layer timeout
Quoted 6 lines · Bart Van Assche
Quoted 2 lines · Dave Dillow
I haven't chewed on the rest yet, but would like to see this one at least in 3.4 if possible.
If you have further comments about any of the patches in this series, these are welcome. The 3.5 merge window isn't that far away anymore.
I will attend to this before the end of the week, sooner if possible.
Signature · 10 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): 1335798691.441.2.camel-zHLflQxYYDO4Hhoo1DtQwJ9G+ZOsUmrO@public.gmane.org
Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
Quoted 9 lines · David Dillow
Quoted 6 lines · Bart Van Assche
Quoted 2 lines · Dave Dillow
I haven't chewed on the rest yet, but would like to see this one at least in 3.4 if possible.
If you have further comments about any of the patches in this series, these are welcome. The 3.5 merge window isn't that far away anymore.
I will attend to this before the end of the week, sooner if possible.
Hello Dave,
I hope you're still doing fine ?
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]
Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
Quoted 10 lines · Bart Van Assche
Quoted 8 lines · David Dillow
Quoted 6 lines · Bart Van Assche
Quoted 2 lines · Dave Dillow
I haven't chewed on the rest yet, but would like to see this one at least in 3.4 if possible.
If you have further comments about any of the patches in this series, these are welcome. The 3.5 merge window isn't that far away anymore.
Quoted 6 lines
Quoted 1 line
I will attend to this before the end of the week, sooner if possible.
Hello Dave,
I hope you're still doing fine ?
Yes, but I've been (mostly) under a rock for the past few weeks on higher priority projects at work. Still, that's no excuse for keeping you waiting; part of the idea was that I should have faster turn-around than Roland...
This first patch was approved and should really go into the current merge window if possible. I remain convinced that we should be able to do the rest in a cleaner manner, without adding so many additional state variables. I may be wrong, though.
My family is leaving Saturday for vacation, so I'll take some evening hours this weekend/early next week and see if I can combine our ideas into something I'm comfortable with.
Thank you for your exceptional patience,
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 v3 2/15 ib_srp: Introduce srp_handle_qp_err()
In reply to Bart Van AsscheMove the IB completion handler error handling code into the new function srp_handle_qp_err(). Convert srp_target_port.qp_in_error from int to bool. Move the initialization of that variable into srp_connect_target(). Help the CPU branch predictor by making it clear that IB_WC_SUCCESS is the most likely case.
--- drivers/infiniband/ulp/srp/ib_srp.c | 37 ++++++++++++++++++---------------- drivers/infiniband/ulp/srp/ib_srp.h | 2 +- 2 files changed, 21 insertions(+), 18 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +20 -17
@@ -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;
@@ -648,7 +650,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;
@@ -1215,6 +1216,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;
@@ -1222,15 +1232,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); } }
@@ -1241,16 +1248,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); } }
@@ -2237,7 +2241,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,
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 · 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 v3 3/15 ib_srp: Micro-optimize srp_queuecommand()
In reply to Bart Van AsscheBlock the SCSI host while reconnecting instead of representing the reconnect activity as a distinct SRP target state.
--- 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
@@ -610,13 +610,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
@@ -654,8 +657,7 @@ 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);return ret;
@@ -673,13 +675,15 @@ 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); }spin_unlock_irq (&target->lock); +scsi_target_unblock (&shost->shost_gendev); +return ret; }
@@ -1268,9 +1272,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 ) { scmnd->result =DID_BAD_TARGET <<16 ;
@@ -1334,7 +1335,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 · 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 v3 4/15 ib_srp: Suppress superfluous error messages
In reply to Bart Van AsscheKeep track of the connection state. Don't send any data over the QP after it has been disconnected. 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 :
@@ -1224,8 +1244,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; }
@@ -1585,6 +1608,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" );
@@ -1908,6 +1932,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 · 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 v3 5/15 ib_srp: Avoid that SCSI error handling triggers a crash
In reply to Bart Van AsscheSending 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 | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +4 -2
@@ -1296,7 +1296,8 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)int len;if (target->state ==SRP_TARGET_DEAD || - target->state ==SRP_TARGET_REMOVED ) { + target->state ==SRP_TARGET_REMOVED || + !target->connected) { scmnd->result =DID_BAD_TARGET <<16 ; scmnd->scsi_done (scmnd);return 0 ;
@@ -1647,7 +1648,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->connected)return -1 ;init_completion (&target->tsk_mgmt_done);
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 v3 6/15 ib_srp: Introduce the helper function srp_remove_target()
In reply to Bart Van Assche--- 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 · 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 v3 7/15 ib_srp: Eliminate state SRP_TARGET_DEAD
In reply to Bart Van AsscheOnly 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 a call to srp_remove_target(). Rename srp_target_port.work into srp_target_port.remove_work to reflect its usage. Move function srp_change_state() to just before srp_change_state_to_removed() to avoid having to introduce a forward declaration.
--- drivers/infiniband/ulp/srp/ib_srp.c | 108 +++++++++++++++-------------------- drivers/infiniband/ulp/srp/ib_srp.h | 5 +- 2 files changed, 49 insertions(+), 64 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +47 -61
@@ -428,6 +428,27 @@ 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; + +spin_lock_irq (&target->lock); +if (target->state == old) { + target->state = new; + changed = true; + } +spin_unlock_irq (&target->lock); + +return changed; +} + +static bool srp_change_state_to_removed (struct srp_target_port *target) +{ +return srp_change_state (target,SRP_TARGET_LIVE ,SRP_TARGET_REMOVED ); +} +static bool srp_change_conn_state (struct srp_target_port *target,bool connected) {
@@ -458,21 +479,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 +514,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 +529,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);
@@ -695,17 +703,9 @@ 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); +if (srp_change_state_to_removed (target)) +queue_work (ib_wq, &target->remove_work);scsi_target_unblock (&shost->shost_gendev);
@@ -1300,9 +1300,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 || - !target->connected) { +if (target->state ==SRP_TARGET_REMOVED || !target->connected) { scmnd->result =DID_BAD_TARGET <<16 ; scmnd->scsi_done (scmnd);return 0 ;
@@ -1652,9 +1650,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 || - !target->connected) +if (target->state ==SRP_TARGET_REMOVED || !target->connected)return -1 ;init_completion (&target->tsk_mgmt_done);
@@ -2229,6 +2225,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);
@@ -2462,8 +2459,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);
@@ -2476,36 +2472,26 @@ 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. Wait for any removal tasks that +* may already have been started. */ 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); +while (!list_empty (&host->target_list)) { + target =list_first_entry (&host->target_list, + struct srp_target_port, list); +if (srp_change_state_to_removed (target)) { +list_del (&target->list); +spin_unlock (&host->target_lock); +srp_remove_target (target); +spin_lock (&host->target_lock); + }else { +spin_unlock (&host->target_lock); +flush_work (&target->remove_work); +spin_lock (&host->target_lock); + } }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. -*/ -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); - } -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 · 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 v3 8/15 ib_srp: Make srp_disconnect_target() wait for IB completions
In reply to Bart Van AsscheModify srp_disconnect_target() such that it waits until it is sure that no new IB completions will be received.
--- drivers/infiniband/ulp/srp/ib_srp.c | 99 ++++++++++++++++++++++++++++++----- drivers/infiniband/ulp/srp/ib_srp.h | 3 + 2 files changed, 89 insertions(+), 13 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +86 -13
@@ -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>
@@ -91,6 +91,7 @@ static void srp_remove_one(struct ib_device *device);static void srp_recv_completion (struct ib_cq *cq,void *target_ptr);static void srp_send_completion (struct ib_cq *cq,void *target_ptr);static int srp_cm_handler (struct ib_cm_id *cm_id,struct ib_cm_event *event); +static void srp_reset_req (struct srp_target_port *target,struct srp_request *req);static struct scsi_transport_template *ib_srp_transport_template;
@@ -229,14 +230,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;
@@ -464,11 +467,67 @@ 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 = { + }; +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 = { + }; +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,
@@ -477,6 +536,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)
@@ -520,7 +593,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);
@@ -548,6 +620,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)
@@ -645,7 +719,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 )
@@ -671,11 +744,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)
@@ -1249,7 +1317,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",
@@ -1267,8 +1335,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 ==0 ) { + target->last_recv_wqe = true; +wake_up (&target->qp_wq); + }srp_handle_qp_err (wc.status, wc.opcode, target); -break ; } } }
@@ -1284,8 +1355,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 ==0 ) + target->last_send_wqe = true;srp_handle_qp_err (wc.status, wc.opcode, target); -break ; } } }
@@ -2229,6 +2301,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
@@ -180,6 +180,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 · 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 v3 9/15 srp_transport: Fix atttribute registration
In reply to Bart Van AsscheRegister 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
Bart Van Assche
maintainerPATCH 10/15 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
PATCH v3 11/15 srp_transport: Document sysfs attributes
In reply to Bart Van Assche--- 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 · 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 v3 12/15 ib_srp: Document sysfs attributes
In reply to Bart Van AsscheDocument 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 with 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 · 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
Bart Van Assche
maintainerPATCH 13/15 ib_srp: Allow SRP disconnect through sysfs
In reply to Bart Van AsscheMake 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 | 24 +++++++++++++++++++++--- drivers/scsi/scsi_transport_srp.c | 22 +++++++++++++++++++++- include/scsi/scsi_transport_srp.h | 8 ++++++++ 4 files changed, 57 insertions(+), 4 deletions(-)
Documentation/ABI/stable/sysfs-transport-srp +7 -0
@@ -1,3 +1,10 @@ +What: /sys/class/srp_remote_ports/port-<h>:<n>/delete +Date: January 1, 2012 +KernelVersion: 3.4 +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 +21 -3
@@ -612,6 +612,21 @@ static void srp_remove_work(struct work_struct *work)srp_remove_target (target); } +/* +* Note: it is up to the caller to ensure that invoking srp_rport_delete() +* does not trigger a race against target port removal in srp_remove_target(). +* As an example, invoking srp_rport_delete() from the SCSI EH is not safe. +*/ +static void srp_rport_delete (struct srp_rport *rport) +{ +struct srp_target_port *target = rport->lld_data; + +BUG_ON (!target); + +if (srp_change_state_to_removed (target)) +queue_work (system_long_wq, &target->remove_work); +} +static int srp_connect_target (struct srp_target_port *target) {int retries =3 ;
@@ -1982,6 +1997,10 @@ static struct scsi_host_template srp_template = { .shost_attrs = srp_host_attrs }; +static struct srp_function_template ib_srp_transport_functions = { + .rport_delete = srp_rport_delete, +}; +static int srp_add_target (struct srp_host *host,struct srp_target_port *target) {struct srp_rport_identifiers ids;
@@ -2002,6 +2021,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);
@@ -2576,9 +2597,6 @@ static void srp_remove_one(struct ib_device *device)kfree (srp_dev); } -static struct srp_function_template ib_srp_transport_functions = { -}; -static int __init srp_init_module (void ) {int ret;
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 · 1 line
1.7.7
Re: [PATCH 13/15] ib_srp: Allow SRP disconnect through sysfs
drivers/infiniband/ulp/srp/ib_srp.c static void srp_remove_work(struct work_struct *work) · line 625
Quoted 6 lines · Bart Van Assche
drivers/infiniband/ulp/srp/ib_srp.c +6
-0+static void srp_rport_delete (struct srp_rport *rport) +{ +struct srp_target_port *target = rport->lld_data; + +BUG_ON (!target); +
I don't think this null check is needed, because below you set the lld_data before you call srp_rport_add which does the transport driver model/sysfs file addition for the rport. So I think the rport delete sysfs file won't show up before you set the lld_data.
drivers/infiniband/ulp/srp/ib_srp.c static int srp_add_target(struct srp_host *host, struct srp_target_port *target) · line 2025
Quoted 9 lines
static int
srp_add_target(struct srp_host *host, struct srp_target_port *target){drivers/infiniband/ulp/srp/ib_srp.c +2
-0struct srp_rport_identifiers ids;@@ -2002,6 +2021,8 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)return PTR_ERR (rport); } + rport->lld_data = target; +
--
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 13/15] ib_srp: Allow SRP disconnect through sysfs
Quoted 12 lines · Bart Van Assche
drivers/infiniband/ulp/srp/ib_srp.c static void srp_remove_work(struct work_struct *work) · line 625
Quoted 6 lines
drivers/infiniband/ulp/srp/ib_srp.c +6
-0+static void srp_rport_delete (struct srp_rport *rport) +{ +struct srp_target_port *target = rport->lld_data; + +BUG_ON (!target); +I don't think this null check is needed, because below you set the
lld_databefore you callsrp_rport_addwhich does the transport driver model/sysfs file addition for the rport. So I think the rport delete sysfs file won't show up before you set thelld_data.
Agreed. I'm fine with leaving that check out.
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 v3 14/15 ib_srp: Introduce a temporary variable in srp_remove_target()
In reply to Bart Van Assche--- drivers/infiniband/ulp/srp/ib_srp.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +6 -4
@@ -587,15 +587,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 · 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 v3 15/15 ib_srp: Maintain a single connection per I_T nexus
In reply to Bart Van AsscheThe 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 | 124 ++++++++++++++++++++++++++++------- drivers/infiniband/ulp/srp/ib_srp.h | 10 +++ 2 files changed, 110 insertions(+), 24 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +100 -24
@@ -449,7 +449,16 @@ static bool srp_change_state(struct srp_target_port *target,static bool srp_change_state_to_removed (struct srp_target_port *target) { -return srp_change_state (target,SRP_TARGET_LIVE ,SRP_TARGET_REMOVED ); +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); + +return changed; } static bool srp_change_conn_state(struct srp_target_port *target,
@@ -592,9 +601,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_remove_host (shost); +scsi_remove_host (shost); + }srp_disconnect_target (target); +mutex_unlock (&target->mutex); +srp_free_target_ib (target);srp_free_req_data (target);scsi_host_put (shost);
@@ -629,6 +644,30 @@ static void srp_rport_delete(struct srp_rport *rport)queue_work (system_long_wq, &target->remove_work); } +/** +* 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 ;
@@ -738,8 +777,20 @@ 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_change_state_to_removed (target)) { +shost_printk (KERN_INFO , target->scsi_host, +PFX "deleting SCSI host because obsolete.\n" ); +queue_work (system_long_wq, &target->remove_work); +return -ENXIO ; + } +if (target->state ==SRP_TARGET_REMOVED ) { +shost_printk (KERN_DEBUG , target->scsi_host, +PFX "Already removed\n" );return -EAGAIN ; + } + +mutex_lock (&target->mutex);scsi_target_block (&shost->shost_gendev);
@@ -775,8 +826,11 @@ static int srp_reconnect_target(struct srp_target_port *target)if (ret) goto err; +unblock:scsi_target_unblock (&shost->shost_gendev); +mutex_unlock (&target->mutex); +return ret; err:
@@ -792,9 +846,7 @@ err:if (srp_change_state_to_removed (target))queue_work (ib_wq, &target->remove_work); -scsi_target_unblock (&shost->shost_gendev); - -return ret; + goto unblock; } static void srp_map_desc(struct srp_map_state *state, dma_addr_t dma_addr,
@@ -1711,6 +1763,10 @@ 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_change_state_to_removed (target)) +queue_work (system_long_wq, &target->remove_work); + comp =1 ; target->status =0 ;break ;
@@ -2025,16 +2081,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 ; }
@@ -2320,6 +2366,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);
@@ -2366,24 +2413,53 @@ 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_change_state_to_removed (target)) { +shost_printk (KERN_INFO , target->scsi_host, +PFX "deleting SCSI host (ret = %d).\n" , ret); +srp_remove_work (&target->remove_work); + } +return ret; err_free_ib:srp_free_target_ib (target);
drivers/infiniband/ulp/srp/ib_srp.h +10 -0
@@ -78,7 +78,15 @@ enum { SRP_MAP_NO_FMR = 1, }; +/** +* enum srp_target_state - State of the SCSI host associated with an SRP target. +* @SRP_TARGET_CONNECTING: IB connection being established and SCSI host being +* added. +* @SRP_TARGET_LIVE: IB RC connection is established and SCSI host is unblocked. +* @SRP_TARGET_REMOVED: SCSI host removal is pending. +*/ enum srp_target_state { +SRP_TARGET_CONNECTING ,SRP_TARGET_LIVE ,SRP_TARGET_REMOVED , };
@@ -163,6 +171,7 @@ struct srp_target_port {u32 rq_tmo_jiffies;bool connected; +bool scsi_host_added;struct ib_cm_id *cm_id;
@@ -183,6 +192,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 · 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
Quoted 1 line · Bart Van Assche
This patch series makes the
ib_srpdriver better suited for use in a H.A. setup because [ ... ]
The patch series is also available here: http://github.com/bvanassche/linux/commits/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
Quoted 7 lines · Bart Van Assche
Quoted 2 lines · Bart Van Assche
This patch series makes the
ib_srpdriver better suited for use in a H.A. setup because [ ... ]The patch series is also available here:
http://github.com/bvanassche/linux/commits/srp-ha/.
Thanks Bart, seems the work mail server is still mangling whitespace, so the repo link is much appreciated. I'll try to review these tomorrow afternoon.
Signature · 8 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