SRP initiator patches for kernel 3.17
Subsystems (3)
v2 cover SRP initiator patches for kernel 3.17
This patch series consists of four patches of which three are bug fixes and one is a performance optimization.
Changes compared with v1:
- Left out the fifth patch since there is no agreement about that patch.
- Added "unlikely(...)" around the tests for overflow / underflow flags in the SRP_CMD response as proposed by Dave Dillow.
The patches included in this series are:
0001-scsi_transport_srp-Fix-fast_io_fail_tmo-dev_loss_tmo.patch 0002-IB-srp-Fix-deadlock-between-host-removal-and-multipa.patch 0003-IB-srp-Fix-residual-handling.patch 0004-IB-srp-Use-P_Key-cache-for-P_Key-lookups.patch
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v2 1/4 scsi_transport_srp: Fix fast_io_fail_tmo=dev_loss_tmo=off behavior
If scsi_remove_host() is called while an rport is in the blocked state then scsi_remove_host() will only finish if the rport is unblocked from inside a timer function. Make sure that an rport only enters the blocked state if a timer will be started that will unblock it. This avoids that unloading the ib_srp kernel module after having disconnected the initiator from the target system results in a deadlock if both the fast_io_fail_tmo and dev_loss_tmo parameters have been set to "off".
--- drivers/scsi/scsi_transport_srp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/scsi/scsi_transport_srp.c +2 -1
@@ -473,7 +473,8 @@ static void __srp_start_tl_fail_timers(struct srp_rport *rport)if (delay >0 )queue_delayed_work (system_long_wq, &rport->reconnect_work,1UL * delay *HZ ); -if (srp_rport_set_state (rport,SRP_RPORT_BLOCKED ) ==0 ) { +if ((fast_io_fail_tmo >=0 || dev_loss_tmo >=0 ) && +srp_rport_set_state (rport,SRP_RPORT_BLOCKED ) ==0 ) {pr_debug ("%s new state: %d\n" ,dev_name (&shost->shost_gendev), rport->state);scsi_target_block (&shost->shost_gendev);
Signature · 6 lines
1.8.4.5 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 2/4 IB/srp: Fix deadlock between host removal and multipathd
In reply to a message not in the archiveIf scsi_remove_host() is invoked after a SCSI device has been blocked, if the fast_io_fail_tmo or dev_loss_tmo work gets scheduled on the workqueue executing srp_remove_work() and if an I/O request is scheduled after the SCSI device had been blocked by e.g. multipathd then the following deadlock can occur:
kworker/6:1 D ffff880831f3c460 0 195 2 0x00000000
Call Trace: [<ffffffff814aafd9>] schedule+0x29/0x70 [<ffffffff814aa0ef>] schedule_timeout+0x10f/0x2a0 [<ffffffff8105af6f>] msleep+0x2f/0x40 [<ffffffff8123b0ae>] __blk_drain_queue+0x4e/0x180 [<ffffffff8123d2d5>] blk_cleanup_queue+0x225/0x230 [<ffffffffa0010732>] __scsi_remove_device+0x62/0xe0 [scsi_mod] [<ffffffffa000ed2f>] scsi_forget_host+0x6f/0x80 [scsi_mod] [<ffffffffa0002eba>] scsi_remove_host+0x7a/0x130 [scsi_mod] [<ffffffffa07cf5c5>] srp_remove_work+0x95/0x180 [ib_srp] [<ffffffff8106d7aa>] process_one_work+0x1ea/0x6c0 [<ffffffff8106dd9b>] worker_thread+0x11b/0x3a0 [<ffffffff810758bd>] kthread+0xed/0x110 [<ffffffff814b972c>] ret_from_fork+0x7c/0xb0 multipathd D ffff880096acc460 0 5340 1 0x00000000 Call Trace: [<ffffffff814aafd9>] schedule+0x29/0x70 [<ffffffff814aa0ef>] schedule_timeout+0x10f/0x2a0 [<ffffffff814ab79b>] io_schedule_timeout+0x9b/0xf0 [<ffffffff814abe1c>] wait_for_completion_io_timeout+0xdc/0x110 [<ffffffff81244b9b>] blk_execute_rq+0x9b/0x100 [<ffffffff8124f665>] sg_io+0x1a5/0x450 [<ffffffff8124fd21>] scsi_cmd_ioctl+0x2a1/0x430 [<ffffffff8124fef2>] scsi_cmd_blk_ioctl+0x42/0x50 [<ffffffffa00ec97e>] sd_ioctl+0xbe/0x140 [sd_mod] [<ffffffff8124bd04>] blkdev_ioctl+0x234/0x840 [<ffffffff811cb491>] block_ioctl+0x41/0x50 [<ffffffff811a0df0>] do_vfs_ioctl+0x300/0x520 [<ffffffff811a1051>] SyS_ioctl+0x41/0x80 [<ffffffff814b9962>] tracesys+0xd0/0xd5
Fix this by scheduling removal work on another workqueue than the transport layer timers.
--- drivers/infiniband/ulp/srp/ib_srp.c | 38 +++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +28 -10
@@ -130,6 +130,7 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr);static int srp_cm_handler (struct ib_cm_id *cm_id,struct ib_cm_event *event);static struct scsi_transport_template *ib_srp_transport_template; +static struct workqueue_struct *srp_remove_wq;static struct ib_client srp_client = { .name ="srp" ,
@@ -731,7 +732,7 @@ static bool srp_queue_remove_work(struct srp_target_port *target)spin_unlock_irq (&target->lock);if (changed) -queue_work (system_long_wq, &target->remove_work); +queue_work (srp_remove_wq, &target->remove_work);return changed; }
@@ -3261,9 +3262,10 @@ static void srp_remove_one(struct ib_device *device)spin_unlock (&host->target_lock);/* -* Wait for target port removal tasks. +* Wait for tl_err and target port removal tasks. */ flush_workqueue (system_long_wq); +flush_workqueue (srp_remove_wq);kfree (host); }
@@ -3313,16 +3315,22 @@ static int __init srp_init_module(void) indirect_sg_entries = cmd_sg_entries; } + srp_remove_wq =create_workqueue ("srp_remove" ); +if (IS_ERR (srp_remove_wq)) { + ret =PTR_ERR (srp_remove_wq); + goto out; + } + + ret = -ENOMEM ; ib_srp_transport_template =srp_attach_transport (&ib_srp_transport_functions);if (!ib_srp_transport_template) -return -ENOMEM ; + goto destroy_wq; ret =class_register (&srp_class);if (ret) {pr_err ("couldn't register class infiniband_srp\n" ); -srp_release_transport (ib_srp_transport_template); -return ret; + goto release_tr; }ib_sa_register_client (&srp_sa_client);
@@ -3330,13 +3338,22 @@ static int __init srp_init_module(void) ret =ib_register_client (&srp_client);if (ret) {pr_err ("couldn't register IB client\n" ); -srp_release_transport (ib_srp_transport_template); -ib_sa_unregister_client (&srp_sa_client); -class_unregister (&srp_class); -return ret; + goto unreg_sa; } -return 0 ; +out: +return ret; + +unreg_sa: +ib_sa_unregister_client (&srp_sa_client); +class_unregister (&srp_class); + +release_tr: +srp_release_transport (ib_srp_transport_template); + +destroy_wq: +destroy_workqueue (srp_remove_wq); + goto out; } static void __exit srp_cleanup_module(void)
@@ -3345,6 +3362,7 @@ static void __exit srp_cleanup_module(void)ib_sa_unregister_client (&srp_sa_client);class_unregister (&srp_class);srp_release_transport (ib_srp_transport_template); +destroy_workqueue (srp_remove_wq); }module_init (srp_init_module);
Signature · 6 lines
1.8.4.5 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 3/4 IB/srp: Fix residual handling
In reply to a message not in the archiveQuoted 1 line
From
Documentation/scsi/scsi_mid_low_api.txt: "resid - an LLD should
set this signed integer to the requested transfer length (i.e. 'request_bufflen') less the number of bytes that are actually transferred." This means that resid > 0 in case of an underrun and also that resid < 0 in case of an overrun. Modify the SRP initiator code such that it matches this requirement.
--- drivers/infiniband/ulp/srp/ib_srp.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +7 -3
@@ -1644,10 +1644,14 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp) SCSI_SENSE_BUFFERSIZE)); } -if (rsp->flags & (SRP_RSP_FLAG_DOOVER |SRP_RSP_FLAG_DOUNDER )) -scsi_set_resid (scmnd,be32_to_cpu (rsp->data_out_res_cnt)); -else if (rsp->flags & (SRP_RSP_FLAG_DIOVER |SRP_RSP_FLAG_DIUNDER )) +if (unlikely (rsp->flags &SRP_RSP_FLAG_DIUNDER ))scsi_set_resid (scmnd,be32_to_cpu (rsp->data_in_res_cnt)); +else if (unlikely (rsp->flags &SRP_RSP_FLAG_DIOVER )) +scsi_set_resid (scmnd, -be32_to_cpu (rsp->data_in_res_cnt)); +else if (unlikely (rsp->flags &SRP_RSP_FLAG_DOUNDER )) +scsi_set_resid (scmnd,be32_to_cpu (rsp->data_out_res_cnt)); +else if (unlikely (rsp->flags &SRP_RSP_FLAG_DOOVER )) +scsi_set_resid (scmnd, -be32_to_cpu (rsp->data_out_res_cnt));srp_free_req (target, req, scmnd,be32_to_cpu (rsp->req_lim_delta));
Signature · 6 lines
1.8.4.5 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH v2 3/4] IB/srp: Fix residual handling
drivers/infiniband/ulp/srp/ib_srp.c static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp) · line 1645
Quoted 5 lines · Bart Van Assche
drivers/infiniband/ulp/srp/ib_srp.c +0
-0@@ -1644,10 +1644,14 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp) SCSI_SENSE_BUFFERSIZE)); }
Above this block, there is a line
if (rsp->flags & SRP_RSP_FLAG_SNSVALID) {My thought was that you could wrap the code starting at that block and ending after the additional scsi_set_resid() calls you added with an outer test like so:
if (unlikely(rsp->flags & ~SRP_RSP_FLAG_RSPVALID)) {
if (rsp->flags & SRP_RSP_FLAG_SNSVALID) {
...instead of adding unlikely() to each test of flags. Once inside the block -- which should be rare -- we're on the slow path anyway.
Your call.
Quoted 4 lines
drivers/infiniband/ulp/srp/ib_srp.c +1
-3-if (rsp->flags & (SRP_RSP_FLAG_DOOVER |SRP_RSP_FLAG_DOUNDER )) -scsi_set_resid (scmnd,be32_to_cpu (rsp->data_out_res_cnt)); -else if (rsp->flags & (SRP_RSP_FLAG_DIOVER |SRP_RSP_FLAG_DIUNDER )) +if (unlikely (rsp->flags &SRP_RSP_FLAG_DIUNDER ))
drivers/infiniband/ulp/srp/ib_srp.c static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp) · line 1657
Quoted 10 lines
drivers/infiniband/ulp/srp/ib_srp.c +6
-0scsi_set_resid (scmnd,be32_to_cpu (rsp->data_in_res_cnt)); + else if (unlikely(rsp->flags & SRP_RSP_FLAG_DIOVER)) + scsi_set_resid(scmnd, -be32_to_cpu(rsp->data_in_res_cnt)); + else if (unlikely(rsp->flags & SRP_RSP_FLAG_DOUNDER)) + scsi_set_resid(scmnd, be32_to_cpu(rsp->data_out_res_cnt)); + else if (unlikely(rsp->flags & SRP_RSP_FLAG_DOOVER)) + scsi_set_resid(scmnd, -be32_to_cpu(rsp->data_out_res_cnt));srp_free_req (target, req, scmnd,be32_to_cpu (rsp->req_lim_delta));
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 4/4 IB/srp: Use P_Key cache for P_Key lookups
In reply to a message not in the archiveThis change slightly reduces the time needed to log in.
--- drivers/infiniband/ulp/srp/ib_srp.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +5 -4
@@ -40,6 +40,7 @@#include <linux/parser.h> #include <linux/random.h> #include <linux/jiffies.h> +#include <rdma/ib_cache.h> #include <linux/atomic.h>
@@ -260,10 +261,10 @@ static int srp_init_qp(struct srp_target_port *target,if (!attr)return -ENOMEM ; - ret =ib_find_pkey (target->srp_host->srp_dev->dev, - target->srp_host->port, -be16_to_cpu (target->path.pkey), - &attr->pkey_index); + ret =ib_find_cached_pkey (target->srp_host->srp_dev->dev, + target->srp_host->port, +be16_to_cpu (target->path.pkey), + &attr->pkey_index);if (ret) goto out;
Signature · 6 lines
1.8.4.5 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html