public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] SRP initiator patches for kernel 3.17
@ 2014-07-09 13:55 Bart Van Assche
       [not found] ` <53BD49E3.3090903-HInyCGIudOg@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Bart Van Assche @ 2014-07-09 13:55 UTC (permalink / raw)
  To: Roland Dreier
  Cc: James Bottomley, Sagi Grimberg, David Dillow,
	Sebastian Parschauer, linux-rdma,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/4] scsi_transport_srp: Fix fast_io_fail_tmo=dev_loss_tmo=off behavior
       [not found] ` <53BD49E3.3090903-HInyCGIudOg@public.gmane.org>
@ 2014-07-09 13:56   ` Bart Van Assche
  2014-07-09 13:57   ` [PATCH v2 3/4] IB/srp: Fix residual handling Bart Van Assche
  1 sibling, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2014-07-09 13:56 UTC (permalink / raw)
  To: Roland Dreier
  Cc: James Bottomley, Sagi Grimberg, David Dillow,
	Sebastian Parschauer, linux-rdma,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

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".

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Reviewed-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: David Dillow <dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
Cc: James Bottomley <jbottomley-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Cc: Sebastian Parschauer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 drivers/scsi/scsi_transport_srp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index 13e8983..a0c5bfd 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -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);
-- 
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] 4+ messages in thread

* [PATCH v2 3/4] IB/srp: Fix residual handling
       [not found] ` <53BD49E3.3090903-HInyCGIudOg@public.gmane.org>
  2014-07-09 13:56   ` [PATCH v2 1/4] scsi_transport_srp: Fix fast_io_fail_tmo=dev_loss_tmo=off behavior Bart Van Assche
@ 2014-07-09 13:57   ` Bart Van Assche
       [not found]     ` <53BD4A5F.6070708-HInyCGIudOg@public.gmane.org>
  1 sibling, 1 reply; 4+ messages in thread
From: Bart Van Assche @ 2014-07-09 13:57 UTC (permalink / raw)
  To: Roland Dreier
  Cc: James Bottomley, Sagi Grimberg, David Dillow,
	Sebastian Parschauer, linux-rdma,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

>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.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Reviewed-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: David Dillow <dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
Cc: Sebastian Parschauer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 8bcdde8..6894822 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -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));
-- 
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] 4+ messages in thread

* Re: [PATCH v2 3/4] IB/srp: Fix residual handling
       [not found]     ` <53BD4A5F.6070708-HInyCGIudOg@public.gmane.org>
@ 2014-07-10  4:07       ` David Dillow
  0 siblings, 0 replies; 4+ messages in thread
From: David Dillow @ 2014-07-10  4:07 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Roland Dreier, James Bottomley, Sagi Grimberg,
	Sebastian Parschauer, linux-rdma,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On Wed, 2014-07-09 at 15:57 +0200, Bart Van Assche wrote:
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -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.

> -		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));


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

end of thread, other threads:[~2014-07-10  4:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-09 13:55 [PATCH v2 0/4] SRP initiator patches for kernel 3.17 Bart Van Assche
     [not found] ` <53BD49E3.3090903-HInyCGIudOg@public.gmane.org>
2014-07-09 13:56   ` [PATCH v2 1/4] scsi_transport_srp: Fix fast_io_fail_tmo=dev_loss_tmo=off behavior Bart Van Assche
2014-07-09 13:57   ` [PATCH v2 3/4] IB/srp: Fix residual handling Bart Van Assche
     [not found]     ` <53BD4A5F.6070708-HInyCGIudOg@public.gmane.org>
2014-07-10  4:07       ` David Dillow

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