* [PATCH 01/18] ib_srp: Introduce pr_fmt()
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
@ 2012-01-14 12:39 ` Bart Van Assche
2012-02-26 6:31 ` David Dillow
2012-01-14 12:40 ` [PATCH 02/18] ib_srp: Consolidate repetitive sysfs code Bart Van Assche
` (13 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:39 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow, Roland Dreier
Make the logging code a little more brief by replacing
printk(KERN_WARNING PFX ...) by pr_warn(...) and by replacing
printk(KERN_ERR PFX ...) by pr_err(...). Join log messages
split over multiple lines into a single line to make it easier
to grep for these messages. Change the severity of the log
statement in srp_qp_event() from "error" to "debug". Change a
double space into a single in the "bad IO class parameter" error
message. Remove one trailing space to avoid a checkpatch warning.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 54 ++++++++++++++++++----------------
1 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 0bfa545..895a9ff 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -30,6 +30,8 @@
* SOFTWARE.
*/
+#define pr_fmt(fmt) PFX fmt
+
#include <linux/module.h>
#include <linux/init.h>
#include <linux/slab.h>
@@ -165,7 +167,7 @@ static void srp_free_iu(struct srp_host *host, struct srp_iu *iu)
static void srp_qp_event(struct ib_event *event, void *context)
{
- printk(KERN_ERR PFX "QP event %d\n", event->event);
+ pr_debug("QP event %d\n", event->event);
}
static int srp_init_qp(struct srp_target_port *target,
@@ -1989,7 +1991,7 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
goto out;
}
if (strlen(p) != 32) {
- printk(KERN_WARNING PFX "bad dest GID parameter '%s'\n", p);
+ pr_warn("bad dest GID parameter '%s'\n", p);
kfree(p);
goto out;
}
@@ -2004,7 +2006,7 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
case SRP_OPT_PKEY:
if (match_hex(args, &token)) {
- printk(KERN_WARNING PFX "bad P_Key parameter '%s'\n", p);
+ pr_warn("bad P_Key parameter '%s'\n", p);
goto out;
}
target->path.pkey = cpu_to_be16(token);
@@ -2023,7 +2025,7 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
case SRP_OPT_MAX_SECT:
if (match_int(args, &token)) {
- printk(KERN_WARNING PFX "bad max sect parameter '%s'\n", p);
+ pr_warn("bad max sect parameter '%s'\n", p);
goto out;
}
target->scsi_host->max_sectors = token;
@@ -2031,7 +2033,8 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
case SRP_OPT_MAX_CMD_PER_LUN:
if (match_int(args, &token)) {
- printk(KERN_WARNING PFX "bad max cmd_per_lun parameter '%s'\n", p);
+ pr_warn("bad max cmd_per_lun parameter '%s'\n",
+ p);
goto out;
}
target->scsi_host->cmd_per_lun = min(token, SRP_CMD_SQ_SIZE);
@@ -2039,14 +2042,14 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
case SRP_OPT_IO_CLASS:
if (match_hex(args, &token)) {
- printk(KERN_WARNING PFX "bad IO class parameter '%s' \n", p);
+ pr_warn("bad IO class parameter '%s'\n", p);
goto out;
}
if (token != SRP_REV10_IB_IO_CLASS &&
token != SRP_REV16A_IB_IO_CLASS) {
- printk(KERN_WARNING PFX "unknown IO class parameter value"
- " %x specified (use %x or %x).\n",
- token, SRP_REV10_IB_IO_CLASS, SRP_REV16A_IB_IO_CLASS);
+ pr_warn("unknown IO class parameter value %x specified (use %x or %x).\n",
+ token, SRP_REV10_IB_IO_CLASS,
+ SRP_REV16A_IB_IO_CLASS);
goto out;
}
target->io_class = token;
@@ -2064,7 +2067,8 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
case SRP_OPT_CMD_SG_ENTRIES:
if (match_int(args, &token) || token < 1 || token > 255) {
- printk(KERN_WARNING PFX "bad max cmd_sg_entries parameter '%s'\n", p);
+ pr_warn("bad max cmd_sg_entries parameter '%s'\n",
+ p);
goto out;
}
target->cmd_sg_cnt = token;
@@ -2072,7 +2076,7 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
case SRP_OPT_ALLOW_EXT_SG:
if (match_int(args, &token)) {
- printk(KERN_WARNING PFX "bad allow_ext_sg parameter '%s'\n", p);
+ pr_warn("bad allow_ext_sg parameter '%s'\n", p);
goto out;
}
target->allow_ext_sg = !!token;
@@ -2081,15 +2085,16 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
case SRP_OPT_SG_TABLESIZE:
if (match_int(args, &token) || token < 1 ||
token > SCSI_MAX_SG_CHAIN_SEGMENTS) {
- printk(KERN_WARNING PFX "bad max sg_tablesize parameter '%s'\n", p);
+ pr_warn("bad max sg_tablesize parameter '%s'\n",
+ p);
goto out;
}
target->sg_tablesize = token;
break;
default:
- printk(KERN_WARNING PFX "unknown parameter or missing value "
- "'%s' in target creation request\n", p);
+ pr_warn("unknown parameter or missing value '%s' in target creation request\n",
+ p);
goto out;
}
}
@@ -2100,9 +2105,8 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
for (i = 0; i < ARRAY_SIZE(srp_opt_tokens); ++i)
if ((srp_opt_tokens[i].token & SRP_OPT_ALL) &&
!(srp_opt_tokens[i].token & opt_mask))
- printk(KERN_WARNING PFX "target creation request is "
- "missing parameter '%s'\n",
- srp_opt_tokens[i].pattern);
+ pr_warn("target creation request is missing parameter '%s'\n",
+ srp_opt_tokens[i].pattern);
out:
kfree(options);
@@ -2149,7 +2153,7 @@ static ssize_t srp_create_target(struct device *dev,
if (!host->srp_dev->fmr_pool && !target->allow_ext_sg &&
target->cmd_sg_cnt < target->sg_tablesize) {
- printk(KERN_WARNING PFX "No FMR pool and no external indirect descriptors, limiting sg_tablesize to cmd_sg_cnt\n");
+ pr_warn("No FMR pool and no external indirect descriptors, limiting sg_tablesize to cmd_sg_cnt\n");
target->sg_tablesize = target->cmd_sg_cnt;
}
@@ -2309,8 +2313,7 @@ static void srp_add_one(struct ib_device *device)
return;
if (ib_query_device(device, dev_attr)) {
- printk(KERN_WARNING PFX "Query device failed for %s\n",
- device->name);
+ pr_warn("Query device failed for %s\n", device->name);
goto free_attr;
}
@@ -2459,7 +2462,7 @@ static int __init srp_init_module(void)
BUILD_BUG_ON(FIELD_SIZEOF(struct ib_wc, wr_id) < sizeof(void *));
if (srp_sg_tablesize) {
- printk(KERN_WARNING PFX "srp_sg_tablesize is deprecated, please use cmd_sg_entries\n");
+ pr_warn("srp_sg_tablesize is deprecated, please use cmd_sg_entries\n");
if (!cmd_sg_entries)
cmd_sg_entries = srp_sg_tablesize;
}
@@ -2468,14 +2471,15 @@ static int __init srp_init_module(void)
cmd_sg_entries = SRP_DEF_SG_TABLESIZE;
if (cmd_sg_entries > 255) {
- printk(KERN_WARNING PFX "Clamping cmd_sg_entries to 255\n");
+ pr_warn("Clamping cmd_sg_entries to 255\n");
cmd_sg_entries = 255;
}
if (!indirect_sg_entries)
indirect_sg_entries = cmd_sg_entries;
else if (indirect_sg_entries < cmd_sg_entries) {
- printk(KERN_WARNING PFX "Bumping up indirect_sg_entries to match cmd_sg_entries (%u)\n", cmd_sg_entries);
+ pr_warn("Bumping up indirect_sg_entries to match cmd_sg_entries (%u)\n",
+ cmd_sg_entries);
indirect_sg_entries = cmd_sg_entries;
}
@@ -2486,7 +2490,7 @@ static int __init srp_init_module(void)
ret = class_register(&srp_class);
if (ret) {
- printk(KERN_ERR PFX "couldn't register class infiniband_srp\n");
+ pr_err("couldn't register class infiniband_srp\n");
srp_release_transport(ib_srp_transport_template);
return ret;
}
@@ -2495,7 +2499,7 @@ static int __init srp_init_module(void)
ret = ib_register_client(&srp_client);
if (ret) {
- printk(KERN_ERR PFX "couldn't register IB client\n");
+ pr_err("couldn't register IB client\n");
srp_release_transport(ib_srp_transport_template);
ib_sa_unregister_client(&srp_sa_client);
class_unregister(&srp_class);
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 01/18] ib_srp: Introduce pr_fmt()
2012-01-14 12:39 ` [PATCH 01/18] ib_srp: Introduce pr_fmt() Bart Van Assche
@ 2012-02-26 6:31 ` David Dillow
0 siblings, 0 replies; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:31 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Roland Dreier
On Sat, 2012-01-14 at 12:39 +0000, Bart Van Assche wrote:
> Make the logging code a little more brief by replacing
> printk(KERN_WARNING PFX ...) by pr_warn(...) and by replacing
> printk(KERN_ERR PFX ...) by pr_err(...). Join log messages
> split over multiple lines into a single line to make it easier
> to grep for these messages. Change the severity of the log
> statement in srp_qp_event() from "error" to "debug". Change a
> double space into a single in the "bad IO class parameter" error
> message. Remove one trailing space to avoid a checkpatch warning.
All you really need for the commit message is "Using pr_* is preferred
over printk". The rest is superfluous and readily apparent from the
patch itself.
Otherwise, this patch is fine.
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 02/18] ib_srp: Consolidate repetitive sysfs code
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
2012-01-14 12:39 ` [PATCH 01/18] ib_srp: Introduce pr_fmt() Bart Van Assche
@ 2012-01-14 12:40 ` Bart Van Assche
2012-02-26 6:31 ` David Dillow
2012-01-14 12:41 ` [PATCH 03/18] ib_srp: Enlarge block layer timeout Bart Van Assche
` (12 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:40 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow, Roland Dreier
Remove sysfs attributes before removing a target instead of
testing the target state in every sysfs attribute callback
method. Note: it is safe to invoke a sysfs attribute removal
method like device_remove_file() twice on the same attribute.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 49 ++++++++++++----------------------
1 files changed, 17 insertions(+), 32 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 895a9ff..bcbf22e 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -474,6 +474,21 @@ static void srp_free_req_data(struct srp_target_port *target)
}
}
+/**
+ * srp_del_scsi_host_attr() - Remove attributes defined in the host template.
+ * @shost: SCSI host whose attributes to remove from sysfs.
+ *
+ * Note: Any attributes defined in the host template and that did not exist
+ * before invocation of this function will be ignored.
+ */
+static void srp_del_scsi_host_attr(struct Scsi_Host *shost)
+{
+ struct device_attribute **attr;
+
+ for (attr = shost->hostt->shost_attrs; attr && *attr; ++attr)
+ device_remove_file(&shost->shost_dev, *attr);
+}
+
static void srp_remove_work(struct work_struct *work)
{
struct srp_target_port *target =
@@ -486,6 +501,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);
@@ -1678,10 +1694,6 @@ static ssize_t show_id_ext(struct device *dev, struct device_attribute *attr,
{
struct srp_target_port *target = host_to_target(class_to_shost(dev));
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED)
- return -ENODEV;
-
return sprintf(buf, "0x%016llx\n",
(unsigned long long) be64_to_cpu(target->id_ext));
}
@@ -1691,10 +1703,6 @@ static ssize_t show_ioc_guid(struct device *dev, struct device_attribute *attr,
{
struct srp_target_port *target = host_to_target(class_to_shost(dev));
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED)
- return -ENODEV;
-
return sprintf(buf, "0x%016llx\n",
(unsigned long long) be64_to_cpu(target->ioc_guid));
}
@@ -1704,10 +1712,6 @@ static ssize_t show_service_id(struct device *dev,
{
struct srp_target_port *target = host_to_target(class_to_shost(dev));
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED)
- return -ENODEV;
-
return sprintf(buf, "0x%016llx\n",
(unsigned long long) be64_to_cpu(target->service_id));
}
@@ -1717,10 +1721,6 @@ static ssize_t show_pkey(struct device *dev, struct device_attribute *attr,
{
struct srp_target_port *target = host_to_target(class_to_shost(dev));
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED)
- return -ENODEV;
-
return sprintf(buf, "0x%04x\n", be16_to_cpu(target->path.pkey));
}
@@ -1729,10 +1729,6 @@ static ssize_t show_dgid(struct device *dev, struct device_attribute *attr,
{
struct srp_target_port *target = host_to_target(class_to_shost(dev));
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED)
- return -ENODEV;
-
return sprintf(buf, "%pI6\n", target->path.dgid.raw);
}
@@ -1741,10 +1737,6 @@ static ssize_t show_orig_dgid(struct device *dev,
{
struct srp_target_port *target = host_to_target(class_to_shost(dev));
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED)
- return -ENODEV;
-
return sprintf(buf, "%pI6\n", target->orig_dgid);
}
@@ -1753,10 +1745,6 @@ static ssize_t show_req_lim(struct device *dev,
{
struct srp_target_port *target = host_to_target(class_to_shost(dev));
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED)
- return -ENODEV;
-
return sprintf(buf, "%d\n", target->req_lim);
}
@@ -1765,10 +1753,6 @@ static ssize_t show_zero_req_lim(struct device *dev,
{
struct srp_target_port *target = host_to_target(class_to_shost(dev));
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED)
- return -ENODEV;
-
return sprintf(buf, "%d\n", target->zero_req_lim);
}
@@ -2432,6 +2416,7 @@ static void srp_remove_one(struct ib_device *device)
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);
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 03/18] ib_srp: Enlarge block layer timeout
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
2012-01-14 12:39 ` [PATCH 01/18] ib_srp: Introduce pr_fmt() Bart Van Assche
2012-01-14 12:40 ` [PATCH 02/18] ib_srp: Consolidate repetitive sysfs code Bart Van Assche
@ 2012-01-14 12:41 ` Bart Van Assche
2012-02-26 6:32 ` David Dillow
2012-01-14 12:42 ` [PATCH 04/18] ib_srp: Micro-optimize completion handlers Bart Van Assche
` (11 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:41 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow, Roland Dreier
Enlarge the block layer timeout 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. Note: the timeout is
only set for SCSI disk devices but not for any other type of
SCSI device (M/O disk, tape, CD-ROM, ...).
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 46 +++++++++++++++++++++++++++++++++++
drivers/infiniband/ulp/srp/ib_srp.h | 2 +
2 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index bcbf22e..b1327c6 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -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 = 1ULL << (12 + 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,22 @@ 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;
+
+ WARN_ON(target->rq_tmo_jiffies == 0);
+ 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 +1866,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,
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 020caf0..e3a6304 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -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;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 03/18] ib_srp: Enlarge block layer timeout
2012-01-14 12:41 ` [PATCH 03/18] ib_srp: Enlarge block layer timeout Bart Van Assche
@ 2012-02-26 6:32 ` David Dillow
[not found] ` <1330237921.1026.81.camel-1q1vX8mYZiGLUyTwlgNVppKKF0rrzTr+@public.gmane.org>
0 siblings, 1 reply; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:32 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Roland Dreier
On Sat, 2012-01-14 at 12:41 +0000, Bart Van Assche wrote:
> Enlarge the block layer timeout 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. Note: the timeout is
> only set for SCSI disk devices but not for any other type of
> SCSI device (M/O disk, tape, CD-ROM, ...).
Why disk only? Different default timeouts? If so, there's a better
solution.
> +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));
Are you guaranteed to get those attributes set if they aren't changed
from the default? You'd get a splat from this non-error case. The user
at least can go looking for the cause of the splat and find a pointer to
why, but I contend it is more user-friendly to just cope:
/* Use the defaults from ITBA spec if CM doesn't supply them */
timeout = (attr_mask * IP_QP_TIMEOUT) ? qp_attr->timeout : 19;
retries = (attr_mask & IP_QP_RETRY_CNT) ? qp_attr->retry_cnt : 7;
> + * 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 = 1ULL << (12 + qp_attr->timeout);
I still think T_tr_ns = 4096 * (1ULL << timeout) is more readable, and
the compiler will do the right thing -- not that it matters in this slow
path.
> +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;
> +
> + WARN_ON(target->rq_tmo_jiffies == 0);
> + if (sdev->type == TYPE_DISK) {
> + timeout = max_t(unsigned, 30 * HZ, target->rq_tmo_jiffies);
> + blk_queue_rq_timeout(q, timeout);
> + }
I think you can get rid of any dependency on the default values by just
doing
if (q->rq_timeout < target->rq_tmo_jiffies)
blk_queue_rq_timeout(q, timeout);
This -- or the changes to srp_compute_rq_tmo() -- would also avoid the
need for the WARN_ON().
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 04/18] ib_srp: Micro-optimize completion handlers
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (2 preceding siblings ...)
2012-01-14 12:41 ` [PATCH 03/18] ib_srp: Enlarge block layer timeout Bart Van Assche
@ 2012-01-14 12:42 ` Bart Van Assche
2012-02-26 6:32 ` David Dillow
2012-01-14 12:43 ` [PATCH 05/18] ib_srp: Separate connection and host state Bart Van Assche
` (10 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:42 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
Reduce completion queue lock contention by polling for multiple
work completions at once. Help the CPU branch predictor by making
it clear that IB_WC_SUCCESS is the most likely case. Move the
error handling code into the new function srp_handle_qp_err().
Also, convert srp_target_port.qp_in_error from int to bool and
move the initialization of that variable into srp_connect_target().
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 57 +++++++++++++++++++++--------------
drivers/infiniband/ulp/srp/ib_srp.h | 4 ++-
2 files changed, 37 insertions(+), 24 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index b1327c6..afbddd8 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -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,42 +1216,53 @@ 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;
- struct ib_wc wc;
+ struct ib_wc *const wc = target->recv_wc;
+ int i, n;
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;
- break;
+ while ((n = ib_poll_cq(cq, ARRAY_SIZE(target->recv_wc), wc)) > 0) {
+ for (i = 0; i < n; ++i) {
+ if (likely(wc[i].status == IB_WC_SUCCESS)) {
+ srp_handle_recv(target, &wc[i]);
+ } else {
+ srp_handle_qp_err(wc[i].status, wc[i].opcode,
+ target);
+ return;
+ }
}
-
- srp_handle_recv(target, &wc);
}
}
static void srp_send_completion(struct ib_cq *cq, void *target_ptr)
{
struct srp_target_port *target = target_ptr;
- struct ib_wc wc;
+ struct ib_wc *const wc = target->send_wc;
+ int i, n;
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;
- break;
+ while ((n = ib_poll_cq(cq, ARRAY_SIZE(target->send_wc), wc)) > 0) {
+ for (i = 0; i < n; ++i) {
+ if (likely(wc[i].status == IB_WC_SUCCESS)) {
+ iu = (struct srp_iu *) (uintptr_t) wc[i].wr_id;
+ list_add(&iu->list, &target->free_tx);
+ } else {
+ srp_handle_qp_err(wc[i].status, wc[i].opcode,
+ target);
+ return;
+ }
}
-
- iu = (struct srp_iu *) (uintptr_t) wc.wr_id;
- list_add(&iu->list, &target->free_tx);
}
}
@@ -2238,7 +2250,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,
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index e3a6304..91f92aa 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -135,6 +135,8 @@ struct srp_target_port {
struct ib_cq *send_cq ____cacheline_aligned_in_smp;
struct ib_cq *recv_cq;
struct ib_qp *qp;
+ struct ib_wc recv_wc[16];
+ struct ib_wc send_wc[16];
u32 lkey;
u32 rkey;
enum srp_target_state state;
@@ -180,7 +182,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;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 04/18] ib_srp: Micro-optimize completion handlers
2012-01-14 12:42 ` [PATCH 04/18] ib_srp: Micro-optimize completion handlers Bart Van Assche
@ 2012-02-26 6:32 ` David Dillow
0 siblings, 0 replies; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:32 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg
On Sat, 2012-01-14 at 12:42 +0000, Bart Van Assche wrote:
> Reduce completion queue lock contention by polling for multiple
> work completions at once. Help the CPU branch predictor by making
> it clear that IB_WC_SUCCESS is the most likely case. Move the
> error handling code into the new function srp_handle_qp_err().
>
> Also, convert srp_target_port.qp_in_error from int to bool and
> move the initialization of that variable into srp_connect_target().
Don't merge the two different changes here. Introducing
srp_handle_qp_err() is fine, but the change to polling for multiple WC
is completely unrelated to that and to the stated intent of this series.
It also hurt performance when I measured it during the IOP scaling work.
Let's get this series out of the way, then we can talk about the WC
changes.
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 05/18] ib_srp: Separate connection and host state
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (3 preceding siblings ...)
2012-01-14 12:42 ` [PATCH 04/18] ib_srp: Micro-optimize completion handlers Bart Van Assche
@ 2012-01-14 12:43 ` Bart Van Assche
2012-02-26 6:32 ` David Dillow
2012-01-14 12:44 ` [PATCH 06/18] ib_srp: Wait for last completion when disconnecting Bart Van Assche
` (9 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:43 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
Separate connection and host state. Only report QP errors while
connected. Only invoke ib_send_cm_dreq() from inside
srp_disconnect_target() when connected such that invoking
srp_disconnect_target() after having received a DREQ does not
cause an error message to be printed.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 52 +++++++++++++++++++++++++----------
drivers/infiniband/ulp/srp/ib_srp.h | 2 +-
2 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index afbddd8..ad8f168 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -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:
@@ -614,7 +634,7 @@ static int srp_reconnect_target(struct srp_target_port *target)
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;
srp_disconnect_target(target);
@@ -654,9 +674,6 @@ 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;
-
return ret;
err:
@@ -673,7 +690,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);
@@ -1220,8 +1237,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;
}
@@ -1276,7 +1296,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
unsigned long flags;
int len;
- if (target->state == SRP_TARGET_CONNECTING)
+ if (!target->connected)
goto err;
if (target->state == SRP_TARGET_DEAD ||
@@ -1593,6 +1613,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");
@@ -1917,6 +1938,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);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 91f92aa..5f288fe 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -80,7 +80,6 @@ enum {
enum srp_target_state {
SRP_TARGET_LIVE,
- SRP_TARGET_CONNECTING,
SRP_TARGET_DEAD,
SRP_TARGET_REMOVED
};
@@ -166,6 +165,7 @@ struct srp_target_port {
int path_query_id;
u32 rq_tmo_jiffies;
+ bool connected;
struct ib_cm_id *cm_id;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 05/18] ib_srp: Separate connection and host state
2012-01-14 12:43 ` [PATCH 05/18] ib_srp: Separate connection and host state Bart Van Assche
@ 2012-02-26 6:32 ` David Dillow
[not found] ` <1330237948.1026.83.camel-1q1vX8mYZiGLUyTwlgNVppKKF0rrzTr+@public.gmane.org>
0 siblings, 1 reply; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:32 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg
On Sat, 2012-01-14 at 12:43 +0000, Bart Van Assche wrote:
> Separate connection and host state. Only report QP errors while
> connected. Only invoke ib_send_cm_dreq() from inside
> srp_disconnect_target() when connected such that invoking
> srp_disconnect_target() after having received a DREQ does not
> cause an error message to be printed.
I'm not sure that splitting connection state from the target state is
really buying you anything other than more storage and complexity.
I looked to later patches for a reason this makes sense, but I'm coming
up short, so maybe I'm just missing it.
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 06/18] ib_srp: Wait for last completion when disconnecting
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (4 preceding siblings ...)
2012-01-14 12:43 ` [PATCH 05/18] ib_srp: Separate connection and host state Bart Van Assche
@ 2012-01-14 12:44 ` Bart Van Assche
2012-02-26 6:32 ` David Dillow
2012-01-14 12:45 ` [PATCH 07/18] ib_srp: Introduce three helper functions Bart Van Assche
` (8 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:44 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
When disconnecting the IB connection via the IB CM, wait until
any invoked completion handlers have finished processing SRP
protocol data and prevent that new work completions are queued.
Change the IB completion handlers such that all error completions
are processed instead of a subset and also such that receiving a
completion with zero wr_id is recognized as an end-of-work marker.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 81 ++++++++++++++++++++++++++++++----
drivers/infiniband/ulp/srp/ib_srp.h | 3 +
2 files changed, 74 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index ad8f168..0265a10 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -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>
@@ -443,8 +443,58 @@ 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)
+{
+ struct ib_recv_wr wr, *bad_wr;
+ int ret;
+
+ if (target->last_recv_wqe)
+ return;
+
+ memset(&wr, 0, sizeof(wr));
+ 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)
+{
+ unsigned long deadline = jiffies + target->rq_tmo_jiffies;
+ struct ib_send_wr wr, *bad_wr;
+ int ret;
+
+ if (target->last_send_wqe)
+ return;
+
+ memset(&wr, 0, sizeof(wr));
+ 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)
{
+ struct ib_qp_attr qp_attr;
+ int ret;
+
if (srp_change_conn_state(target, false)) {
/* XXX should send SRP_I_LOGOUT request */
@@ -456,6 +506,16 @@ static void srp_disconnect_target(struct srp_target_port *target)
wait_for_completion(&target->done);
}
}
+
+ if (target->qp) {
+ qp_attr.qp_state = IB_QPS_ERR;
+ 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 bool srp_change_state(struct srp_target_port *target,
@@ -535,6 +595,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)
@@ -631,7 +693,6 @@ static void srp_reset_req(struct srp_target_port *target, struct srp_request *re
static int srp_reconnect_target(struct srp_target_port *target)
{
struct ib_qp_attr qp_attr;
- struct ib_wc wc;
int i, ret;
if (target->state != SRP_TARGET_LIVE)
@@ -655,11 +716,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)
@@ -1237,7 +1293,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",
@@ -1257,9 +1313,12 @@ static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)
if (likely(wc[i].status == IB_WC_SUCCESS)) {
srp_handle_recv(target, &wc[i]);
} else {
+ if (wc[i].wr_id == 0) {
+ target->last_recv_wqe = true;
+ wake_up(&target->qp_wq);
+ }
srp_handle_qp_err(wc[i].status, wc[i].opcode,
target);
- return;
}
}
}
@@ -1278,9 +1337,10 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr)
iu = (struct srp_iu *) (uintptr_t) wc[i].wr_id;
list_add(&iu->list, &target->free_tx);
} else {
+ if (wc[i].wr_id == 0)
+ target->last_send_wqe = true;
srp_handle_qp_err(wc[i].status, wc[i].opcode,
target);
- return;
}
}
}
@@ -2231,6 +2291,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];
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 5f288fe..fc411ae 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -183,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;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 07/18] ib_srp: Introduce three helper functions
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (5 preceding siblings ...)
2012-01-14 12:44 ` [PATCH 06/18] ib_srp: Wait for last completion when disconnecting Bart Van Assche
@ 2012-01-14 12:45 ` Bart Van Assche
2012-02-26 6:32 ` David Dillow
2012-01-14 12:46 ` [PATCH 08/18] ib_srp: Eliminate state SRP_TARGET_DEAD Bart Van Assche
` (7 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:45 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
Introduce srp_remove_target(), srp_change_state_to_removed() and
srp_scan_target().
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 49 +++++++++++++++++++++++++----------
1 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 0265a10..57f8a6f 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -428,6 +428,20 @@ static int srp_send_req(struct srp_target_port *target)
return status;
}
+static bool srp_change_state_to_removed(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);
+
+ return changed;
+}
+
static bool srp_change_conn_state(struct srp_target_port *target,
bool connected)
{
@@ -566,6 +580,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 =
@@ -578,13 +603,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)
@@ -690,6 +709,12 @@ static void srp_reset_req(struct srp_target_port *target, struct srp_request *re
srp_remove_req(target, req, 0);
}
+static void srp_scan_target(struct srp_target_port *target)
+{
+ scsi_scan_target(&target->scsi_host->shost_gendev, 0, target->scsi_id,
+ SCAN_WILD_CARD, 0);
+}
+
static int srp_reconnect_target(struct srp_target_port *target)
{
struct ib_qp_attr qp_attr;
@@ -2000,8 +2025,7 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
target->state = SRP_TARGET_LIVE;
target->connected = false;
- scsi_scan_target(&target->scsi_host->shost_gendev,
- 0, target->scsi_id, SCAN_WILD_CARD, 0);
+ srp_scan_target(target);
return 0;
}
@@ -2540,11 +2564,8 @@ static void srp_remove_one(struct ib_device *device)
* commands and don't try to reconnect.
*/
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_change_state_to_removed(target);
spin_unlock(&host->target_lock);
/*
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 07/18] ib_srp: Introduce three helper functions
2012-01-14 12:45 ` [PATCH 07/18] ib_srp: Introduce three helper functions Bart Van Assche
@ 2012-02-26 6:32 ` David Dillow
[not found] ` <1330237969.1026.85.camel-1q1vX8mYZiGLUyTwlgNVppKKF0rrzTr+@public.gmane.org>
0 siblings, 1 reply; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:32 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg
On Sat, 2012-01-14 at 12:45 +0000, Bart Van Assche wrote:
> Introduce srp_remove_target(), srp_change_state_to_removed() and
> srp_scan_target().
> +static bool srp_change_state_to_removed(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);
> +
> + return changed;
> +}
I'm not sure why you introduce this here, just to move
srp_change_state() up and change this back to
srp_change_state(target, SRP_TARGET_LIVE, SRP_TARGET_REMOVED);
in the next patch.
There also doesn't seem to be much of a reason to introduce
srp_scan_target() now rather than in patch 18 where you add another use
of it.
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 08/18] ib_srp: Eliminate state SRP_TARGET_DEAD
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (6 preceding siblings ...)
2012-01-14 12:45 ` [PATCH 07/18] ib_srp: Introduce three helper functions Bart Van Assche
@ 2012-01-14 12:46 ` Bart Van Assche
2012-02-26 6:33 ` David Dillow
2012-01-14 12:51 ` [PATCH 12/18] ib_srp: Document sysfs attributes Bart Van Assche
` (6 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:46 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
Only 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. Rename srp_target_port.work into
srp_target_port.remove_work and move function srp_change_state()
to just before srp_change_state_to_removed().
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 97 +++++++++++++---------------------
drivers/infiniband/ulp/srp/ib_srp.h | 5 +-
2 files changed, 39 insertions(+), 63 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 57f8a6f..67932aa 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -428,13 +428,15 @@ static int srp_send_req(struct srp_target_port *target)
return status;
}
-static bool srp_change_state_to_removed(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 != SRP_TARGET_REMOVED) {
- target->state = SRP_TARGET_REMOVED;
+ if (target->state == old) {
+ target->state = new;
changed = true;
}
spin_unlock_irq(&target->lock);
@@ -442,6 +444,11 @@ static bool srp_change_state_to_removed(struct srp_target_port *target)
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)
{
@@ -532,21 +539,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;
@@ -582,9 +574,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);
@@ -594,10 +589,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);
@@ -766,17 +760,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);
return ret;
}
@@ -1384,8 +1370,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
if (!target->connected)
goto err;
- if (target->state == SRP_TARGET_DEAD ||
- target->state == SRP_TARGET_REMOVED) {
+ if (target->state == SRP_TARGET_REMOVED) {
scmnd->result = DID_BAD_TARGET << 16;
scmnd->scsi_done(scmnd);
return 0;
@@ -1736,8 +1721,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)
return -1;
init_completion(&target->tsk_mgmt_done);
@@ -2312,6 +2296,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);
@@ -2546,8 +2531,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);
@@ -2560,32 +2544,25 @@ 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)
- srp_change_state_to_removed(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.
- */
- 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);
+ 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_sync(&target->remove_work);
+ spin_lock(&host->target_lock);
+ }
}
+ spin_unlock(&host->target_lock);
kfree(host);
}
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index fc411ae..3882adf 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -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 {
@@ -177,7 +176,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;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 08/18] ib_srp: Eliminate state SRP_TARGET_DEAD
2012-01-14 12:46 ` [PATCH 08/18] ib_srp: Eliminate state SRP_TARGET_DEAD Bart Van Assche
@ 2012-02-26 6:33 ` David Dillow
0 siblings, 0 replies; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:33 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg
On Sat, 2012-01-14 at 12:46 +0000, Bart Van Assche wrote:
> Only 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. Rename srp_target_port.work into
> srp_target_port.remove_work and move function srp_change_state()
> to just before srp_change_state_to_removed().
Again, commit messages should be about why you changed, not a mechanical
listing of what you change.
"Rename srp_target_port.work into srp_target_port.remove_work" doesn't
tell me anything. I had to read later patches to see that what you mean
was really "Rename srp_target_port.work to reflect its usage and remove
a future conflict when push the SCSI scan into it's own work task."
This would have saved me some head scratching, and is why I try to
review as much as I can of a series in one go -- all too often you make
changes that leave me wondering until I see a later patch.
I'm also not keen on how the code looks after these changes, but I do
like getting rid of the DEAD state... I'm not sure there is a good way
to handle the removal of the module that doesn't look a bit ugly.
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 12/18] ib_srp: Document sysfs attributes
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (7 preceding siblings ...)
2012-01-14 12:46 ` [PATCH 08/18] ib_srp: Eliminate state SRP_TARGET_DEAD Bart Van Assche
@ 2012-01-14 12:51 ` Bart Van Assche
2012-02-26 6:33 ` David Dillow
2012-01-14 12:52 ` [PATCH 13/18] ib_srp: Allow SRP disconnect through sysfs Bart Van Assche
` (5 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:51 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
Document the sysfs attributes of the SRP initiator (ib_srp) according
to the rules specified in Documentation/ABI/README.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
Documentation/ABI/stable/sysfs-driver-ib_srp | 173 ++++++++++++++++++++++++++
1 files changed, 173 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-ib_srp
diff --git a/Documentation/ABI/stable/sysfs-driver-ib_srp b/Documentation/ABI/stable/sysfs-driver-ib_srp
new file mode 100644
index 0000000..0e0224f
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-ib_srp
@@ -0,0 +1,173 @@
+What: /sys/class/infiniband_srp/srp-<hca>-<port_number>/add_target
+Date: January 2, 2006
+KernelVersion: 2.6.15
+Contact: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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 16-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 direct data buffer descriptors in an
+ SRP_CMD. Direct data buffer descriptors are communicated via
+ the SRP_CMD header. With allow_ext_sg=0 the parameter
+ cmd_sg_entries defines the maximum S/G list length for a
+ single SRP_CMD. Also, 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 use indirect data
+ buffer descriptors when communicating with an SRP target.
+ Indirect data buffer descriptors are communicated between
+ initiator and target via an additional RDMA
+ transfer. Enabling indirect data buffer descriptors
+ increases the maximum amount of data that can be transferred
+ via a single SCSI command between initiator and
+ target. Since not all SRP target implementations support
+ indirect data buffer descriptors by default support for
+ indirect data buffer descriptors is disabled.
+ * 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 indirect data buffer 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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: HCA name (<hca>).
+
+What: /sys/class/infiniband_srp/srp-<hca>-<port_number>/port
+Date: January 2, 2006
+KernelVersion: 2.6.15
+Contact: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: Whether ib_srp is allowed to use indirect data buffer
+ descriptors 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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: Maximum number of direct data buffer descriptors to send to
+ the target in a single SRP_CMD.
+
+What: /sys/class/scsi_host/host<n>/dgid
+Date: June 17, 2006
+KernelVersion: 2.6.17
+Contact: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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>/failed_reconnects
+Date: January 1, 2012
+KernelVersion: 3.3
+Contact: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: Number of consecutive failed SRP reconnection attempts.
+
+What: /sys/class/scsi_host/host<n>/id_ext
+Date: June 17, 2006
+KernelVersion: 2.6.17
+Contact: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: A 16-bit number representing the InfiniBand partition key used
+ for communication with the SRP target.
+
+What: /sys/class/scsi_host/host<n>/reconnect_tmo
+Date: January 1, 2012
+KernelVersion: 3.3
+Contact: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: Time in seconds to wait after a failed connection attempt
+ before trying to reconnect. Setting this parameter to zero or
+ to a negative value prevents ib_srp to attempt to reconnect.
+ Changing this parameter from a value <= 0 into a value > 0 at
+ a time there is no connection with the target will cause
+ ib_srp to try to reconnect with the target.
+
+What: /sys/class/scsi_host/host<n>/req_lim
+Date: October 20, 2010
+KernelVersion: 2.6.36
+Contact: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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.
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 12/18] ib_srp: Document sysfs attributes
2012-01-14 12:51 ` [PATCH 12/18] ib_srp: Document sysfs attributes Bart Van Assche
@ 2012-02-26 6:33 ` David Dillow
0 siblings, 0 replies; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:33 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg
On Sat, 2012-01-14 at 12:51 +0000, Bart Van Assche wrote:
> + * cmd_sg_entries, a number in the range 1..255 that specifies
> + the maximum number of direct data buffer descriptors in an
s/direct/indirect/
> + SRP_CMD. Direct data buffer descriptors are communicated via
> + the SRP_CMD header. With allow_ext_sg=0 the parameter
So are indirect buffer descriptors, which is actually what this
parameter controls.
> + cmd_sg_entries defines the maximum S/G list length for a
> + single SRP_CMD. Also, commands whose S/G list length exceeds
s/. Also,/ and/
> + this limit after S/G list collapsing will fail.
> + * allow_ext_sg, whether ib_srp is allowed to use indirect data
> + buffer descriptors when communicating with an SRP target.
allow_ext_sg, if true, the initiator is allowed to use more indirect
data buffer descriptors than will fit in the SRP_CMD, allowing for
larger requests to be handled. If false (the default), the length of the
S/G list after merging is limited to cmd_sg_entries. Target support for
retrieving the indirect descriptor table via RDMA is required to set
this to true.
> + * 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 indirect data buffer support enabled
> + (allow_ext_sg=1).
s/only safe/only completely safe/
If this value exceeds cmd_sg_entries and a failure occurs while
collapsing the list into fewer S/G entries, the command will fail. This
failure case only appears to be possible on IBM HCAs.
> +What: /sys/class/scsi_host/host<n>/allow_ext_sg
> +Date: May 19, 2011
> +KernelVersion: 2.6.39
> +Contact: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description: Whether ib_srp is allowed to use indirect data buffer
> + descriptors when communicating with an SRP target.
Whether the initiator is allowed to use more S/G entries than will fit
in the command request 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: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description: Maximum number of direct data buffer descriptors to send to
> + the target in a single SRP_CMD.
s/direct/indirect/
> +What: /sys/class/scsi_host/host<n>/failed_reconnects
>+What: /sys/class/scsi_host/host<n>/reconnect_tmo
These attributes does not exist at this point in the patch series.
Perhaps this patch should be the last one in the series?
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 13/18] ib_srp: Allow SRP disconnect through sysfs
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (8 preceding siblings ...)
2012-01-14 12:51 ` [PATCH 12/18] ib_srp: Document sysfs attributes Bart Van Assche
@ 2012-01-14 12:52 ` Bart Van Assche
2012-02-26 6:33 ` David Dillow
2012-01-14 12:53 ` [PATCH 14/18] ib_srp: Move target port removal code Bart Van Assche
` (4 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:52 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA, dillowda-1Heg1YXhbW8,
roland-BHEL68pLQRGGvPXPguhicg,
fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg,
brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
Make 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.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Cc: FUJITA Tomonori <fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Cc: Brian King <brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
Documentation/ABI/stable/sysfs-transport-srp | 7 +++++++
drivers/infiniband/ulp/srp/ib_srp.c | 26 +++++++++++++++++++++++---
drivers/infiniband/ulp/srp/ib_srp.h | 1 +
drivers/scsi/scsi_transport_srp.c | 20 +++++++++++++++++++-
include/scsi/scsi_transport_srp.h | 10 ++++++++++
5 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-transport-srp b/Documentation/ABI/stable/sysfs-transport-srp
index 7b0d4a5..9b78ace 100644
--- a/Documentation/ABI/stable/sysfs-transport-srp
+++ b/Documentation/ABI/stable/sysfs-transport-srp
@@ -1,3 +1,10 @@
+What: /sys/class/srp_remote_ports/port-<h>:<n>/delete
+Date: January 1, 2012
+KernelVersion: 3.3
+Contact: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 67932aa..e8b699b 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -600,6 +600,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,10 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
return PTR_ERR(rport);
}
+ rport->ft = &ib_srp_transport_functions;
+ rport->lld_data = target;
+ target->rport = rport;
+
spin_lock(&host->target_lock);
list_add_tail(&target->list, &host->target_list);
spin_unlock(&host->target_lock);
@@ -2575,9 +2598,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;
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 3882adf..22e0c5d 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -154,6 +154,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;
diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index 0d85f79..35f85bc 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -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 3
struct srp_internal {
struct scsi_transport_template t;
@@ -116,6 +116,22 @@ 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);
+
+ if (rport->ft->rport_delete) {
+ rport->ft->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 +325,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));
diff --git a/include/scsi/scsi_transport_srp.h b/include/scsi/scsi_transport_srp.h
index 9c60ca1..1a109ff 100644
--- a/include/scsi/scsi_transport_srp.h
+++ b/include/scsi/scsi_transport_srp.h
@@ -14,13 +14,23 @@ struct srp_rport_identifiers {
};
struct srp_rport {
+ /* for initiator and target drivers */
+
+ struct srp_function_template *ft;
+
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);
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 13/18] ib_srp: Allow SRP disconnect through sysfs
2012-01-14 12:52 ` [PATCH 13/18] ib_srp: Allow SRP disconnect through sysfs Bart Van Assche
@ 2012-02-26 6:33 ` David Dillow
0 siblings, 0 replies; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:33 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-scsi-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg,
fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg,
brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
On Sat, 2012-01-14 at 12:52 +0000, Bart Van Assche wrote:
> Make 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.
>
> + rport->ft = &ib_srp_transport_functions;
The initiator should not be mucking around with internal functions of
the transport. Even if this is going to live in srp_rport rather than
srp_internal like other transports, this assignment should be done in
srp_rport_add().
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
> index 3882adf..22e0c5d 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.h
> +++ b/drivers/infiniband/ulp/srp/ib_srp.h
> @@ -154,6 +154,7 @@ struct srp_target_port {
> u16 io_class;
> struct srp_host *srp_host;
> struct Scsi_Host *scsi_host;
> + struct srp_rport *rport;
Why do we need to save this in our target? You don't use it anywhere in
this patch.
> --- a/drivers/scsi/scsi_transport_srp.c
> +++ b/drivers/scsi/scsi_transport_srp.c
> @@ -116,6 +116,22 @@ 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);
This should translate to the srp_internal and then use the function
template there instead of adding them to srp_rport.
struct Scsi_Host *shost = rport_to_shost(rport);
struct srp_internal *i = to_srp_internal(shost->transportt);
if (!i->f->rport_delete)
return -ENOSYS;
i->f->rport_delete(rport);
return count;
> diff --git a/include/scsi/scsi_transport_srp.h b/include/scsi/scsi_transport_srp.h
> index 9c60ca1..1a109ff 100644
> --- a/include/scsi/scsi_transport_srp.h
> +++ b/include/scsi/scsi_transport_srp.h
> @@ -14,13 +14,23 @@ struct srp_rport_identifiers {
> };
>
> struct srp_rport {
> + /* for initiator and target drivers */
> +
> + struct srp_function_template *ft;
This isn't needed; while perhaps more efficient, it breaks from the
practice used by other SCSI transports.
> struct device dev;
>
> u8 port_id[16];
> u8 roles;
> +
> + /* for initiator drivers */
> +
> + void *lld_data; /* LLD private data */
Other transports add a private data size to the transport's function
templates, and allocate space at the end of the rport. See
fc_function_template:dd_fcrport_size and fc_rport:dd_data.
Using this model will let use store our pointer there for now, and may
let us merge the rport allocation of the initiator with the transport's
allocation in the future.
Or it may just be complexity for no real gain; I can certainly see that
argument against it.
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 14/18] ib_srp: Move target port removal code
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (9 preceding siblings ...)
2012-01-14 12:52 ` [PATCH 13/18] ib_srp: Allow SRP disconnect through sysfs Bart Van Assche
@ 2012-01-14 12:53 ` Bart Van Assche
2012-01-14 12:54 ` [PATCH 15/18] ib_srp: Maintain a single connection per I_T nexus Bart Van Assche
` (3 subsequent siblings)
14 siblings, 0 replies; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:53 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
Move the code for removing a target port if reconnecting during
a reset triggered by the SCSI mid-layer fails from inside
srp_reconnect_target() into srp_reset_host().
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 34 ++++++++++++++++------------------
1 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index e8b699b..a8a3a2f 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -764,21 +764,7 @@ static int srp_reconnect_target(struct srp_target_port *target)
if (ret)
goto err;
- return ret;
-
err:
- shost_printk(KERN_ERR, target->scsi_host,
- PFX "reconnect failed (%d), removing target port.\n", ret);
-
- /*
- * We couldn't reconnect, so kill our target port off.
- * 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().
- */
- if (srp_change_state_to_removed(target))
- queue_work(ib_wq, &target->remove_work);
-
return ret;
}
@@ -1825,14 +1811,26 @@ static int srp_reset_device(struct scsi_cmnd *scmnd)
static int srp_reset_host(struct scsi_cmnd *scmnd)
{
struct srp_target_port *target = host_to_target(scmnd->device->host);
- int ret = FAILED;
+ int res;
shost_printk(KERN_ERR, target->scsi_host, PFX "SRP reset_host called\n");
- if (!srp_reconnect_target(target))
- ret = SUCCESS;
+ res = srp_reconnect_target(target);
+ if (res == 0)
+ return SUCCESS;
- return ret;
+ /*
+ * We couldn't reconnect, so kill our target port off. 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().
+ */
+ shost_printk(KERN_ERR, target->scsi_host,
+ PFX "reconnect failed (%d), removing target port.\n",
+ res);
+ if (srp_change_state_to_removed(target))
+ queue_work(system_long_wq, &target->remove_work);
+ return FAILED;
}
static int srp_slave_configure(struct scsi_device *sdev)
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 15/18] ib_srp: Maintain a single connection per I_T nexus
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (10 preceding siblings ...)
2012-01-14 12:53 ` [PATCH 14/18] ib_srp: Move target port removal code Bart Van Assche
@ 2012-01-14 12:54 ` Bart Van Assche
2012-02-26 6:34 ` David Dillow
2012-01-14 12:57 ` [PATCH 18/18] ib_srp: Rework error handling Bart Van Assche
` (2 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:54 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
The 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.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 97 +++++++++++++++++++++++++++--------
drivers/infiniband/ulp/srp/ib_srp.h | 10 ++++
2 files changed, 86 insertions(+), 21 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index a8a3a2f..eb31a14 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -574,16 +574,20 @@ 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);
+ if (target->scsi_host_added) {
+ srp_remove_host(shost);
+ scsi_remove_host(shost);
+ }
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);
+ scsi_host_put(shost);
}
static void srp_remove_work(struct work_struct *work)
@@ -615,6 +619,32 @@ 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.
+ *
+ * Connections are compared by their SCSI target port identifier: identifier
+ * extension + IO controller GUID. See also paragraph B.5 in the SRP spec.
+ */
+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) {
+ ret = false;
+ break;
+ }
+ }
+ spin_unlock(&host->target_lock);
+
+ return ret;
+}
+
static int srp_connect_target(struct srp_target_port *target)
{
int retries = 3;
@@ -728,9 +758,21 @@ static int srp_reconnect_target(struct srp_target_port *target)
{
struct ib_qp_attr qp_attr;
int i, ret;
+ bool unique;
- if (target->state != SRP_TARGET_LIVE)
+ unique = srp_conn_unique(target->srp_host, target);
+ if (unique) {
+ } else if (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;
+ }
srp_disconnect_target(target);
/*
@@ -2023,15 +2065,6 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
rport->lld_data = target;
target->rport = rport;
- 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;
-
- srp_scan_target(target);
-
return 0;
}
@@ -2317,6 +2350,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);
@@ -2363,24 +2397,45 @@ 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;
+ mutex_unlock(&target->mutex);
+
+ srp_scan_target(target);
+
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))
+ srp_remove_work(&target->remove_work);
+ return ret;
err_free_ib:
srp_free_target_ib(target);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 22e0c5d..a603c6d 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -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,
};
@@ -166,6 +174,7 @@ struct srp_target_port {
u32 rq_tmo_jiffies;
bool connected;
+ bool scsi_host_added;
struct ib_cm_id *cm_id;
@@ -186,6 +195,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;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 15/18] ib_srp: Maintain a single connection per I_T nexus
2012-01-14 12:54 ` [PATCH 15/18] ib_srp: Maintain a single connection per I_T nexus Bart Van Assche
@ 2012-02-26 6:34 ` David Dillow
[not found] ` <1330238040.1026.89.camel-1q1vX8mYZiGLUyTwlgNVppKKF0rrzTr+@public.gmane.org>
0 siblings, 1 reply; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:34 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg
On Sat, 2012-01-14 at 12:54 +0000, Bart Van Assche wrote:
> The 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.
I was OK before with preventing the addition of a duplicate connection,
but the initiator shouldn't be enforcing the removal of previous
connections -- that should happen via the target issuing a DREQ.
If you're going to disallow duplicate requests, then just disallow
adding the new one, don't tear down the old one -- let the admin
manually do that if desired.
> +/**
> + * srp_conn_unique() - Check whether the connection to a target is unique.
> + *
> + * Connections are compared by their SCSI target port identifier: identifier
> + * extension + IO controller GUID. See also paragraph B.5 in the SRP spec.
> + */
> +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) {
You also need to consider target->initiator_ext here -- that will change
the I_T nexus.
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 18/18] ib_srp: Rework error handling
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (11 preceding siblings ...)
2012-01-14 12:54 ` [PATCH 15/18] ib_srp: Maintain a single connection per I_T nexus Bart Van Assche
@ 2012-01-14 12:57 ` Bart Van Assche
2012-02-26 6:39 ` David Dillow
2012-01-14 22:10 ` [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes David Dillow
2012-02-06 16:16 ` Bart Van Assche
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-01-14 12:57 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg
Add fast_io_fail_tmo and dev_loss_tmo sysfs attributes. Block
the SCSI target as soon as a transport layer error has been
detected (ping timeout, disconnect or IB error completion). Try
to reconnect until dev_loss_tmo elapses.
Disconnect the IB connection earlier in srp_remove_target() to
make sure that error recovery is not triggered during host
removal. Swap the "connected" and "removed" tests in
srp_queuecommand() because of this change.
Rescan LUNs after having unblocked a SCSI target controlled by
ib_srp.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
Documentation/ABI/stable/sysfs-transport-srp | 35 ++++
drivers/infiniband/ulp/srp/ib_srp.c | 248 ++++++++++++++++++++++++--
drivers/infiniband/ulp/srp/ib_srp.h | 9 +
3 files changed, 280 insertions(+), 12 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-transport-srp b/Documentation/ABI/stable/sysfs-transport-srp
index 9b78ace..85d1eba 100644
--- a/Documentation/ABI/stable/sysfs-transport-srp
+++ b/Documentation/ABI/stable/sysfs-transport-srp
@@ -5,6 +5,41 @@ Contact: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
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.3
+Contact: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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.3
+Contact: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+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>/ping_interval
+Date: January 1, 2012
+KernelVersion: 3.3
+Contact: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: Time in seconds between two sucessive ping attempts. Setting
+ this parameter to zero or a negative value disables the ping
+ mechanism.
+
+What: /sys/class/srp_remote_ports/port-<h>:<n>/ping_timeout
+Date: January 1, 2012
+KernelVersion: 3.3
+Contact: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: If more time has elapsed than the specified number of seconds
+ since the latest successful ping attempt, the SRP initiator
+ driver that enabled this feature is informed about a transport
+ layer timeout by invoking its rport_ping_timedout callback
+ function.
+
What: /sys/class/srp_remote_ports/port-<h>:<n>/port_id
Date: June 27, 2007
KernelVersion: 2.6.24
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index eb31a14..36a55e0 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2005 Cisco Systems. All rights reserved.
+ * Copyright (c) 2010-2011 Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>.
*
* This software is available to you under a choice of one of two
* licenses. You may choose to be licensed under the terms of the GNU
@@ -446,7 +447,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,
@@ -511,7 +521,8 @@ static void srp_wait_last_send_wqe(struct srp_target_port *target)
WARN_ON(!target->last_send_wqe);
}
-static void srp_disconnect_target(struct srp_target_port *target)
+static void srp_disconnect_target(struct srp_target_port *target,
+ bool cancel_block_work)
{
struct ib_qp_attr qp_attr;
int ret;
@@ -537,6 +548,9 @@ static void srp_disconnect_target(struct srp_target_port *target)
srp_wait_last_send_wqe(target);
}
+
+ if (cancel_block_work)
+ cancel_work_sync(&target->block_work);
}
static void srp_free_req_data(struct srp_target_port *target)
@@ -572,18 +586,72 @@ static void srp_del_scsi_host_attr(struct Scsi_Host *shost)
device_remove_file(&shost->shost_dev, *attr);
}
+/**
+ * srp_disable_ping() - Stop pinging a target.
+ *
+ * Note: Can be invoked concurrently via the SCSI host sysfs attribute "delete"
+ * and one of the rport callback functions.
+ */
+static void srp_disable_ping(struct scsi_device *sdev)
+{
+ struct Scsi_Host *shost = sdev->host;
+ struct srp_target_port *target = host_to_target(shost);
+ struct srp_rport *rport = target->rport;
+
+ if (rport->sdev == sdev) {
+ shost_printk(KERN_DEBUG, target->scsi_host,
+ PFX "Disabled pinging\n");
+ srp_rport_set_sdev(rport, NULL);
+ srp_rport_disable_ping(rport);
+ }
+}
+
+/*
+ * srp_remove_target() - Remove an SRP target.
+ *
+ * The strategy to remove a target is as follows:
+ * - The caller must have set target->state to SRP_TARGET_REMOVED before
+ * invoking or queueing this function such that new calls to
+ * srp_disconnect_target(), srp_reconnect_target() or srp_block_work() do
+ * not have any effect.
+ * - Remove the sysfs attributes registered by ib_srp such that the registered
+ * sysfs callback functions won't be invoked anymore.
+ * - Disconnect the IB connection, wait until processing completions finished
+ * and cancel block_work.
+ * - Unblock the rport such that srp_stop_rport() doesn't deadlock.
+ * - Cancel any asynchronous work started by the SRP transport layer.
+ * - Invoke scsi_remove_host() such that all pending SCSI commands get killed.
+ * See e.g. sd_probe().
+ * - Cancel any asynchronous work started by ib_srp.
+ * - Tear down the IB resources associated with the target.
+ * - Invoke scsi_host_put() which will also free the target structure.
+ */
static void srp_remove_target(struct srp_target_port *target)
{
- struct Scsi_Host *shost = target->scsi_host;
+ struct Scsi_Host *shost;
+ struct srp_rport *rport;
WARN_ON(target->state != SRP_TARGET_REMOVED);
- srp_del_scsi_host_attr(shost);
+ /* Wait until any concurrent critical sections have finished. */
+ mutex_lock(&target->mutex);
+ mutex_unlock(&target->mutex);
+
+ shost = target->scsi_host;
+ rport = target->rport;
+ WARN_ON((rport != NULL) != target->scsi_host_added);
+
+ if (target->scsi_host_added)
+ srp_del_scsi_host_attr(shost);
+ srp_disconnect_target(target, true);
if (target->scsi_host_added) {
+ srp_resume_io(rport);
+ srp_stop_rport(rport);
srp_remove_host(shost);
scsi_remove_host(shost);
}
- srp_disconnect_target(target);
+ cancel_work_sync(&target->scan_work);
+ cancel_delayed_work_sync(&target->reconnect_work);
ib_destroy_cm_id(target->cm_id);
srp_free_target_ib(target);
srp_free_req_data(target);
@@ -645,6 +713,21 @@ static bool srp_conn_unique(struct srp_host *host,
return ret;
}
+static void srp_ping_timedout(struct srp_rport *rport)
+{
+ struct srp_target_port *target = rport->lld_data;
+
+ pr_debug("ping timeout: rport = %p; target = %p / state %d\n",
+ rport, target, target->state);
+
+ mutex_lock(&target->mutex);
+ if (srp_change_state(target, SRP_TARGET_LIVE, SRP_TARGET_BLOCKED)) {
+ srp_block_rport(rport);
+ srp_start_tl_fail_timers(target->rport);
+ }
+ mutex_unlock(&target->mutex);
+}
+
static int srp_connect_target(struct srp_target_port *target)
{
int retries = 3;
@@ -676,6 +759,7 @@ static int srp_connect_target(struct srp_target_port *target)
switch (target->status) {
case 0:
srp_change_conn_state(target, true);
+ target->failed_reconnects = 0;
return 0;
case SRP_PORT_REDIRECT:
@@ -754,7 +838,16 @@ static void srp_scan_target(struct srp_target_port *target)
SCAN_WILD_CARD, 0);
}
-static int srp_reconnect_target(struct srp_target_port *target)
+static void srp_scan_work(struct work_struct *work)
+{
+ struct srp_target_port *target;
+
+ target = container_of(work, struct srp_target_port, scan_work);
+ srp_scan_target(target);
+}
+
+static int srp_reconnect_target(struct srp_target_port *target,
+ bool cancel_block_work)
{
struct ib_qp_attr qp_attr;
int i, ret;
@@ -762,6 +855,11 @@ static int srp_reconnect_target(struct srp_target_port *target)
unique = srp_conn_unique(target->srp_host, target);
if (unique) {
+ mutex_lock(&target->mutex);
+ if (srp_change_state(target, SRP_TARGET_LIVE,
+ SRP_TARGET_BLOCKED))
+ srp_block_rport(target->rport);
+ mutex_unlock(&target->mutex);
} else if (srp_change_state_to_removed(target)) {
shost_printk(KERN_INFO, target->scsi_host,
PFX "deleting SCSI host because obsolete.\n");
@@ -774,7 +872,7 @@ static int srp_reconnect_target(struct srp_target_port *target)
return -EAGAIN;
}
- srp_disconnect_target(target);
+ srp_disconnect_target(target, cancel_block_work);
/*
* Now get a new local CM ID so that we avoid confusing the
* target in case things are really fouled up.
@@ -806,10 +904,63 @@ static int srp_reconnect_target(struct srp_target_port *target)
if (ret)
goto err;
+ mutex_lock(&target->mutex);
+ if (srp_change_state(target, SRP_TARGET_BLOCKED, SRP_TARGET_LIVE))
+ srp_resume_io(target->rport);
+ else
+ ret = -EAGAIN;
+ mutex_unlock(&target->mutex);
+
+ /*
+ * Since this code can be invoked from the context of the SCSI error
+ * handler, invoke SCSI scanning asynchronously.
+ */
+ if (ret == 0)
+ queue_work(system_long_wq, &target->scan_work);
+
err:
return ret;
}
+/**
+ * srp_reconnect_repeatedly() - Attempt to reconnect repeatedly.
+ *
+ * Return value: True if and only if the reconnect attempt hasn't succeeded
+ * and a subsequent reconnect attempt is scheduled. If the return value is
+ * true that also means that the target and the rport state have been changed
+ * from LIVE into BLOCKED.
+ */
+static void srp_reconnect_repeatedly(struct srp_target_port *target,
+ bool cancel_block_work)
+{
+ int res, tmo;
+
+ res = srp_reconnect_target(target, cancel_block_work);
+ if (res == 0)
+ return;
+
+ ++target->failed_reconnects;
+
+ shost_printk(KERN_ERR, target->scsi_host,
+ PFX "reconnect attempt %d failed (%d).\n",
+ target->failed_reconnects, res);
+
+ tmo = target->reconnect_tmo;
+ if (tmo > 0)
+ queue_delayed_work(system_long_wq, &target->reconnect_work,
+ tmo * HZ);
+}
+
+static void srp_reconnect_work(struct work_struct *work)
+{
+ struct srp_target_port *target;
+
+ target = container_of(to_delayed_work(work), struct srp_target_port,
+ reconnect_work);
+
+ srp_reconnect_repeatedly(target, true);
+}
+
static void srp_map_desc(struct srp_map_state *state, dma_addr_t dma_addr,
unsigned int dma_len, u32 rkey)
{
@@ -1343,15 +1494,30 @@ static void srp_handle_recv(struct srp_target_port *target, struct ib_wc *wc)
PFX "Recv failed with error code %d\n", res);
}
+/*
+ * Start the transport layer failure timers, block the SCSI host and try to
+ * reconnect.
+ */
+static void srp_block_work(struct work_struct *work)
+{
+ struct srp_target_port *target;
+
+ target = container_of(work, struct srp_target_port, block_work);
+ srp_start_tl_fail_timers(target->rport);
+ srp_reconnect_repeatedly(target, false);
+}
+
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->block_work);
+ }
target->qp_in_error = true;
}
@@ -1410,15 +1576,15 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
unsigned long flags;
int len;
- if (!target->connected)
- goto err;
-
if (target->state == SRP_TARGET_REMOVED) {
scmnd->result = DID_BAD_TARGET << 16;
scmnd->scsi_done(scmnd);
return 0;
}
+ if (!target->connected)
+ goto err;
+
spin_lock_irqsave(&target->lock, flags);
iu = __srp_get_tx_iu(target, SRP_IU_CMD);
if (!iu)
@@ -1730,6 +1896,7 @@ 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->block_work);
break;
case IB_CM_TIMEWAIT_EXIT:
@@ -1857,7 +2024,7 @@ static int srp_reset_host(struct scsi_cmnd *scmnd)
shost_printk(KERN_ERR, target->scsi_host, PFX "SRP reset_host called\n");
- res = srp_reconnect_target(target);
+ res = srp_reconnect_target(target, true);
if (res == 0)
return SUCCESS;
@@ -1879,6 +2046,7 @@ 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 srp_rport *rport = target->rport;
struct request_queue *q = sdev->request_queue;
unsigned long timeout;
@@ -1888,6 +2056,12 @@ static int srp_slave_configure(struct scsi_device *sdev)
blk_queue_rq_timeout(q, timeout);
}
+ if (!rport->sdev) {
+ shost_printk(KERN_DEBUG, target->scsi_host,
+ PFX "Enabled pinging\n");
+ srp_rport_set_sdev(rport, sdev);
+ }
+
return 0;
}
@@ -1990,6 +2164,45 @@ static ssize_t show_allow_ext_sg(struct device *dev,
return sprintf(buf, "%s\n", target->allow_ext_sg ? "true" : "false");
}
+static ssize_t show_reconnect_tmo(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct srp_target_port *target = host_to_target(class_to_shost(dev));
+
+ return sprintf(buf, "%d\n", target->reconnect_tmo);
+}
+
+static ssize_t store_reconnect_tmo(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, const size_t count)
+{
+ struct srp_target_port *target = host_to_target(class_to_shost(dev));
+ char ch[16];
+ int res, tmo;
+
+ sprintf(ch, "%.*s", (int)min(sizeof(ch) - 1, count), buf);
+ res = kstrtoint(ch, 0, &tmo);
+ if (res)
+ goto out;
+ target->reconnect_tmo = tmo;
+ if (tmo > 0 && target->state == SRP_TARGET_BLOCKED)
+ queue_delayed_work(system_long_wq, &target->reconnect_work,
+ tmo * HZ);
+ else if (tmo <= 0)
+ cancel_delayed_work(&target->reconnect_work);
+ res = count;
+out:
+ return res;
+}
+
+static ssize_t show_failed_reconnects(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct srp_target_port *target = host_to_target(class_to_shost(dev));
+
+ return sprintf(buf, "%d\n", target->failed_reconnects);
+}
+
static DEVICE_ATTR(id_ext, S_IRUGO, show_id_ext, NULL);
static DEVICE_ATTR(ioc_guid, S_IRUGO, show_ioc_guid, NULL);
static DEVICE_ATTR(service_id, S_IRUGO, show_service_id, NULL);
@@ -2002,6 +2215,9 @@ static DEVICE_ATTR(local_ib_port, S_IRUGO, show_local_ib_port, NULL);
static DEVICE_ATTR(local_ib_device, S_IRUGO, show_local_ib_device, NULL);
static DEVICE_ATTR(cmd_sg_entries, S_IRUGO, show_cmd_sg_entries, NULL);
static DEVICE_ATTR(allow_ext_sg, S_IRUGO, show_allow_ext_sg, NULL);
+static DEVICE_ATTR(reconnect_tmo, S_IRUGO | S_IWUSR, show_reconnect_tmo,
+ store_reconnect_tmo);
+static DEVICE_ATTR(failed_reconnects, S_IRUGO, show_failed_reconnects, NULL);
static struct device_attribute *srp_host_attrs[] = {
&dev_attr_id_ext,
@@ -2016,6 +2232,8 @@ static struct device_attribute *srp_host_attrs[] = {
&dev_attr_local_ib_device,
&dev_attr_cmd_sg_entries,
&dev_attr_allow_ext_sg,
+ &dev_attr_reconnect_tmo,
+ &dev_attr_failed_reconnects,
NULL
};
@@ -2024,6 +2242,7 @@ static struct scsi_host_template srp_template = {
.name = "InfiniBand SRP initiator",
.proc_name = DRV_NAME,
.slave_configure = srp_slave_configure,
+ .slave_delete = srp_disable_ping,
.info = srp_target_info,
.queuecommand = srp_queuecommand,
.eh_abort_handler = srp_abort,
@@ -2038,6 +2257,7 @@ static struct scsi_host_template srp_template = {
};
static struct srp_function_template ib_srp_transport_functions = {
+ .rport_ping_timedout = srp_ping_timedout,
.rport_delete = srp_rport_delete,
};
@@ -2351,7 +2571,11 @@ 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->block_work, srp_block_work);
INIT_WORK(&target->remove_work, srp_remove_work);
+ INIT_WORK(&target->scan_work, srp_scan_work);
+ INIT_DELAYED_WORK(&target->reconnect_work, srp_reconnect_work);
+ target->reconnect_tmo = 10;
spin_lock_init(&target->lock);
INIT_LIST_HEAD(&target->free_tx);
INIT_LIST_HEAD(&target->free_reqs);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index a603c6d..e76000e 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -83,11 +83,14 @@ enum {
* @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_BLOCKED: An IB RC error occurred. Recovery timer may be running.
+ * SCSI host is blocked.
* @SRP_TARGET_REMOVED: SCSI host removal is pending.
*/
enum srp_target_state {
SRP_TARGET_CONNECTING,
SRP_TARGET_LIVE,
+ SRP_TARGET_BLOCKED,
SRP_TARGET_REMOVED,
};
@@ -186,7 +189,13 @@ struct srp_target_port {
struct srp_iu *rx_ring[SRP_RQ_SIZE];
struct srp_request req_ring[SRP_CMD_SQ_SIZE];
+ struct work_struct block_work;
struct work_struct remove_work;
+ struct work_struct scan_work;
+
+ int reconnect_tmo;
+ int failed_reconnects;
+ struct delayed_work reconnect_work;
struct list_head list;
struct completion done;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 18/18] ib_srp: Rework error handling
2012-01-14 12:57 ` [PATCH 18/18] ib_srp: Rework error handling Bart Van Assche
@ 2012-02-26 6:39 ` David Dillow
[not found] ` <1330238354.1026.93.camel-1q1vX8mYZiGLUyTwlgNVppKKF0rrzTr+@public.gmane.org>
0 siblings, 1 reply; 45+ messages in thread
From: David Dillow @ 2012-02-26 6:39 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg
On Sat, 2012-01-14 at 12:57 +0000, Bart Van Assche wrote:
> Add fast_io_fail_tmo and dev_loss_tmo sysfs attributes. Block
> the SCSI target as soon as a transport layer error has been
> detected (ping timeout, disconnect or IB error completion). Try
> to reconnect until dev_loss_tmo elapses.
>
> Disconnect the IB connection earlier in srp_remove_target() to
> make sure that error recovery is not triggered during host
> removal. Swap the "connected" and "removed" tests in
> srp_queuecommand() because of this change.
>
> Rescan LUNs after having unblocked a SCSI target controlled by
> ib_srp.
The inclusion of the ping gets 16..18 a NAK from me. As I've previously
said, this is better handled by user space and you add additional
failure cases when you try to retrofit a transport ping into SRP. Just
don't do it.
I'll try to parse through the rest of this soon to give you feedback on
the conversion to dev_loss_tmo etc.
--
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (12 preceding siblings ...)
2012-01-14 12:57 ` [PATCH 18/18] ib_srp: Rework error handling Bart Van Assche
@ 2012-01-14 22:10 ` David Dillow
[not found] ` <1326579013.8227.4.camel-1q1vX8mYZiGLUyTwlgNVppKKF0rrzTr+@public.gmane.org>
2012-02-06 16:16 ` Bart Van Assche
14 siblings, 1 reply; 45+ messages in thread
From: David Dillow @ 2012-01-14 22:10 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-rdma, linux-scsi, Roland Dreier, Vu Pham
On Sat, 2012-01-14 at 12:36 +0000, Bart Van Assche wrote:
> This patch series makes the ib_srp driver better suited for use in a H.A.
What kernel version is this based on?
Patch 17 (srp_transport: Add transport layer recovery support) doesn't
apply with 'git am'. I haven't investigated why yet, so it may be quite
simple.
--
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes
2012-01-14 12:36 [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes Bart Van Assche
` (13 preceding siblings ...)
2012-01-14 22:10 ` [PATCH 00/18, v2] Make ib_srp better suited for H.A. purposes David Dillow
@ 2012-02-06 16:16 ` Bart Van Assche
[not found] ` <CAO+b5-q7q+-spucP821tpmQW5Qp7GXg+kTyL9TxesA32hAVbFA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
14 siblings, 1 reply; 45+ messages in thread
From: Bart Van Assche @ 2012-02-06 16:16 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow
On Sat, Jan 14, 2012 at 1:36 PM, Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> This patch series makes the ib_srp driver better suited for use in a H.A.
Hi Dave,
Do you have any review comments about this patch series ?
Thanks,
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread