All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.