[PATCH 1/2] ipr: Disable async aborts

[PATCH 1/2] ipr: Disable async aborts

[Brian King]

The ipr driver does not yet support async aborts, so disable this
for now.

Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---

 drivers/scsi/ipr.c |    1 +
 1 file changed, 1 insertion(+)

diff -puN drivers/scsi/ipr.c~ipr_no_async_eh drivers/scsi/ipr.c
--- linux/drivers/scsi/ipr.c~ipr_no_async_eh	2014-10-24 08:57:53.169169509 -0500
+++ linux-bjking1/drivers/scsi/ipr.c	2014-10-24 08:57:53.175169579 -0500
@@ -6357,6 +6357,7 @@ static struct scsi_host_template driver_
 	.sdev_attrs = ipr_dev_attrs,
 	.proc_name = IPR_NAME,
 	.no_write_same = 1,
+	.no_async_abort = 1,
 };
 
 /**
_

Re: [PATCH 1/2] ipr: Disable async aborts

[James Bottomley]

On Mon, 2014-10-27 at 09:37 -0500, Brian King wrote:
> The ipr driver does not yet support async aborts, so disable this
> for now.
> 
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>

By "does not yet support" do you mean doesn't work with or merely not
tested with but seems to work OK?  if the former, then surely this
should be a fixes patch with a cc to stable for all the kernels async
abort is in (which is 3.14)

James

> ---
> 
>  drivers/scsi/ipr.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff -puN drivers/scsi/ipr.c~ipr_no_async_eh drivers/scsi/ipr.c
> --- linux/drivers/scsi/ipr.c~ipr_no_async_eh	2014-10-24 08:57:53.169169509 -0500
> +++ linux-bjking1/drivers/scsi/ipr.c	2014-10-24 08:57:53.175169579 -0500
> @@ -6357,6 +6357,7 @@ static struct scsi_host_template driver_
>  	.sdev_attrs = ipr_dev_attrs,
>  	.proc_name = IPR_NAME,
>  	.no_write_same = 1,
> +	.no_async_abort = 1,
>  };
>  
>  /**
> _
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/2] ipr: Disable async aborts

[Christoph Hellwig]

On Mon, Oct 27, 2014 at 07:50:03AM -0700, James Bottomley wrote:
> On Mon, 2014-10-27 at 09:37 -0500, Brian King wrote:
> > The ipr driver does not yet support async aborts, so disable this
> > for now.
> > 
> > Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> 
> By "does not yet support" do you mean doesn't work with or merely not
> tested with but seems to work OK?  if the former, then surely this
> should be a fixes patch with a cc to stable for all the kernels async
> abort is in (which is 3.14)

I'm pretty sure this patch is incorrect, and that what ever issue should
have been described in this patch are the same EH problems we fixed in
the last couple releases.  I've also talked to Hannes a while ago, and
we agree that we want to remove this flag ASAP as it's inviting this

Re: [PATCH 1/2] ipr: Disable async aborts

[Brian King]

On 10/28/2014 03:47 AM, Christoph Hellwig wrote:
> On Mon, Oct 27, 2014 at 07:50:03AM -0700, James Bottomley wrote:
>> On Mon, 2014-10-27 at 09:37 -0500, Brian King wrote:
>>> The ipr driver does not yet support async aborts, so disable this
>>> for now.
>>>
>>> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
>>
>> By "does not yet support" do you mean doesn't work with or merely not
>> tested with but seems to work OK?  if the former, then surely this
>> should be a fixes patch with a cc to stable for all the kernels async
>> abort is in (which is 3.14)
> 
> I'm pretty sure this patch is incorrect, and that what ever issue should
> have been described in this patch are the same EH problems we fixed in
> the last couple releases.  I've also talked to Hannes a while ago, and
> we agree that we want to remove this flag ASAP as it's inviting this

I think we can likely drop the first in the series here. I've reverted it
and have been running abort testing for a bit and haven't hit any issues.
Unless I see anything overnight, I'll resend the second patch by itself and
cc stable.

Thanks,

Brian

-- 
Brian King
Power Linux I/O
IBM Linux Technology Center

Re: [PATCH 1/2] ipr: Disable async aborts

[Christoph Hellwig]

On Wed, Oct 29, 2014 at 04:16:08PM -0500, Brian King wrote:
> I think we can likely drop the first in the series here. I've reverted it
> and have been running abort testing for a bit and haven't hit any issues.
> Unless I see anything overnight, I'll resend the second patch by itself and
> cc stable.

Thanks, please try to find a reviewer for it as well.

Re: [PATCH 1/2] ipr: Disable async aborts

[Brian King]

On 10/27/2014 09:50 AM, James Bottomley wrote:
> On Mon, 2014-10-27 at 09:37 -0500, Brian King wrote:
>> The ipr driver does not yet support async aborts, so disable this
>> for now.
>>
>> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> 
> By "does not yet support" do you mean doesn't work with or merely not
> tested with but seems to work OK?  if the former, then surely this
> should be a fixes patch with a cc to stable for all the kernels async
> abort is in (which is 3.14)

We were seeing some issues in our lab in abort paths with double completions.
This was the first change I made to help resolve this. Patch 2 is definitely
required and the first patch is more for sanity until we can do more abort
path regression testing. What I'd like to do is queue both of these patches
for fixes / stable, then run additional regression testing with async aborts
and re-enable it in a future kernel once I'm more comfortable with it.

If it simplifies things, I can resend the series and cc stable.

Thanks,

Brian


> 
> James
> 
>> ---
>>
>>  drivers/scsi/ipr.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff -puN drivers/scsi/ipr.c~ipr_no_async_eh drivers/scsi/ipr.c
>> --- linux/drivers/scsi/ipr.c~ipr_no_async_eh	2014-10-24 08:57:53.169169509 -0500
>> +++ linux-bjking1/drivers/scsi/ipr.c	2014-10-24 08:57:53.175169579 -0500
>> @@ -6357,6 +6357,7 @@ static struct scsi_host_template driver_
>>  	.sdev_attrs = ipr_dev_attrs,
>>  	.proc_name = IPR_NAME,
>>  	.no_write_same = 1,
>> +	.no_async_abort = 1,
>>  };
>>  
>>  /**
>> _
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 


-- 
Brian King
Power Linux I/O
IBM Linux Technology Center