public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] SRP initiator patches for kernel 3.15
@ 2014-03-10 11:39 Bart Van Assche
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2014-03-10 11:39 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

Hello Roland,

Please consider this patch series for kernel 3.15.

Changes compared to v1:
- Added a patch from Sagi Grimberg ("Check ib_query_gid() return
  value").
- Left out a patch to give Dave Dillow a chance to rework it as a SCSI
  core patch ("IB/srp: Fail SCSI commands silently").

These patches are also available at
https://github.com/bvanassche/linux.git; branch srp-initiator. That git
tree is based on
git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git;
branch for-next.

This patch series includes the following seven patches:
0001-scsi_transport_srp-Fix-two-kernel-doc-warnings.patch
0002-IB-srp-Check-ib_query_gid-return-value.patch
0003-IB-srp-Add-more-logging.patch
0004-IB-srp-Avoid-duplicate-connections.patch
0005-IB-srp-Make-writing-into-the-add_target-sysfs-attrib.patch
0006-IB-srp-Avoid-that-writing-into-add_target-hangs-due-.patch
0007-IB-srp-Fix-a-race-condition-between-failing-I-O-and-.patch

--
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] 16+ messages in thread

* [PATCH v2 1/7] scsi_transport_srp: Fix two kernel-doc warnings
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
@ 2014-03-10 11:40   ` Bart Van Assche
  2014-03-10 11:42   ` [PATCH v2 2/7] IB/srp: Check ib_query_gid return value Bart Van Assche
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-10 11:40 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

This patch fixes the following two kernel-doc warnings:

Warning(drivers/scsi/scsi_transport_srp.c:819): No description found for parameter 'rport'
Warning(include/scsi/scsi_transport_srp.h:75): Excess struct/union/enum/typedef member 'deleted' description in 'srp_rport'

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Reported-by: Masanari Iida <standby24x7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: James Bottomley <jbottomley-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/scsi/scsi_transport_srp.c | 1 +
 include/scsi/scsi_transport_srp.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index d47ffc8..13e8983 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -810,6 +810,7 @@ EXPORT_SYMBOL_GPL(srp_remove_host);
 
 /**
  * srp_stop_rport_timers - stop the transport layer recovery timers
+ * @rport: SRP remote port for which to stop the timers.
  *
  * Must be called after srp_remove_host() and scsi_remove_host(). The caller
  * must hold a reference on the rport (rport->dev) and on the SCSI host
diff --git a/include/scsi/scsi_transport_srp.h b/include/scsi/scsi_transport_srp.h
index b11da5c..cdb05dd 100644
--- a/include/scsi/scsi_transport_srp.h
+++ b/include/scsi/scsi_transport_srp.h
@@ -41,7 +41,6 @@ enum srp_rport_state {
  * @mutex:             Protects against concurrent rport reconnect /
  *                     fast_io_fail / dev_loss_tmo activity.
  * @state:             rport state.
- * @deleted:           Whether or not srp_rport_del() has already been invoked.
  * @reconnect_delay:   Reconnect delay in seconds.
  * @failed_reconnects: Number of failed reconnect attempts.
  * @reconnect_work:    Work structure used for scheduling reconnect attempts.
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v2 2/7] IB/srp: Check ib_query_gid return value
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
  2014-03-10 11:40   ` [PATCH v2 1/7] scsi_transport_srp: Fix two kernel-doc warnings Bart Van Assche
@ 2014-03-10 11:42   ` Bart Van Assche
  2014-03-10 11:43   ` [PATCH v2 3/7] IB/srp: Add more logging Bart Van Assche
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-10 11:42 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	linux-rdma

From: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Detected by Coverity.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 529b6bc..8903226 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2649,7 +2649,9 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_free_mem;
 
-	ib_query_gid(ibdev, host->port, 0, &target->path.sgid);
+	ret = ib_query_gid(ibdev, host->port, 0, &target->path.sgid);
+	if (ret)
+		goto err_free_mem;
 
 	shost_printk(KERN_DEBUG, target->scsi_host, PFX
 		     "new target: id_ext %016llx ioc_guid %016llx pkey %04x "
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v2 3/7] IB/srp: Add more logging
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
  2014-03-10 11:40   ` [PATCH v2 1/7] scsi_transport_srp: Fix two kernel-doc warnings Bart Van Assche
  2014-03-10 11:42   ` [PATCH v2 2/7] IB/srp: Check ib_query_gid return value Bart Van Assche
@ 2014-03-10 11:43   ` Bart Van Assche
  2014-03-10 11:44   ` [PATCH v2 4/7] IB/srp: Avoid duplicate connections Bart Van Assche
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-10 11:43 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	linux-rdma

Log sgid and dgid when reporting that a login has been rejected or when
a host has been added. This makes it easy to figure out which initiator
and target ports these messages apply to.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 8903226..2ec9c05 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1804,8 +1804,10 @@ static void srp_cm_rej_handler(struct ib_cm_id *cm_id,
 				shost_printk(KERN_WARNING, shost,
 					     PFX "SRP_LOGIN_REJ: requested max_it_iu_len too large\n");
 			else
-				shost_printk(KERN_WARNING, shost,
-					    PFX "SRP LOGIN REJECTED, reason 0x%08x\n", reason);
+				shost_printk(KERN_WARNING, shost, PFX
+					     "SRP LOGIN from %pI6 to %pI6 REJECTED, reason 0x%08x\n",
+					     target->path.sgid.raw,
+					     target->orig_dgid, reason);
 		} else
 			shost_printk(KERN_WARNING, shost,
 				     "  REJ reason: IB_CM_REJ_CONSUMER_DEFINED,"
@@ -2653,15 +2655,6 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_free_mem;
 
-	shost_printk(KERN_DEBUG, target->scsi_host, PFX
-		     "new target: id_ext %016llx ioc_guid %016llx pkey %04x "
-		     "service_id %016llx dgid %pI6\n",
-	       (unsigned long long) be64_to_cpu(target->id_ext),
-	       (unsigned long long) be64_to_cpu(target->ioc_guid),
-	       be16_to_cpu(target->path.pkey),
-	       (unsigned long long) be64_to_cpu(target->service_id),
-	       target->path.dgid.raw);
-
 	ret = srp_create_target_ib(target);
 	if (ret)
 		goto err_free_mem;
@@ -2681,6 +2674,14 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_disconnect;
 
+	shost_printk(KERN_DEBUG, target->scsi_host, PFX
+		     "new target: id_ext %016llx ioc_guid %016llx pkey %04x service_id %016llx sgid %pI6 dgid %pI6\n",
+		     be64_to_cpu(target->id_ext),
+		     be64_to_cpu(target->ioc_guid),
+		     be16_to_cpu(target->path.pkey),
+		     be64_to_cpu(target->service_id),
+		     target->path.sgid.raw, target->path.dgid.raw);
+
 	return count;
 
 err_disconnect:
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v2 4/7] IB/srp: Avoid duplicate connections
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
                     ` (2 preceding siblings ...)
  2014-03-10 11:43   ` [PATCH v2 3/7] IB/srp: Add more logging Bart Van Assche
@ 2014-03-10 11:44   ` Bart Van Assche
  2014-03-10 11:45   ` [PATCH v2 5/7] IB/srp: Make writing into the "add_target" sysfs attribute interruptible Bart Van Assche
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-10 11:44 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	linux-rdma

The connection uniqueness check is performed before a new connection
is added to the target list. This patch protects both actions by a
mutex such that simultaneous writes from two different threads into the
"add_target" variable do not result in duplicate connections.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 12 +++++++++---
 drivers/infiniband/ulp/srp/ib_srp.h |  1 +
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 2ec9c05..3294f10 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2614,6 +2614,8 @@ static ssize_t srp_create_target(struct device *dev,
 	target->tl_retry_count	= 7;
 	target->queue_size	= SRP_DEFAULT_QUEUE_SIZE;
 
+	mutex_lock(&host->add_target_mutex);
+
 	ret = srp_parse_options(buf, target);
 	if (ret)
 		goto err;
@@ -2682,7 +2684,11 @@ static ssize_t srp_create_target(struct device *dev,
 		     be64_to_cpu(target->service_id),
 		     target->path.sgid.raw, target->path.dgid.raw);
 
-	return count;
+	ret = count;
+
+out:
+	mutex_unlock(&host->add_target_mutex);
+	return ret;
 
 err_disconnect:
 	srp_disconnect_target(target);
@@ -2698,8 +2704,7 @@ err_free_mem:
 
 err:
 	scsi_host_put(target_host);
-
-	return ret;
+	goto out;
 }
 
 static DEVICE_ATTR(add_target, S_IWUSR, NULL, srp_create_target);
@@ -2735,6 +2740,7 @@ static struct srp_host *srp_add_port(struct srp_device *device, u8 port)
 	INIT_LIST_HEAD(&host->target_list);
 	spin_lock_init(&host->target_lock);
 	init_completion(&host->released);
+	mutex_init(&host->add_target_mutex);
 	host->srp_dev = device;
 	host->port = port;
 
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 5756810..aad27b7 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -105,6 +105,7 @@ struct srp_host {
 	spinlock_t		target_lock;
 	struct completion	released;
 	struct list_head	list;
+	struct mutex		add_target_mutex;
 };
 
 struct srp_request {
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v2 5/7] IB/srp: Make writing into the "add_target" sysfs attribute interruptible
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
                     ` (3 preceding siblings ...)
  2014-03-10 11:44   ` [PATCH v2 4/7] IB/srp: Avoid duplicate connections Bart Van Assche
@ 2014-03-10 11:45   ` Bart Van Assche
  2014-03-10 11:46   ` [PATCH v2 6/7] IB/srp: Avoid that writing into "add_target" hangs due to a cable pull Bart Van Assche
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-10 11:45 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	linux-rdma

Avoid that stopping srp_daemon takes unusually long due to a cable
pull by making writing into the "add_target" sysfs attribute
interruptible.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 3294f10..481c873 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -411,6 +411,8 @@ static void srp_path_rec_completion(int status,
 
 static int srp_lookup_path(struct srp_target_port *target)
 {
+	int ret;
+
 	target->path.numb_path = 1;
 
 	init_completion(&target->done);
@@ -431,7 +433,9 @@ static int srp_lookup_path(struct srp_target_port *target)
 	if (target->path_query_id < 0)
 		return target->path_query_id;
 
-	wait_for_completion(&target->done);
+	ret = wait_for_completion_interruptible(&target->done);
+	if (ret < 0)
+		return ret;
 
 	if (target->status < 0)
 		shost_printk(KERN_WARNING, target->scsi_host,
@@ -710,7 +714,9 @@ static int srp_connect_target(struct srp_target_port *target)
 		ret = srp_send_req(target);
 		if (ret)
 			return ret;
-		wait_for_completion(&target->done);
+		ret = wait_for_completion_interruptible(&target->done);
+		if (ret < 0)
+			return ret;
 
 		/*
 		 * The CM event handling code will set status to
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v2 6/7] IB/srp: Avoid that writing into "add_target" hangs due to a cable pull
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
                     ` (4 preceding siblings ...)
  2014-03-10 11:45   ` [PATCH v2 5/7] IB/srp: Make writing into the "add_target" sysfs attribute interruptible Bart Van Assche
@ 2014-03-10 11:46   ` Bart Van Assche
  2014-03-10 11:47   ` [PATCH v2 7/7] IB/srp: Fix a race condition between failing I/O and I/O queueing Bart Van Assche
  2014-03-14 12:50   ` [PATCH v3 0/7] SRP initiator patches for kernel 3.15 Bart Van Assche
  7 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-10 11:46 UTC (permalink / raw)
  To: Bart Van Assche, Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	linux-rdma

If a cable is pulled while srp_connect_target() is in progress
that can result in that function never to return. That makes the
process, e.g. srp_daemon, that invoked this function unkillable.
Avoid this by letting srp_connect_target() finish if the event
IB_CM_TIMEWAIT_EXIT is received. This patch fixes a hang with the
following call trace:
 [<ffffffff814eae85>] schedule_timeout+0x215/0x2e0
 [<ffffffff814eab03>] wait_for_common+0x123/0x180
 [<ffffffff814eac1d>] wait_for_completion+0x1d/0x20
 [<ffffffffa03b398c>] srp_connect_target+0x1dc/0x410 [ib_srp]
 [<ffffffffa03b5809>] srp_create_target+0xba9/0xe70 [ib_srp]
 [<ffffffff8133e590>] dev_attr_store+0x20/0x30
 [<ffffffff811eb8f5>] sysfs_write_file+0xe5/0x170
 [<ffffffff811767c8>] vfs_write+0xb8/0x1a0
 [<ffffffff811770c1>] sys_write+0x51/0x90
 [<ffffffff8100b072>] system_call_fastpath+0x16/0x1b

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 481c873..a64e469 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1871,6 +1871,7 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 	case IB_CM_TIMEWAIT_EXIT:
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "connection closed\n");
+		comp = 1;
 
 		target->status = 0;
 		break;
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v2 7/7] IB/srp: Fix a race condition between failing I/O and I/O queueing
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
                     ` (5 preceding siblings ...)
  2014-03-10 11:46   ` [PATCH v2 6/7] IB/srp: Avoid that writing into "add_target" hangs due to a cable pull Bart Van Assche
@ 2014-03-10 11:47   ` Bart Van Assche
  2014-03-14 12:50   ` [PATCH v3 0/7] SRP initiator patches for kernel 3.15 Bart Van Assche
  7 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-10 11:47 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	linux-rdma

Avoid that srp_unmap_data() can get invoked concurrently by
srp_terminate_io() and from the error path in srp_queuecommand()
if srp_post_send() fails.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 46 +++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index a64e469..c7091ef 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -783,6 +783,7 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd,
  * srp_claim_req - Take ownership of the scmnd associated with a request.
  * @target: SRP target port.
  * @req: SRP request.
+ * @sdev: If not NULL, only take ownership for this SCSI device.
  * @scmnd: If NULL, take ownership of @req->scmnd. If not NULL, only take
  *         ownership of @req->scmnd if it equals @scmnd.
  *
@@ -791,16 +792,17 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd,
  */
 static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target,
 				       struct srp_request *req,
+				       struct scsi_device *sdev,
 				       struct scsi_cmnd *scmnd)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&target->lock, flags);
-	if (!scmnd) {
+	if (req->scmnd &&
+	    (!sdev || req->scmnd->device == sdev) &&
+	    (!scmnd || req->scmnd == scmnd)) {
 		scmnd = req->scmnd;
 		req->scmnd = NULL;
-	} else if (req->scmnd == scmnd) {
-		req->scmnd = NULL;
 	} else {
 		scmnd = NULL;
 	}
@@ -827,9 +829,10 @@ static void srp_free_req(struct srp_target_port *target,
 }
 
 static void srp_finish_req(struct srp_target_port *target,
-			   struct srp_request *req, int result)
+			   struct srp_request *req, struct scsi_device *sdev,
+			   int result)
 {
-	struct scsi_cmnd *scmnd = srp_claim_req(target, req, NULL);
+	struct scsi_cmnd *scmnd = srp_claim_req(target, req, sdev, NULL);
 
 	if (scmnd) {
 		srp_free_req(target, req, scmnd, 0);
@@ -845,7 +848,7 @@ static void srp_terminate_io(struct srp_rport *rport)
 
 	for (i = 0; i < target->req_ring_size; ++i) {
 		struct srp_request *req = &target->req_ring[i];
-		srp_finish_req(target, req, DID_TRANSPORT_FAILFAST << 16);
+		srp_finish_req(target, req, NULL, DID_TRANSPORT_FAILFAST << 16);
 	}
 }
 
@@ -882,7 +885,7 @@ static int srp_rport_reconnect(struct srp_rport *rport)
 
 	for (i = 0; i < target->req_ring_size; ++i) {
 		struct srp_request *req = &target->req_ring[i];
-		srp_finish_req(target, req, DID_RESET << 16);
+		srp_finish_req(target, req, NULL, DID_RESET << 16);
 	}
 
 	INIT_LIST_HEAD(&target->free_tx);
@@ -1290,7 +1293,7 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)
 		complete(&target->tsk_mgmt_done);
 	} else {
 		req = &target->req_ring[rsp->tag];
-		scmnd = srp_claim_req(target, req, NULL);
+		scmnd = srp_claim_req(target, req, NULL, NULL);
 		if (!scmnd) {
 			shost_printk(KERN_ERR, target->scsi_host,
 				     "Null scmnd for RSP w/tag %016llx\n",
@@ -1509,7 +1512,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	struct srp_cmd *cmd;
 	struct ib_device *dev;
 	unsigned long flags;
-	int len, result;
+	int len, result, ret = 0;
 	const bool in_scsi_eh = !in_interrupt() && current == shost->ehandler;
 
 	/*
@@ -1552,8 +1555,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	cmd->tag    = req->index;
 	memcpy(cmd->cdb, scmnd->cmnd, scmnd->cmd_len);
 
-	req->scmnd    = scmnd;
-	req->cmd      = iu;
+	req->cmd = iu;
 
 	len = srp_map_data(scmnd, target, req);
 	if (len < 0) {
@@ -1565,7 +1567,14 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	ib_dma_sync_single_for_device(dev, iu->dma, target->max_iu_len,
 				      DMA_TO_DEVICE);
 
-	if (srp_post_send(target, iu, len)) {
+	spin_lock_irqsave(&target->lock, flags);
+	if (srp_post_send(target, iu, len) == 0)
+		req->scmnd = scmnd;
+	else
+		ret = SCSI_MLQUEUE_HOST_BUSY;
+	spin_unlock_irqrestore(&target->lock, flags);
+
+	if (ret) {
 		shost_printk(KERN_ERR, target->scsi_host, PFX "Send failed\n");
 		goto err_unmap;
 	}
@@ -1574,7 +1583,7 @@ unlock_rport:
 	if (in_scsi_eh)
 		mutex_unlock(&rport->mutex);
 
-	return 0;
+	return ret;
 
 err_unmap:
 	srp_unmap_data(scmnd, target, req);
@@ -1588,10 +1597,8 @@ err_iu:
 err_unlock:
 	spin_unlock_irqrestore(&target->lock, flags);
 
-	if (in_scsi_eh)
-		mutex_unlock(&rport->mutex);
-
-	return SCSI_MLQUEUE_HOST_BUSY;
+	ret = SCSI_MLQUEUE_HOST_BUSY;
+	goto unlock_rport;
 }
 
 /*
@@ -2008,7 +2015,7 @@ static int srp_abort(struct scsi_cmnd *scmnd)
 
 	shost_printk(KERN_ERR, target->scsi_host, "SRP abort called\n");
 
-	if (!req || !srp_claim_req(target, req, scmnd))
+	if (!req || !srp_claim_req(target, req, NULL, scmnd))
 		return SUCCESS;
 	if (srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
 			      SRP_TSK_ABORT_TASK) == 0)
@@ -2039,8 +2046,7 @@ static int srp_reset_device(struct scsi_cmnd *scmnd)
 
 	for (i = 0; i < target->req_ring_size; ++i) {
 		struct srp_request *req = &target->req_ring[i];
-		if (req->scmnd && req->scmnd->device == scmnd->device)
-			srp_finish_req(target, req, DID_RESET << 16);
+		srp_finish_req(target, req, scmnd->device, DID_RESET << 16);
 	}
 
 	return SUCCESS;
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v3 0/7] SRP initiator patches for kernel 3.15
       [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
                     ` (6 preceding siblings ...)
  2014-03-10 11:47   ` [PATCH v2 7/7] IB/srp: Fix a race condition between failing I/O and I/O queueing Bart Van Assche
@ 2014-03-14 12:50   ` Bart Van Assche
       [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
  7 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2014-03-14 12:50 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

Hello Roland,

This is version three of a series with small changes for the SRP
initiator. Please consider this patch series for kernel 3.15.

Changes compared to v2:
- Reworked the locking around srp_post_send() again in patch 7/7.

Changes compared to v1:
- Added a patch from Sagi Grimberg ("Check ib_query_gid() return
  value").
- Left out a patch to give Dave Dillow a chance to rework it as a SCSI
  core patch ("IB/srp: Fail SCSI commands silently").

These patches are also available at
https://github.com/bvanassche/linux.git; branch srp-initiator; (commit
ID e0e1adbcb68f). That git tree is based on
git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git;
branch for-next.

This patch series includes the following seven patches:
0001-scsi_transport_srp-Fix-two-kernel-doc-warnings.patch
0002-IB-srp-Check-ib_query_gid-return-value.patch
0003-IB-srp-Add-more-logging.patch
0004-IB-srp-Avoid-duplicate-connections.patch
0005-IB-srp-Make-writing-into-the-add_target-sysfs-attrib.patch
0006-IB-srp-Avoid-that-writing-into-add_target-hangs-due-.patch
0007-IB-srp-Fix-a-race-condition-between-failing-I-O-and-.patch



--
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] 16+ messages in thread

* [PATCH v3 1/7] scsi_transport_srp: Fix two kernel-doc warnings
       [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
@ 2014-03-14 12:51       ` Bart Van Assche
  2014-03-14 12:51       ` [PATCH v3 2/7] IB/srp: Check ib_query_gid return value Bart Van Assche
                         ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-14 12:51 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

This patch fixes the following two kernel-doc warnings:

Warning(drivers/scsi/scsi_transport_srp.c:819): No description found for parameter 'rport'
Warning(include/scsi/scsi_transport_srp.h:75): Excess struct/union/enum/typedef member 'deleted' description in 'srp_rport'

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Reported-by: Masanari Iida <standby24x7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: James Bottomley <jbottomley-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/scsi/scsi_transport_srp.c | 1 +
 include/scsi/scsi_transport_srp.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index d47ffc8..13e8983 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -810,6 +810,7 @@ EXPORT_SYMBOL_GPL(srp_remove_host);
 
 /**
  * srp_stop_rport_timers - stop the transport layer recovery timers
+ * @rport: SRP remote port for which to stop the timers.
  *
  * Must be called after srp_remove_host() and scsi_remove_host(). The caller
  * must hold a reference on the rport (rport->dev) and on the SCSI host
diff --git a/include/scsi/scsi_transport_srp.h b/include/scsi/scsi_transport_srp.h
index b11da5c..cdb05dd 100644
--- a/include/scsi/scsi_transport_srp.h
+++ b/include/scsi/scsi_transport_srp.h
@@ -41,7 +41,6 @@ enum srp_rport_state {
  * @mutex:             Protects against concurrent rport reconnect /
  *                     fast_io_fail / dev_loss_tmo activity.
  * @state:             rport state.
- * @deleted:           Whether or not srp_rport_del() has already been invoked.
  * @reconnect_delay:   Reconnect delay in seconds.
  * @failed_reconnects: Number of failed reconnect attempts.
  * @reconnect_work:    Work structure used for scheduling reconnect attempts.
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v3 2/7] IB/srp: Check ib_query_gid return value
       [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
  2014-03-14 12:51       ` [PATCH v3 1/7] scsi_transport_srp: Fix two kernel-doc warnings Bart Van Assche
@ 2014-03-14 12:51       ` Bart Van Assche
  2014-03-14 12:52       ` [PATCH v3 3/7] IB/srp: Add more logging Bart Van Assche
                         ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-14 12:51 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

From: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Detected by Coverity.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 529b6bc..8903226 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2649,7 +2649,9 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_free_mem;
 
-	ib_query_gid(ibdev, host->port, 0, &target->path.sgid);
+	ret = ib_query_gid(ibdev, host->port, 0, &target->path.sgid);
+	if (ret)
+		goto err_free_mem;
 
 	shost_printk(KERN_DEBUG, target->scsi_host, PFX
 		     "new target: id_ext %016llx ioc_guid %016llx pkey %04x "
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v3 3/7] IB/srp: Add more logging
       [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
  2014-03-14 12:51       ` [PATCH v3 1/7] scsi_transport_srp: Fix two kernel-doc warnings Bart Van Assche
  2014-03-14 12:51       ` [PATCH v3 2/7] IB/srp: Check ib_query_gid return value Bart Van Assche
@ 2014-03-14 12:52       ` Bart Van Assche
  2014-03-14 12:52       ` [PATCH v3 4/7] IB/srp: Avoid duplicate connections Bart Van Assche
                         ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-14 12:52 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

Log sgid and dgid when reporting that a login has been rejected or when
a host has been added. This makes it easy to figure out which initiator
and target ports these messages apply to.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 8903226..2ec9c05 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1804,8 +1804,10 @@ static void srp_cm_rej_handler(struct ib_cm_id *cm_id,
 				shost_printk(KERN_WARNING, shost,
 					     PFX "SRP_LOGIN_REJ: requested max_it_iu_len too large\n");
 			else
-				shost_printk(KERN_WARNING, shost,
-					    PFX "SRP LOGIN REJECTED, reason 0x%08x\n", reason);
+				shost_printk(KERN_WARNING, shost, PFX
+					     "SRP LOGIN from %pI6 to %pI6 REJECTED, reason 0x%08x\n",
+					     target->path.sgid.raw,
+					     target->orig_dgid, reason);
 		} else
 			shost_printk(KERN_WARNING, shost,
 				     "  REJ reason: IB_CM_REJ_CONSUMER_DEFINED,"
@@ -2653,15 +2655,6 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_free_mem;
 
-	shost_printk(KERN_DEBUG, target->scsi_host, PFX
-		     "new target: id_ext %016llx ioc_guid %016llx pkey %04x "
-		     "service_id %016llx dgid %pI6\n",
-	       (unsigned long long) be64_to_cpu(target->id_ext),
-	       (unsigned long long) be64_to_cpu(target->ioc_guid),
-	       be16_to_cpu(target->path.pkey),
-	       (unsigned long long) be64_to_cpu(target->service_id),
-	       target->path.dgid.raw);
-
 	ret = srp_create_target_ib(target);
 	if (ret)
 		goto err_free_mem;
@@ -2681,6 +2674,14 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_disconnect;
 
+	shost_printk(KERN_DEBUG, target->scsi_host, PFX
+		     "new target: id_ext %016llx ioc_guid %016llx pkey %04x service_id %016llx sgid %pI6 dgid %pI6\n",
+		     be64_to_cpu(target->id_ext),
+		     be64_to_cpu(target->ioc_guid),
+		     be16_to_cpu(target->path.pkey),
+		     be64_to_cpu(target->service_id),
+		     target->path.sgid.raw, target->path.dgid.raw);
+
 	return count;
 
 err_disconnect:
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v3 4/7] IB/srp: Avoid duplicate connections
       [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
                         ` (2 preceding siblings ...)
  2014-03-14 12:52       ` [PATCH v3 3/7] IB/srp: Add more logging Bart Van Assche
@ 2014-03-14 12:52       ` Bart Van Assche
  2014-03-14 12:53       ` [PATCH v3 5/7] IB/srp: Make writing into the "add_target" sysfs attribute interruptible Bart Van Assche
                         ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-14 12:52 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

The connection uniqueness check is performed before a new connection
is added to the target list. This patch protects both actions by a
mutex such that simultaneous writes from two different threads into the
"add_target" variable do not result in duplicate connections.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 12 +++++++++---
 drivers/infiniband/ulp/srp/ib_srp.h |  1 +
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
b/drivers/infiniband/ulp/srp/ib_srp.c
index 2ec9c05..3294f10 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2614,6 +2614,8 @@ static ssize_t srp_create_target(struct device *dev,
 	target->tl_retry_count	= 7;
 	target->queue_size	= SRP_DEFAULT_QUEUE_SIZE;

+	mutex_lock(&host->add_target_mutex);
+
 	ret = srp_parse_options(buf, target);
 	if (ret)
 		goto err;
@@ -2682,7 +2684,11 @@ static ssize_t srp_create_target(struct device *dev,
 		     be64_to_cpu(target->service_id),
 		     target->path.sgid.raw, target->path.dgid.raw);

-	return count;
+	ret = count;
+
+out:
+	mutex_unlock(&host->add_target_mutex);
+	return ret;

 err_disconnect:
 	srp_disconnect_target(target);
@@ -2698,8 +2704,7 @@ err_free_mem:

 err:
 	scsi_host_put(target_host);
-
-	return ret;
+	goto out;
 }

 static DEVICE_ATTR(add_target, S_IWUSR, NULL, srp_create_target);
@@ -2735,6 +2740,7 @@ static struct srp_host *srp_add_port(struct
srp_device *device, u8 port)
 	INIT_LIST_HEAD(&host->target_list);
 	spin_lock_init(&host->target_lock);
 	init_completion(&host->released);
+	mutex_init(&host->add_target_mutex);
 	host->srp_dev = device;
 	host->port = port;

diff --git a/drivers/infiniband/ulp/srp/ib_srp.h
b/drivers/infiniband/ulp/srp/ib_srp.h
index 5756810..aad27b7 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -105,6 +105,7 @@ struct srp_host {
 	spinlock_t		target_lock;
 	struct completion	released;
 	struct list_head	list;
+	struct mutex		add_target_mutex;
 };

 struct srp_request {
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v3 5/7] IB/srp: Make writing into the "add_target" sysfs attribute interruptible
       [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
                         ` (3 preceding siblings ...)
  2014-03-14 12:52       ` [PATCH v3 4/7] IB/srp: Avoid duplicate connections Bart Van Assche
@ 2014-03-14 12:53       ` Bart Van Assche
  2014-03-14 12:53       ` [PATCH v3 6/7] IB/srp: Avoid that writing into "add_target" hangs due to a cable pull Bart Van Assche
  2014-03-14 12:54       ` [PATCH v3 7/7] IB/srp: Fix a race condition between failing I/O and I/O completion Bart Van Assche
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-14 12:53 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

Avoid that stopping srp_daemon takes unusually long due to a cable
pull by making writing into the "add_target" sysfs attribute
interruptible.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 3294f10..481c873 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -411,6 +411,8 @@ static void srp_path_rec_completion(int status,
 
 static int srp_lookup_path(struct srp_target_port *target)
 {
+	int ret;
+
 	target->path.numb_path = 1;
 
 	init_completion(&target->done);
@@ -431,7 +433,9 @@ static int srp_lookup_path(struct srp_target_port *target)
 	if (target->path_query_id < 0)
 		return target->path_query_id;
 
-	wait_for_completion(&target->done);
+	ret = wait_for_completion_interruptible(&target->done);
+	if (ret < 0)
+		return ret;
 
 	if (target->status < 0)
 		shost_printk(KERN_WARNING, target->scsi_host,
@@ -710,7 +714,9 @@ static int srp_connect_target(struct srp_target_port *target)
 		ret = srp_send_req(target);
 		if (ret)
 			return ret;
-		wait_for_completion(&target->done);
+		ret = wait_for_completion_interruptible(&target->done);
+		if (ret < 0)
+			return ret;
 
 		/*
 		 * The CM event handling code will set status to
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v3 6/7] IB/srp: Avoid that writing into "add_target" hangs due to a cable pull
       [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
                         ` (4 preceding siblings ...)
  2014-03-14 12:53       ` [PATCH v3 5/7] IB/srp: Make writing into the "add_target" sysfs attribute interruptible Bart Van Assche
@ 2014-03-14 12:53       ` Bart Van Assche
  2014-03-14 12:54       ` [PATCH v3 7/7] IB/srp: Fix a race condition between failing I/O and I/O completion Bart Van Assche
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-14 12:53 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

If a cable is pulled while srp_connect_target() is in progress
that can result in that function never to return. That makes the
process, e.g. srp_daemon, that invoked this function unkillable.
Avoid this by letting srp_connect_target() finish if the event
IB_CM_TIMEWAIT_EXIT is received. This patch fixes a hang with the
following call trace:
 [<ffffffff814eae85>] schedule_timeout+0x215/0x2e0
 [<ffffffff814eab03>] wait_for_common+0x123/0x180
 [<ffffffff814eac1d>] wait_for_completion+0x1d/0x20
 [<ffffffffa03b398c>] srp_connect_target+0x1dc/0x410 [ib_srp]
 [<ffffffffa03b5809>] srp_create_target+0xba9/0xe70 [ib_srp]
 [<ffffffff8133e590>] dev_attr_store+0x20/0x30
 [<ffffffff811eb8f5>] sysfs_write_file+0xe5/0x170
 [<ffffffff811767c8>] vfs_write+0xb8/0x1a0
 [<ffffffff811770c1>] sys_write+0x51/0x90
 [<ffffffff8100b072>] system_call_fastpath+0x16/0x1b

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 481c873..a64e469 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1871,6 +1871,7 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 	case IB_CM_TIMEWAIT_EXIT:
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "connection closed\n");
+		comp = 1;
 
 		target->status = 0;
 		break;
-- 
1.8.4.5

--
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] 16+ messages in thread

* [PATCH v3 7/7] IB/srp: Fix a race condition between failing I/O and I/O completion
       [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
                         ` (5 preceding siblings ...)
  2014-03-14 12:53       ` [PATCH v3 6/7] IB/srp: Avoid that writing into "add_target" hangs due to a cable pull Bart Van Assche
@ 2014-03-14 12:54       ` Bart Van Assche
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2014-03-14 12:54 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Sagi Grimberg, Vu Pham, Sebastian Riemer, David Dillow,
	James Bottomley, Masanari Iida, linux-rdma

Avoid that srp_terminate_io() can access req->scmnd after it has been
cleared by the I/O completion code. Do this by protecting req->scmnd
accesses from srp_terminate_io() via locking

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Acked-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index a64e469..66a908b 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -783,6 +783,7 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd,
  * srp_claim_req - Take ownership of the scmnd associated with a request.
  * @target: SRP target port.
  * @req: SRP request.
+ * @sdev: If not NULL, only take ownership for this SCSI device.
  * @scmnd: If NULL, take ownership of @req->scmnd. If not NULL, only take
  *         ownership of @req->scmnd if it equals @scmnd.
  *
@@ -791,16 +792,17 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd,
  */
 static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target,
 				       struct srp_request *req,
+				       struct scsi_device *sdev,
 				       struct scsi_cmnd *scmnd)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&target->lock, flags);
-	if (!scmnd) {
+	if (req->scmnd &&
+	    (!sdev || req->scmnd->device == sdev) &&
+	    (!scmnd || req->scmnd == scmnd)) {
 		scmnd = req->scmnd;
 		req->scmnd = NULL;
-	} else if (req->scmnd == scmnd) {
-		req->scmnd = NULL;
 	} else {
 		scmnd = NULL;
 	}
@@ -827,9 +829,10 @@ static void srp_free_req(struct srp_target_port *target,
 }
 
 static void srp_finish_req(struct srp_target_port *target,
-			   struct srp_request *req, int result)
+			   struct srp_request *req, struct scsi_device *sdev,
+			   int result)
 {
-	struct scsi_cmnd *scmnd = srp_claim_req(target, req, NULL);
+	struct scsi_cmnd *scmnd = srp_claim_req(target, req, sdev, NULL);
 
 	if (scmnd) {
 		srp_free_req(target, req, scmnd, 0);
@@ -841,11 +844,20 @@ static void srp_finish_req(struct srp_target_port *target,
 static void srp_terminate_io(struct srp_rport *rport)
 {
 	struct srp_target_port *target = rport->lld_data;
+	struct Scsi_Host *shost = target->scsi_host;
+	struct scsi_device *sdev;
 	int i;
 
+	/*
+	 * Invoking srp_terminate_io() while srp_queuecommand() is running
+	 * is not safe. Hence the warning statement below.
+	 */
+	shost_for_each_device(sdev, shost)
+		WARN_ON_ONCE(sdev->request_queue->request_fn_active);
+
 	for (i = 0; i < target->req_ring_size; ++i) {
 		struct srp_request *req = &target->req_ring[i];
-		srp_finish_req(target, req, DID_TRANSPORT_FAILFAST << 16);
+		srp_finish_req(target, req, NULL, DID_TRANSPORT_FAILFAST << 16);
 	}
 }
 
@@ -882,7 +894,7 @@ static int srp_rport_reconnect(struct srp_rport *rport)
 
 	for (i = 0; i < target->req_ring_size; ++i) {
 		struct srp_request *req = &target->req_ring[i];
-		srp_finish_req(target, req, DID_RESET << 16);
+		srp_finish_req(target, req, NULL, DID_RESET << 16);
 	}
 
 	INIT_LIST_HEAD(&target->free_tx);
@@ -1290,7 +1302,7 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)
 		complete(&target->tsk_mgmt_done);
 	} else {
 		req = &target->req_ring[rsp->tag];
-		scmnd = srp_claim_req(target, req, NULL);
+		scmnd = srp_claim_req(target, req, NULL, NULL);
 		if (!scmnd) {
 			shost_printk(KERN_ERR, target->scsi_host,
 				     "Null scmnd for RSP w/tag %016llx\n",
@@ -2008,7 +2020,7 @@ static int srp_abort(struct scsi_cmnd *scmnd)
 
 	shost_printk(KERN_ERR, target->scsi_host, "SRP abort called\n");
 
-	if (!req || !srp_claim_req(target, req, scmnd))
+	if (!req || !srp_claim_req(target, req, NULL, scmnd))
 		return SUCCESS;
 	if (srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
 			      SRP_TSK_ABORT_TASK) == 0)
@@ -2039,8 +2051,7 @@ static int srp_reset_device(struct scsi_cmnd *scmnd)
 
 	for (i = 0; i < target->req_ring_size; ++i) {
 		struct srp_request *req = &target->req_ring[i];
-		if (req->scmnd && req->scmnd->device == scmnd->device)
-			srp_finish_req(target, req, DID_RESET << 16);
+		srp_finish_req(target, req, scmnd->device, DID_RESET << 16);
 	}
 
 	return SUCCESS;
-- 
1.8.4.5

--
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] 16+ messages in thread

end of thread, other threads:[~2014-03-14 12:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-10 11:39 [PATCH v2 0/7] SRP initiator patches for kernel 3.15 Bart Van Assche
     [not found] ` <531DA483.3010400-HInyCGIudOg@public.gmane.org>
2014-03-10 11:40   ` [PATCH v2 1/7] scsi_transport_srp: Fix two kernel-doc warnings Bart Van Assche
2014-03-10 11:42   ` [PATCH v2 2/7] IB/srp: Check ib_query_gid return value Bart Van Assche
2014-03-10 11:43   ` [PATCH v2 3/7] IB/srp: Add more logging Bart Van Assche
2014-03-10 11:44   ` [PATCH v2 4/7] IB/srp: Avoid duplicate connections Bart Van Assche
2014-03-10 11:45   ` [PATCH v2 5/7] IB/srp: Make writing into the "add_target" sysfs attribute interruptible Bart Van Assche
2014-03-10 11:46   ` [PATCH v2 6/7] IB/srp: Avoid that writing into "add_target" hangs due to a cable pull Bart Van Assche
2014-03-10 11:47   ` [PATCH v2 7/7] IB/srp: Fix a race condition between failing I/O and I/O queueing Bart Van Assche
2014-03-14 12:50   ` [PATCH v3 0/7] SRP initiator patches for kernel 3.15 Bart Van Assche
     [not found]     ` <5322FAF8.6020504-HInyCGIudOg@public.gmane.org>
2014-03-14 12:51       ` [PATCH v3 1/7] scsi_transport_srp: Fix two kernel-doc warnings Bart Van Assche
2014-03-14 12:51       ` [PATCH v3 2/7] IB/srp: Check ib_query_gid return value Bart Van Assche
2014-03-14 12:52       ` [PATCH v3 3/7] IB/srp: Add more logging Bart Van Assche
2014-03-14 12:52       ` [PATCH v3 4/7] IB/srp: Avoid duplicate connections Bart Van Assche
2014-03-14 12:53       ` [PATCH v3 5/7] IB/srp: Make writing into the "add_target" sysfs attribute interruptible Bart Van Assche
2014-03-14 12:53       ` [PATCH v3 6/7] IB/srp: Avoid that writing into "add_target" hangs due to a cable pull Bart Van Assche
2014-03-14 12:54       ` [PATCH v3 7/7] IB/srp: Fix a race condition between failing I/O and I/O completion Bart Van Assche

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox