All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] IB/srp patches for the stable tree
@ 2013-10-10 11:48 Bart Van Assche
       [not found] ` <5256941A.3040506-HInyCGIudOg@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Bart Van Assche @ 2013-10-10 11:48 UTC (permalink / raw)
  To: David Dillow
  Cc: Vu Pham, Sagi Grimberg, Sebastian Riemer, Jack Wang, linux-rdma

The new rules for stable kernel tree patches require these patches to
be posted separately from patches introducing new features. Hence a 
short patch series with the following three patches:

0001-IB-srp-Remove-target-from-list-before-freeing-Scsi_H.patch
0002-IB-srp-Avoid-offlining-operational-SCSI-devices.patch
0003-IB-srp-Report-receive-errors-correctly.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] 9+ messages in thread

* [PATCH 1/3] IB/srp: Remove target from list before freeing Scsi_Host structure
       [not found] ` <5256941A.3040506-HInyCGIudOg@public.gmane.org>
@ 2013-10-10 11:50   ` Bart Van Assche
       [not found]     ` <52569485.8050708-HInyCGIudOg@public.gmane.org>
  2013-10-10 11:52   ` [PATCH 2/3] IB/srp: Avoid offlining operational SCSI devices Bart Van Assche
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Bart Van Assche @ 2013-10-10 11:50 UTC (permalink / raw)
  To: David Dillow
  Cc: Vu Pham, Sagi Grimberg, Sebastian Riemer, Jack Wang, linux-rdma,
	Roland Dreier

From: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Remove an SRP target from the SRP target list before invoking the
last scsi_host_put() call. This change is necessary because that
last put frees the memory that holds the srp_target_port structure.

This patch prevents that the following kernel oops can be triggered:

RIP: 0010:[<ffffffff810b00d0>] __lock_acquire+0x500/0x1570
Call Trace:
 [<ffffffff810b11e4>] lock_acquire+0xa4/0x120
 [<ffffffff81531206>] _spin_lock+0x36/0x70
 [<ffffffffa01b6d8f>] srp_remove_work+0xef/0x180 [ib_srp]
 [<ffffffff8109125c>] worker_thread+0x21c/0x3d0
 [<ffffffff81096e86>] kthread+0x96/0xa0
 [<ffffffff8100c20a>] child_rip+0xa/0x20

Signed-off-by: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
[bvanassche: Modified path description and CC'ed stable]
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index f93baf8..f1318a8 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -534,6 +534,11 @@ static void srp_remove_target(struct srp_target_port *target)
 	ib_destroy_cm_id(target->cm_id);
 	srp_free_target_ib(target);
 	srp_free_req_data(target);
+
+	spin_lock(&target->srp_host->target_lock);
+	list_del(&target->list);
+	spin_unlock(&target->srp_host->target_lock);
+
 	scsi_host_put(target->scsi_host);
 }
 
@@ -545,10 +550,6 @@ static void srp_remove_work(struct work_struct *work)
 	WARN_ON_ONCE(target->state != SRP_TARGET_REMOVED);
 
 	srp_remove_target(target);
-
-	spin_lock(&target->srp_host->target_lock);
-	list_del(&target->list);
-	spin_unlock(&target->srp_host->target_lock);
 }
 
 static void srp_rport_delete(struct srp_rport *rport)
-- 
1.8.1.4

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

* [PATCH 2/3] IB/srp: Avoid offlining operational SCSI devices
       [not found] ` <5256941A.3040506-HInyCGIudOg@public.gmane.org>
  2013-10-10 11:50   ` [PATCH 1/3] IB/srp: Remove target from list before freeing Scsi_Host structure Bart Van Assche
@ 2013-10-10 11:52   ` Bart Van Assche
       [not found]     ` <52569501.5030108-HInyCGIudOg@public.gmane.org>
  2013-10-10 11:53   ` [PATCH 3/3] IB/srp: Report receive errors correctly Bart Van Assche
  2013-10-25 22:16   ` [PATCH 0/3] IB/srp patches for the stable tree David Dillow
  3 siblings, 1 reply; 9+ messages in thread
From: Bart Van Assche @ 2013-10-10 11:52 UTC (permalink / raw)
  To: David Dillow
  Cc: Vu Pham, Sagi Grimberg, Sebastian Riemer, Jack Wang, linux-rdma,
	Roland Dreier

If SCSI commands are submitted with a SCSI request timeout that is
lower than the the IB RC timeout it can happen that the SCSI error
handler has already started device recovery before transport layer
error handling starts. So it can happen that the SCSI error handler
tries to abort a SCSI command after it has been reset by
srp_rport_reconnect(). Tell the SCSI error handler that such commands
have finished and that it is not necessary to continue its recovery
strategy for commands that have been reset by srp_rport_reconnect().

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index f1318a8..93a35a1 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1752,7 +1752,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))
-		return FAILED;
+		return SUCCESS;
 	if (srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
 			      SRP_TSK_ABORT_TASK) == 0)
 		ret = SUCCESS;
-- 
1.8.1.4

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

* [PATCH 3/3] IB/srp: Report receive errors correctly
       [not found] ` <5256941A.3040506-HInyCGIudOg@public.gmane.org>
  2013-10-10 11:50   ` [PATCH 1/3] IB/srp: Remove target from list before freeing Scsi_Host structure Bart Van Assche
  2013-10-10 11:52   ` [PATCH 2/3] IB/srp: Avoid offlining operational SCSI devices Bart Van Assche
@ 2013-10-10 11:53   ` Bart Van Assche
  2013-10-25 22:16   ` [PATCH 0/3] IB/srp patches for the stable tree David Dillow
  3 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2013-10-10 11:53 UTC (permalink / raw)
  To: David Dillow
  Cc: Vu Pham, Sagi Grimberg, Sebastian Riemer, Jack Wang, linux-rdma

The IB spec does not guarantee that the opcode is available in error
completions. Hence do not rely on it. See also commit 948d1e88
("IB/srp: Introduce srp_handle_qp_err()").

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Vu Pham <vu-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.8
---
 drivers/infiniband/ulp/srp/ib_srp.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 93a35a1..17b58f4 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1303,14 +1303,13 @@ static void srp_handle_recv(struct srp_target_port *target, struct ib_wc *wc)
 			     PFX "Recv failed with error code %d\n", res);
 }
 
-static void srp_handle_qp_err(enum ib_wc_status wc_status,
-			      enum ib_wc_opcode wc_opcode,
+static void srp_handle_qp_err(enum ib_wc_status wc_status, bool send_err,
 			      struct srp_target_port *target)
 {
 	if (target->connected && !target->qp_in_error) {
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "failed %s status %d\n",
-			     wc_opcode & IB_WC_RECV ? "receive" : "send",
+			     send_err ? "send" : "receive",
 			     wc_status);
 	}
 	target->qp_in_error = true;
@@ -1326,7 +1325,7 @@ static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)
 		if (likely(wc.status == IB_WC_SUCCESS)) {
 			srp_handle_recv(target, &wc);
 		} else {
-			srp_handle_qp_err(wc.status, wc.opcode, target);
+			srp_handle_qp_err(wc.status, false, target);
 		}
 	}
 }
@@ -1342,7 +1341,7 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr)
 			iu = (struct srp_iu *) (uintptr_t) wc.wr_id;
 			list_add(&iu->list, &target->free_tx);
 		} else {
-			srp_handle_qp_err(wc.status, wc.opcode, target);
+			srp_handle_qp_err(wc.status, true, target);
 		}
 	}
 }
-- 
1.8.1.4

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

* Re: [PATCH 2/3] IB/srp: Avoid offlining operational SCSI devices
       [not found]     ` <52569501.5030108-HInyCGIudOg@public.gmane.org>
@ 2013-10-10 12:31       ` Jack Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Jack Wang @ 2013-10-10 12:31 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: David Dillow, Vu Pham, Sagi Grimberg, Sebastian Riemer,
	linux-rdma, Roland Dreier

On 10/10/2013 01:52 PM, Bart Van Assche wrote:
> If SCSI commands are submitted with a SCSI request timeout that is
> lower than the the IB RC timeout it can happen that the SCSI error
> handler has already started device recovery before transport layer
> error handling starts. So it can happen that the SCSI error handler
> tries to abort a SCSI command after it has been reset by
> srp_rport_reconnect(). Tell the SCSI error handler that such commands
> have finished and that it is not necessary to continue its recovery
> strategy for commands that have been reset by srp_rport_reconnect().
Another possible is srp has already finish the req (req is NULL)when
scsi error handle call into srp_abort.

I tested it on my side, works for me.
You can add my Tested-by if needed.

Thanks Bart.

Jack


> 
> Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
> Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
> Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
> Cc: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> ---
>  drivers/infiniband/ulp/srp/ib_srp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index f1318a8..93a35a1 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -1752,7 +1752,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))
> -		return FAILED;
> +		return SUCCESS;
>  	if (srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
>  			      SRP_TSK_ABORT_TASK) == 0)
>  		ret = SUCCESS;
> 

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

* Re: [PATCH 1/3] IB/srp: Remove target from list before freeing Scsi_Host structure
       [not found]     ` <52569485.8050708-HInyCGIudOg@public.gmane.org>
@ 2013-10-10 12:45       ` Jack Wang
       [not found]         ` <5256A183.3030404-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Jack Wang @ 2013-10-10 12:45 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: David Dillow, Vu Pham, Sagi Grimberg, Sebastian Riemer,
	linux-rdma, Roland Dreier

On 10/10/2013 01:50 PM, Bart Van Assche wrote:
> From: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Remove an SRP target from the SRP target list before invoking the
> last scsi_host_put() call. This change is necessary because that
> last put frees the memory that holds the srp_target_port structure.
> 
> This patch prevents that the following kernel oops can be triggered:
> 
> RIP: 0010:[<ffffffff810b00d0>] __lock_acquire+0x500/0x1570
> Call Trace:
>  [<ffffffff810b11e4>] lock_acquire+0xa4/0x120
>  [<ffffffff81531206>] _spin_lock+0x36/0x70
>  [<ffffffffa01b6d8f>] srp_remove_work+0xef/0x180 [ib_srp]
>  [<ffffffff8109125c>] worker_thread+0x21c/0x3d0
>  [<ffffffff81096e86>] kthread+0x96/0xa0
>  [<ffffffff8100c20a>] child_rip+0xa/0x20
> 
> Signed-off-by: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> [bvanassche: Modified path description and CC'ed stable]
> Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
> Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
> Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
> Cc: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> ---
>  drivers/infiniband/ulp/srp/ib_srp.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index f93baf8..f1318a8 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -534,6 +534,11 @@ static void srp_remove_target(struct srp_target_port *target)
>  	ib_destroy_cm_id(target->cm_id);
>  	srp_free_target_ib(target);
>  	srp_free_req_data(target);
> +
> +	spin_lock(&target->srp_host->target_lock);
> +	list_del(&target->list);
> +	spin_unlock(&target->srp_host->target_lock);
> +
>  	scsi_host_put(target->scsi_host);
>  }
>  
> @@ -545,10 +550,6 @@ static void srp_remove_work(struct work_struct *work)
>  	WARN_ON_ONCE(target->state != SRP_TARGET_REMOVED);
>  
>  	srp_remove_target(target);
> -
> -	spin_lock(&target->srp_host->target_lock);
> -	list_del(&target->list);
> -	spin_unlock(&target->srp_host->target_lock);
>  }

Or move the 3 line code on top of srp_remove_target(target);?
Patch looks good.

Jack
>  
>  static void srp_rport_delete(struct srp_rport *rport)
> 

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

* Re: [PATCH 1/3] IB/srp: Remove target from list before freeing Scsi_Host structure
       [not found]         ` <5256A183.3030404-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
@ 2013-10-10 12:48           ` Bart Van Assche
       [not found]             ` <5256A230.9020007-HInyCGIudOg@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Bart Van Assche @ 2013-10-10 12:48 UTC (permalink / raw)
  To: Jack Wang
  Cc: David Dillow, Vu Pham, Sagi Grimberg, Sebastian Riemer,
	linux-rdma, Roland Dreier

On 10/10/13 14:45, Jack Wang wrote:
> On 10/10/2013 01:50 PM, Bart Van Assche wrote:
>> From: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> Remove an SRP target from the SRP target list before invoking the
>> last scsi_host_put() call. This change is necessary because that
>> last put frees the memory that holds the srp_target_port structure.
>>
>> This patch prevents that the following kernel oops can be triggered:
>>
>> RIP: 0010:[<ffffffff810b00d0>] __lock_acquire+0x500/0x1570
>> Call Trace:
>>   [<ffffffff810b11e4>] lock_acquire+0xa4/0x120
>>   [<ffffffff81531206>] _spin_lock+0x36/0x70
>>   [<ffffffffa01b6d8f>] srp_remove_work+0xef/0x180 [ib_srp]
>>   [<ffffffff8109125c>] worker_thread+0x21c/0x3d0
>>   [<ffffffff81096e86>] kthread+0x96/0xa0
>>   [<ffffffff8100c20a>] child_rip+0xa/0x20
>>
>> Signed-off-by: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> [bvanassche: Modified path description and CC'ed stable]
>> Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
>> Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
>> Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
>> Cc: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
>> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>> ---
>>   drivers/infiniband/ulp/srp/ib_srp.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
>> index f93baf8..f1318a8 100644
>> --- a/drivers/infiniband/ulp/srp/ib_srp.c
>> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
>> @@ -534,6 +534,11 @@ static void srp_remove_target(struct srp_target_port *target)
>>   	ib_destroy_cm_id(target->cm_id);
>>   	srp_free_target_ib(target);
>>   	srp_free_req_data(target);
>> +
>> +	spin_lock(&target->srp_host->target_lock);
>> +	list_del(&target->list);
>> +	spin_unlock(&target->srp_host->target_lock);
>> +
>>   	scsi_host_put(target->scsi_host);
>>   }
>>
>> @@ -545,10 +550,6 @@ static void srp_remove_work(struct work_struct *work)
>>   	WARN_ON_ONCE(target->state != SRP_TARGET_REMOVED);
>>
>>   	srp_remove_target(target);
>> -
>> -	spin_lock(&target->srp_host->target_lock);
>> -	list_del(&target->list);
>> -	spin_unlock(&target->srp_host->target_lock);
>>   }
>
> Or move the 3 line code on top of srp_remove_target(target);?

That would break srp_conn_unique() - removal from the target list must 
only happen after a DREQ has been sent by the initiator to the target.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/3] IB/srp: Remove target from list before freeing Scsi_Host structure
       [not found]             ` <5256A230.9020007-HInyCGIudOg@public.gmane.org>
@ 2013-10-10 13:42               ` Jack Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Jack Wang @ 2013-10-10 13:42 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: David Dillow, Vu Pham, Sagi Grimberg, Sebastian Riemer,
	linux-rdma, Roland Dreier

On 10/10/2013 02:48 PM, Bart Van Assche wrote:
> On 10/10/13 14:45, Jack Wang wrote:
>> On 10/10/2013 01:50 PM, Bart Van Assche wrote:
>>> From: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>
>>> Remove an SRP target from the SRP target list before invoking the
>>> last scsi_host_put() call. This change is necessary because that
>>> last put frees the memory that holds the srp_target_port structure.
>>>
>>> This patch prevents that the following kernel oops can be triggered:
>>>
>>> RIP: 0010:[<ffffffff810b00d0>] __lock_acquire+0x500/0x1570
>>> Call Trace:
>>>   [<ffffffff810b11e4>] lock_acquire+0xa4/0x120
>>>   [<ffffffff81531206>] _spin_lock+0x36/0x70
>>>   [<ffffffffa01b6d8f>] srp_remove_work+0xef/0x180 [ib_srp]
>>>   [<ffffffff8109125c>] worker_thread+0x21c/0x3d0
>>>   [<ffffffff81096e86>] kthread+0x96/0xa0
>>>   [<ffffffff8100c20a>] child_rip+0xa/0x20
>>>
>>> Signed-off-by: Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> [bvanassche: Modified path description and CC'ed stable]
>>> Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
>>> Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
>>> Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Cc: Sebastian Riemer <sebastian.riemer-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
>>> Cc: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
>>> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>>> ---
>>>   drivers/infiniband/ulp/srp/ib_srp.c | 9 +++++----
>>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
>>> b/drivers/infiniband/ulp/srp/ib_srp.c
>>> index f93baf8..f1318a8 100644
>>> --- a/drivers/infiniband/ulp/srp/ib_srp.c
>>> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
>>> @@ -534,6 +534,11 @@ static void srp_remove_target(struct
>>> srp_target_port *target)
>>>       ib_destroy_cm_id(target->cm_id);
>>>       srp_free_target_ib(target);
>>>       srp_free_req_data(target);
>>> +
>>> +    spin_lock(&target->srp_host->target_lock);
>>> +    list_del(&target->list);
>>> +    spin_unlock(&target->srp_host->target_lock);
>>> +
>>>       scsi_host_put(target->scsi_host);
>>>   }
>>>
>>> @@ -545,10 +550,6 @@ static void srp_remove_work(struct work_struct
>>> *work)
>>>       WARN_ON_ONCE(target->state != SRP_TARGET_REMOVED);
>>>
>>>       srp_remove_target(target);
>>> -
>>> -    spin_lock(&target->srp_host->target_lock);
>>> -    list_del(&target->list);
>>> -    spin_unlock(&target->srp_host->target_lock);
>>>   }
>>
>> Or move the 3 line code on top of srp_remove_target(target);?
> 
> That would break srp_conn_unique() - removal from the target list must
> only happen after a DREQ has been sent by the initiator to the target.
> 
> Bart.
> 
You're right, this solution is better. It is possible to break
srp_conn_unique() in my proposal.

Thanks,
Jack


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

* Re: [PATCH 0/3] IB/srp patches for the stable tree
       [not found] ` <5256941A.3040506-HInyCGIudOg@public.gmane.org>
                     ` (2 preceding siblings ...)
  2013-10-10 11:53   ` [PATCH 3/3] IB/srp: Report receive errors correctly Bart Van Assche
@ 2013-10-25 22:16   ` David Dillow
  3 siblings, 0 replies; 9+ messages in thread
From: David Dillow @ 2013-10-25 22:16 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Vu Pham, Sagi Grimberg, Sebastian Riemer, Jack Wang, linux-rdma

On Thu, 2013-10-10 at 13:48 +0200, Bart Van Assche wrote:
> The new rules for stable kernel tree patches require these patches to
> be posted separately from patches introducing new features. Hence a 
> short patch series with the following three patches:
> 
> 0001-IB-srp-Remove-target-from-list-before-freeing-Scsi_H.patch
> 0002-IB-srp-Avoid-offlining-operational-SCSI-devices.patch
> 0003-IB-srp-Report-receive-errors-correctly.patch

Acked-by: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>

for all three. Thanks Bart!
-- 
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2013-10-25 22:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-10 11:48 [PATCH 0/3] IB/srp patches for the stable tree Bart Van Assche
     [not found] ` <5256941A.3040506-HInyCGIudOg@public.gmane.org>
2013-10-10 11:50   ` [PATCH 1/3] IB/srp: Remove target from list before freeing Scsi_Host structure Bart Van Assche
     [not found]     ` <52569485.8050708-HInyCGIudOg@public.gmane.org>
2013-10-10 12:45       ` Jack Wang
     [not found]         ` <5256A183.3030404-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
2013-10-10 12:48           ` Bart Van Assche
     [not found]             ` <5256A230.9020007-HInyCGIudOg@public.gmane.org>
2013-10-10 13:42               ` Jack Wang
2013-10-10 11:52   ` [PATCH 2/3] IB/srp: Avoid offlining operational SCSI devices Bart Van Assche
     [not found]     ` <52569501.5030108-HInyCGIudOg@public.gmane.org>
2013-10-10 12:31       ` Jack Wang
2013-10-10 11:53   ` [PATCH 3/3] IB/srp: Report receive errors correctly Bart Van Assche
2013-10-25 22:16   ` [PATCH 0/3] IB/srp patches for the stable tree David Dillow

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.