public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 06/15] qla4xxx: Do not wait for the outstanding commands to complete
@ 2010-10-07  5:49 Vikas Chaudhary
  2010-10-08  2:46 ` Mike Christie
  0 siblings, 1 reply; 6+ messages in thread
From: Vikas Chaudhary @ 2010-10-07  5:49 UTC (permalink / raw)
  To: James.Bottomley@suse.de, linux-scsi@vger.kernel.org
  Cc: Nilesh Javali, Vikas Chaudhary, Ravi Anand

From: Nilesh Javali <nilesh.javali@qlogic.com>

Do not wait for the outstanding commands to complete in case
of firmware hang.

Signed-off-by: Nilesh Javali <nilesh.javali@qlogic.com>
Signed-off-by: Vikas Chaudhary <vikas.chaudhary@qlogic.com>
Signed-off-by: Ravi Anand <ravi.anand@qlogic.com>
---
 drivers/scsi/qla4xxx/ql4_os.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 56962e5..a6455fb 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -1102,7 +1102,8 @@ static int qla4xxx_recover_adapter(struct scsi_qla_host *ha)
 		    ha->host_no, __func__));
 		status = ha->isp_ops->reset_firmware(ha);
 		if (status == QLA_SUCCESS) {
-			qla4xxx_cmd_wait(ha);
+			if (!test_bit(AF_FW_RECOVERY, &ha->flags))
+				qla4xxx_cmd_wait(ha);
 			ha->isp_ops->disable_intrs(ha);
 			qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
 			qla4xxx_abort_active_cmds(ha, DID_RESET << 16);
@@ -1119,7 +1120,8 @@ static int qla4xxx_recover_adapter(struct scsi_qla_host *ha)
 	 * or if stop_firmware fails for ISP-82xx.
 	 * This is the default case for ISP-4xxx */
 	if (!is_qla8022(ha) || reset_chip) {
-		qla4xxx_cmd_wait(ha);
+		if (!test_bit(AF_FW_RECOVERY, &ha->flags))
+			qla4xxx_cmd_wait(ha);
 		qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
 		qla4xxx_abort_active_cmds(ha, DID_RESET << 16);
 		DEBUG2(ql4_printk(KERN_INFO, ha,
-- 
1.6.6


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

* Re: [PATCH 06/15] qla4xxx: Do not wait for the outstanding commands to complete
  2010-10-07  5:49 [PATCH 06/15] qla4xxx: Do not wait for the outstanding commands to complete Vikas Chaudhary
@ 2010-10-08  2:46 ` Mike Christie
  2010-10-11  9:24   ` Vikas Chaudhary
  0 siblings, 1 reply; 6+ messages in thread
From: Mike Christie @ 2010-10-08  2:46 UTC (permalink / raw)
  To: Vikas Chaudhary
  Cc: James.Bottomley@suse.de, linux-scsi@vger.kernel.org,
	Nilesh Javali, Ravi Anand

[-- Attachment #1: Type: text/plain, Size: 2380 bytes --]

On 10/07/2010 12:49 AM, Vikas Chaudhary wrote:
> From: Nilesh Javali<nilesh.javali@qlogic.com>
>
> Do not wait for the outstanding commands to complete in case
> of firmware hang.
>
> Signed-off-by: Nilesh Javali<nilesh.javali@qlogic.com>
> Signed-off-by: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
> Signed-off-by: Ravi Anand<ravi.anand@qlogic.com>
> ---
>   drivers/scsi/qla4xxx/ql4_os.c |    6 ++++--
>   1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 56962e5..a6455fb 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -1102,7 +1102,8 @@ static int qla4xxx_recover_adapter(struct scsi_qla_host *ha)
>   		    ha->host_no, __func__));
>   		status = ha->isp_ops->reset_firmware(ha);
>   		if (status == QLA_SUCCESS) {
> -			qla4xxx_cmd_wait(ha);
> +			if (!test_bit(AF_FW_RECOVERY,&ha->flags))
> +				qla4xxx_cmd_wait(ha);
>   			ha->isp_ops->disable_intrs(ha);
>   			qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
>   			qla4xxx_abort_active_cmds(ha, DID_RESET<<  16);
> @@ -1119,7 +1120,8 @@ static int qla4xxx_recover_adapter(struct scsi_qla_host *ha)
>   	 * or if stop_firmware fails for ISP-82xx.
>   	 * This is the default case for ISP-4xxx */
>   	if (!is_qla8022(ha) || reset_chip) {
> -		qla4xxx_cmd_wait(ha);
> +		if (!test_bit(AF_FW_RECOVERY,&ha->flags))
> +			qla4xxx_cmd_wait(ha);
>   		qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
>   		qla4xxx_abort_active_cmds(ha, DID_RESET<<  16);
>   		DEBUG2(ql4_printk(KERN_INFO, ha,

If we go from qla4xxx_eh_host_reset->qla4xxx_recover_adapter and you do 
not wait, is is possible for the driver to be accessing scsi commands 
after qla4xxx_eh_host_reset returns? If so I think there is a problem 
where the scsi eh will fail or retry the cmd so the memory for the scsi 
command could be reallocated/freed.

On a related note, has qla4xxx_eh_host_reset ever returned SUCCESS? I 
think there is a problem in qla4xxx_cmd_wait when the scsi eh is 
running. At that time blk_finish_request->blk_queue_end_tag is not going 
to be run, because if the driver were to call scsi_done the block layer 
would do blk_complete_request and the blk_mark_rq_complete would fail 
(this is because the scsi/block eh timeout could has already marked it 
complete).

I think you need the attached patch.


[-- Attachment #2: qla4xxx-fix-cmd-wait-cmd-check.patch --]
[-- Type: text/plain, Size: 1260 bytes --]

qla4xxx: Fix cmd check in qla4xxx_cmd_wait

If the command has timedout then the block layer has called
blk_mark_rq_complete. If qla4xxx_cmd_wait is then called
from qla4xxx_eh_host_reset, we will always fail, because if
the driver calls scsi_done then the the block layer will fail
at blk_complete_request's blk_mark_rq_complete call instead of
calling the normal completion path including the function,
blk_queue_end_tag, which releases the tag.

Patch is not tested.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>


diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 370d40f..97f6d68 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -885,7 +885,13 @@ static int qla4xxx_cmd_wait(struct scsi_qla_host *ha)
 		/* Find a command that hasn't completed. */
 		for (index = 0; index < ha->host->can_queue; index++) {
 			cmd = scsi_host_find_tag(ha->host, index);
-			if (cmd != NULL)
+			/*
+			 * We cannot just check if the index is valid,
+			 * becase if we are run from the scsi eh, then
+			 * the scsi/block layer is going to prevent
+			 * the tag from being released.
+			 */ 
+			if (cmd != NULL && CMD_SP(cmd))
 				break;
 		}
 		spin_unlock_irqrestore(&ha->hardware_lock, flags);

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

* RE: [PATCH 06/15] qla4xxx: Do not wait for the outstanding commands to complete
  2010-10-08  2:46 ` Mike Christie
@ 2010-10-11  9:24   ` Vikas Chaudhary
  2010-10-25 19:56     ` James Bottomley
  0 siblings, 1 reply; 6+ messages in thread
From: Vikas Chaudhary @ 2010-10-11  9:24 UTC (permalink / raw)
  To: Mike Christie
  Cc: James.Bottomley@suse.de, linux-scsi@vger.kernel.org,
	Nilesh Javali, Ravi Anand

>-----Original Message-----
>From: Mike Christie [mailto:michaelc@cs.wisc.edu]
>Sent: Friday, October 08, 2010 8:16 AM
>To: Vikas Chaudhary
>Cc: James.Bottomley@suse.de; linux-scsi@vger.kernel.org; Nilesh Javali;
>Ravi Anand
>Subject: Re: [PATCH 06/15] qla4xxx: Do not wait for the outstanding
>commands to complete
>
>On 10/07/2010 12:49 AM, Vikas Chaudhary wrote:
>> From: Nilesh Javali<nilesh.javali@qlogic.com>
>>
>> Do not wait for the outstanding commands to complete in case of
>> firmware hang.
>>
>> Signed-off-by: Nilesh Javali<nilesh.javali@qlogic.com>
>> Signed-off-by: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
>> Signed-off-by: Ravi Anand<ravi.anand@qlogic.com>
>> ---
>>   drivers/scsi/qla4xxx/ql4_os.c |    6 ++++--
>>   1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/qla4xxx/ql4_os.c
>> b/drivers/scsi/qla4xxx/ql4_os.c index 56962e5..a6455fb 100644
>> --- a/drivers/scsi/qla4xxx/ql4_os.c
>> +++ b/drivers/scsi/qla4xxx/ql4_os.c
>> @@ -1102,7 +1102,8 @@ static int qla4xxx_recover_adapter(struct
>scsi_qla_host *ha)
>>   		    ha->host_no, __func__));
>>   		status = ha->isp_ops->reset_firmware(ha);
>>   		if (status == QLA_SUCCESS) {
>> -			qla4xxx_cmd_wait(ha);
>> +			if (!test_bit(AF_FW_RECOVERY,&ha->flags))
>> +				qla4xxx_cmd_wait(ha);
>>   			ha->isp_ops->disable_intrs(ha);
>>   			qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
>>   			qla4xxx_abort_active_cmds(ha, DID_RESET<<  16); @@ -
>1119,7
>> +1120,8 @@ static int qla4xxx_recover_adapter(struct scsi_qla_host *ha)
>>   	 * or if stop_firmware fails for ISP-82xx.
>>   	 * This is the default case for ISP-4xxx */
>>   	if (!is_qla8022(ha) || reset_chip) {
>> -		qla4xxx_cmd_wait(ha);
>> +		if (!test_bit(AF_FW_RECOVERY,&ha->flags))
>> +			qla4xxx_cmd_wait(ha);
>>   		qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
>>   		qla4xxx_abort_active_cmds(ha, DID_RESET<<  16);
>>   		DEBUG2(ql4_printk(KERN_INFO, ha,
>
>If we go from qla4xxx_eh_host_reset->qla4xxx_recover_adapter and you do not
>wait, is is possible for the driver to be accessing scsi commands after
>qla4xxx_eh_host_reset returns? If so I think there is a problem where the
>scsi eh will fail or retry the cmd so the memory for the scsi command could
>be reallocated/freed.
>

We do not access command after qla4xxx_eh_host_reset returns.

>On a related note, has qla4xxx_eh_host_reset ever returned SUCCESS? I think
>there is a problem in qla4xxx_cmd_wait when the scsi eh is running. At that
>time blk_finish_request->blk_queue_end_tag is not going to be run, because
>if the driver were to call scsi_done the block layer would do
>blk_complete_request and the blk_mark_rq_complete would fail (this is
>because the scsi/block eh timeout could has already marked it complete).
>
>I think you need the attached patch.

Yes. We need attached patch. I am doing some testing with this patch.
I will send out this patch for inclusion soon.

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

* RE: [PATCH 06/15] qla4xxx: Do not wait for the outstanding commands to complete
  2010-10-11  9:24   ` Vikas Chaudhary
@ 2010-10-25 19:56     ` James Bottomley
  2010-10-25 23:22       ` Ravi Anand
  2010-10-26 12:44       ` Vikas Chaudhary
  0 siblings, 2 replies; 6+ messages in thread
From: James Bottomley @ 2010-10-25 19:56 UTC (permalink / raw)
  To: Vikas Chaudhary
  Cc: Mike Christie, linux-scsi@vger.kernel.org, Nilesh Javali,
	Ravi Anand

On Mon, 2010-10-11 at 02:24 -0700, Vikas Chaudhary wrote:
> >-----Original Message-----
> >From: Mike Christie [mailto:michaelc@cs.wisc.edu]
> >Sent: Friday, October 08, 2010 8:16 AM
> >To: Vikas Chaudhary
> >Cc: James.Bottomley@suse.de; linux-scsi@vger.kernel.org; Nilesh Javali;
> >Ravi Anand
> >Subject: Re: [PATCH 06/15] qla4xxx: Do not wait for the outstanding
> >commands to complete
> >
> >On 10/07/2010 12:49 AM, Vikas Chaudhary wrote:
> >> From: Nilesh Javali<nilesh.javali@qlogic.com>
> >>
> >> Do not wait for the outstanding commands to complete in case of
> >> firmware hang.
> >>
> >> Signed-off-by: Nilesh Javali<nilesh.javali@qlogic.com>
> >> Signed-off-by: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
> >> Signed-off-by: Ravi Anand<ravi.anand@qlogic.com>
> >> ---
> >>   drivers/scsi/qla4xxx/ql4_os.c |    6 ++++--
> >>   1 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/scsi/qla4xxx/ql4_os.c
> >> b/drivers/scsi/qla4xxx/ql4_os.c index 56962e5..a6455fb 100644
> >> --- a/drivers/scsi/qla4xxx/ql4_os.c
> >> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> >> @@ -1102,7 +1102,8 @@ static int qla4xxx_recover_adapter(struct
> >scsi_qla_host *ha)
> >>   		    ha->host_no, __func__));
> >>   		status = ha->isp_ops->reset_firmware(ha);
> >>   		if (status == QLA_SUCCESS) {
> >> -			qla4xxx_cmd_wait(ha);
> >> +			if (!test_bit(AF_FW_RECOVERY,&ha->flags))
> >> +				qla4xxx_cmd_wait(ha);
> >>   			ha->isp_ops->disable_intrs(ha);
> >>   			qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
> >>   			qla4xxx_abort_active_cmds(ha, DID_RESET<<  16); @@ -
> >1119,7
> >> +1120,8 @@ static int qla4xxx_recover_adapter(struct scsi_qla_host *ha)
> >>   	 * or if stop_firmware fails for ISP-82xx.
> >>   	 * This is the default case for ISP-4xxx */
> >>   	if (!is_qla8022(ha) || reset_chip) {
> >> -		qla4xxx_cmd_wait(ha);
> >> +		if (!test_bit(AF_FW_RECOVERY,&ha->flags))
> >> +			qla4xxx_cmd_wait(ha);
> >>   		qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
> >>   		qla4xxx_abort_active_cmds(ha, DID_RESET<<  16);
> >>   		DEBUG2(ql4_printk(KERN_INFO, ha,
> >
> >If we go from qla4xxx_eh_host_reset->qla4xxx_recover_adapter and you do not
> >wait, is is possible for the driver to be accessing scsi commands after
> >qla4xxx_eh_host_reset returns? If so I think there is a problem where the
> >scsi eh will fail or retry the cmd so the memory for the scsi command could
> >be reallocated/freed.
> >
> 
> We do not access command after qla4xxx_eh_host_reset returns.
> 
> >On a related note, has qla4xxx_eh_host_reset ever returned SUCCESS? I think
> >there is a problem in qla4xxx_cmd_wait when the scsi eh is running. At that
> >time blk_finish_request->blk_queue_end_tag is not going to be run, because
> >if the driver were to call scsi_done the block layer would do
> >blk_complete_request and the blk_mark_rq_complete would fail (this is
> >because the scsi/block eh timeout could has already marked it complete).
> >
> >I think you need the attached patch.
> 
> Yes. We need attached patch. I am doing some testing with this patch.
> I will send out this patch for inclusion soon.

Is there an update on this?  It would be nice to include it before the
merge window closes.

James



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

* Re: [PATCH 06/15] qla4xxx: Do not wait for the outstanding commands to complete
  2010-10-25 19:56     ` James Bottomley
@ 2010-10-25 23:22       ` Ravi Anand
  2010-10-26 12:44       ` Vikas Chaudhary
  1 sibling, 0 replies; 6+ messages in thread
From: Ravi Anand @ 2010-10-25 23:22 UTC (permalink / raw)
  To: James Bottomley
  Cc: Vikas Chaudhary, Mike Christie, linux-scsi@vger.kernel.org,
	Nilesh Javali


On Oct 25, 2010, at 12:56 PM, James Bottomley wrote:

> On Mon, 2010-10-11 at 02:24 -0700, Vikas Chaudhary wrote:
>>> -----Original Message-----
>>> From: Mike Christie [mailto:michaelc@cs.wisc.edu]
>>> Sent: Friday, October 08, 2010 8:16 AM
>>> To: Vikas Chaudhary
>>> Cc: James.Bottomley@suse.de; linux-scsi@vger.kernel.org; Nilesh Javali;
>>> Ravi Anand
>>> Subject: Re: [PATCH 06/15] qla4xxx: Do not wait for the outstanding
>>> commands to complete
>>> 
>>> On 10/07/2010 12:49 AM, Vikas Chaudhary wrote:
>>>> From: Nilesh Javali<nilesh.javali@qlogic.com>
>>>> 
>>>> Do not wait for the outstanding commands to complete in case of
>>>> firmware hang.
>>>> 
>>>> Signed-off-by: Nilesh Javali<nilesh.javali@qlogic.com>
>>>> Signed-off-by: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
>>>> Signed-off-by: Ravi Anand<ravi.anand@qlogic.com>
>>>> ---
>>>>  drivers/scsi/qla4xxx/ql4_os.c |    6 ++++--
>>>>  1 files changed, 4 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/drivers/scsi/qla4xxx/ql4_os.c
>>>> b/drivers/scsi/qla4xxx/ql4_os.c index 56962e5..a6455fb 100644
>>>> --- a/drivers/scsi/qla4xxx/ql4_os.c
>>>> +++ b/drivers/scsi/qla4xxx/ql4_os.c
>>>> @@ -1102,7 +1102,8 @@ static int qla4xxx_recover_adapter(struct
>>> scsi_qla_host *ha)
>>>>  		    ha->host_no, __func__));
>>>>  		status = ha->isp_ops->reset_firmware(ha);
>>>>  		if (status == QLA_SUCCESS) {
>>>> -			qla4xxx_cmd_wait(ha);
>>>> +			if (!test_bit(AF_FW_RECOVERY,&ha->flags))
>>>> +				qla4xxx_cmd_wait(ha);
>>>>  			ha->isp_ops->disable_intrs(ha);
>>>>  			qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
>>>>  			qla4xxx_abort_active_cmds(ha, DID_RESET<<  16); @@ -
>>> 1119,7
>>>> +1120,8 @@ static int qla4xxx_recover_adapter(struct scsi_qla_host *ha)
>>>>  	 * or if stop_firmware fails for ISP-82xx.
>>>>  	 * This is the default case for ISP-4xxx */
>>>>  	if (!is_qla8022(ha) || reset_chip) {
>>>> -		qla4xxx_cmd_wait(ha);
>>>> +		if (!test_bit(AF_FW_RECOVERY,&ha->flags))
>>>> +			qla4xxx_cmd_wait(ha);
>>>>  		qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
>>>>  		qla4xxx_abort_active_cmds(ha, DID_RESET<<  16);
>>>>  		DEBUG2(ql4_printk(KERN_INFO, ha,
>>> 
>>> If we go from qla4xxx_eh_host_reset->qla4xxx_recover_adapter and you do not
>>> wait, is is possible for the driver to be accessing scsi commands after
>>> qla4xxx_eh_host_reset returns? If so I think there is a problem where the
>>> scsi eh will fail or retry the cmd so the memory for the scsi command could
>>> be reallocated/freed.
>>> 
>> 
>> We do not access command after qla4xxx_eh_host_reset returns.
>> 
>>> On a related note, has qla4xxx_eh_host_reset ever returned SUCCESS? I think
>>> there is a problem in qla4xxx_cmd_wait when the scsi eh is running. At that
>>> time blk_finish_request->blk_queue_end_tag is not going to be run, because
>>> if the driver were to call scsi_done the block layer would do
>>> blk_complete_request and the blk_mark_rq_complete would fail (this is
>>> because the scsi/block eh timeout could has already marked it complete).
>>> 
>>> I think you need the attached patch.
>> 
>> Yes. We need attached patch. I am doing some testing with this patch.
>> I will send out this patch for inclusion soon.
> 
> Is there an update on this?  It would be nice to include it before the
> merge window closes.
> 


Make sense. We will close the loop internally and will have an update.

Thanks
ravi


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

* RE: [PATCH 06/15] qla4xxx: Do not wait for the outstanding commands to complete
  2010-10-25 19:56     ` James Bottomley
  2010-10-25 23:22       ` Ravi Anand
@ 2010-10-26 12:44       ` Vikas Chaudhary
  1 sibling, 0 replies; 6+ messages in thread
From: Vikas Chaudhary @ 2010-10-26 12:44 UTC (permalink / raw)
  To: James Bottomley
  Cc: Mike Christie, linux-scsi@vger.kernel.org, Nilesh Javali,
	Ravi Anand

>-----Original Message-----
>From: James Bottomley [mailto:James.Bottomley@suse.de]
>Sent: Tuesday, October 26, 2010 1:27 AM
>To: Vikas Chaudhary
>Cc: Mike Christie; linux-scsi@vger.kernel.org; Nilesh Javali; Ravi Anand
>Subject: RE: [PATCH 06/15] qla4xxx: Do not wait for the outstanding
>commands to complete
>
>On Mon, 2010-10-11 at 02:24 -0700, Vikas Chaudhary wrote:
>> >-----Original Message-----
>> >From: Mike Christie [mailto:michaelc@cs.wisc.edu]
>> >Sent: Friday, October 08, 2010 8:16 AM
>> >To: Vikas Chaudhary
>> >Cc: James.Bottomley@suse.de; linux-scsi@vger.kernel.org; Nilesh Javali;
>> >Ravi Anand
>> >Subject: Re: [PATCH 06/15] qla4xxx: Do not wait for the outstanding
>> >commands to complete
>> >
>> >On 10/07/2010 12:49 AM, Vikas Chaudhary wrote:
>> >> From: Nilesh Javali<nilesh.javali@qlogic.com>
>> >>
>> >> Do not wait for the outstanding commands to complete in case of
>> >> firmware hang.
>> >>
>> >> Signed-off-by: Nilesh Javali<nilesh.javali@qlogic.com>
>> >> Signed-off-by: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
>> >> Signed-off-by: Ravi Anand<ravi.anand@qlogic.com>
>> >> ---
>> >>   drivers/scsi/qla4xxx/ql4_os.c |    6 ++++--
>> >>   1 files changed, 4 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/scsi/qla4xxx/ql4_os.c
>> >> b/drivers/scsi/qla4xxx/ql4_os.c index 56962e5..a6455fb 100644
>> >> --- a/drivers/scsi/qla4xxx/ql4_os.c
>> >> +++ b/drivers/scsi/qla4xxx/ql4_os.c
>> >> @@ -1102,7 +1102,8 @@ static int qla4xxx_recover_adapter(struct
>> >scsi_qla_host *ha)
>> >>   		    ha->host_no, __func__));
>> >>   		status = ha->isp_ops->reset_firmware(ha);
>> >>   		if (status == QLA_SUCCESS) {
>> >> -			qla4xxx_cmd_wait(ha);
>> >> +			if (!test_bit(AF_FW_RECOVERY,&ha->flags))
>> >> +				qla4xxx_cmd_wait(ha);
>> >>   			ha->isp_ops->disable_intrs(ha);
>> >>   			qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
>> >>   			qla4xxx_abort_active_cmds(ha, DID_RESET<<  16); @@
>-
>> >1119,7
>> >> +1120,8 @@ static int qla4xxx_recover_adapter(struct scsi_qla_host
>*ha)
>> >>   	 * or if stop_firmware fails for ISP-82xx.
>> >>   	 * This is the default case for ISP-4xxx */
>> >>   	if (!is_qla8022(ha) || reset_chip) {
>> >> -		qla4xxx_cmd_wait(ha);
>> >> +		if (!test_bit(AF_FW_RECOVERY,&ha->flags))
>> >> +			qla4xxx_cmd_wait(ha);
>> >>   		qla4xxx_process_aen(ha, FLUSH_DDB_CHANGED_AENS);
>> >>   		qla4xxx_abort_active_cmds(ha, DID_RESET<<  16);
>> >>   		DEBUG2(ql4_printk(KERN_INFO, ha,
>> >
>> >If we go from qla4xxx_eh_host_reset->qla4xxx_recover_adapter and you do
>not
>> >wait, is is possible for the driver to be accessing scsi commands after
>> >qla4xxx_eh_host_reset returns? If so I think there is a problem where
>the
>> >scsi eh will fail or retry the cmd so the memory for the scsi command
>could
>> >be reallocated/freed.
>> >
>>
>> We do not access command after qla4xxx_eh_host_reset returns.
>>
>> >On a related note, has qla4xxx_eh_host_reset ever returned SUCCESS? I
>think
>> >there is a problem in qla4xxx_cmd_wait when the scsi eh is running. At
>that
>> >time blk_finish_request->blk_queue_end_tag is not going to be run,
>because
>> >if the driver were to call scsi_done the block layer would do
>> >blk_complete_request and the blk_mark_rq_complete would fail (this is
>> >because the scsi/block eh timeout could has already marked it complete).
>> >
>> >I think you need the attached patch.
>>
>> Yes. We need attached patch. I am doing some testing with this patch.
>> I will send out this patch for inclusion soon.
>
>Is there an update on this?  It would be nice to include it before the
>merge window closes.
>

I am sending update patch in next email.

Thanks,
Vikas.


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

end of thread, other threads:[~2010-10-26 12:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-07  5:49 [PATCH 06/15] qla4xxx: Do not wait for the outstanding commands to complete Vikas Chaudhary
2010-10-08  2:46 ` Mike Christie
2010-10-11  9:24   ` Vikas Chaudhary
2010-10-25 19:56     ` James Bottomley
2010-10-25 23:22       ` Ravi Anand
2010-10-26 12:44       ` Vikas Chaudhary

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