Re: [PATCHv3 6/6] scsi: make asynchronous aborts mandatory

2017-03-16 Thread Hannes Reinecke
On 03/15/2017 06:55 PM, Benjamin Block wrote:
> On Wed, Mar 15, 2017 at 02:54:09PM +0100, Hannes Reinecke wrote:
>> On 03/14/2017 06:33 PM, Benjamin Block wrote:
>>> Hello Hannes,
>>>
>>> On Wed, Mar 01, 2017 at 10:15:20AM +0100, Hannes Reinecke wrote:
 There hasn't been any reports for HBAs where asynchronous abort
 would not work, so we should make it mandatory and remove
 the fallback.

 Signed-off-by: Hannes Reinecke 
 Reviewed-by: Johannes Thumshirn 
 Reviewed-by: Bart Van Assche 
 ---
  Documentation/scsi/scsi_eh.txt | 18 --
  drivers/scsi/scsi_error.c  | 81 
 --
  drivers/scsi/scsi_lib.c|  2 +-
  drivers/scsi/scsi_priv.h   |  3 +-
  include/scsi/scsi_host.h   |  5 ---
  5 files changed, 15 insertions(+), 94 deletions(-)

 diff --git a/Documentation/scsi/scsi_eh.txt 
 b/Documentation/scsi/scsi_eh.txt
 index 4edb9c1c..11e447b 100644
 --- a/Documentation/scsi/scsi_eh.txt
 +++ b/Documentation/scsi/scsi_eh.txt
 @@ -70,7 +70,7 @@ with the command.
scmd is requeued to blk queue.

   - otherwise
 -  scsi_eh_scmd_add(scmd, 0) is invoked for the command.  See
 +  scsi_eh_scmd_add(scmd) is invoked for the command.  See
[1-3] for details of this function.


 @@ -103,9 +103,7 @@ function
  eh_timed_out() callback did not handle the command.
Step #2 is taken.

 - 2. If the host supports asynchronous completion (as indicated by the
 -no_async_abort setting in the host template) scsi_abort_command()
 -is invoked to schedule an asynchrous abort.
 + 2. scsi_abort_command() is invoked to schedule an asynchrous abort.
  Asynchronous abort are not invoked for commands which the
  SCSI_EH_ABORT_SCHEDULED flag is set (this indicates that the command
  already had been aborted once, and this is a retry which failed),
 @@ -127,16 +125,13 @@ function

   scmds enter EH via scsi_eh_scmd_add(), which does the following.

 - 1. Turns on scmd->eh_eflags as requested.  It's 0 for error
 -completions and SCSI_EH_CANCEL_CMD for timeouts.
 + 1. Links scmd->eh_entry to shost->eh_cmd_q

 - 2. Links scmd->eh_entry to shost->eh_cmd_q
 + 2. Sets SHOST_RECOVERY bit in shost->shost_state

 - 3. Sets SHOST_RECOVERY bit in shost->shost_state
 + 3. Increments shost->host_failed

 - 4. Increments shost->host_failed
 -
 - 5. Wakes up SCSI EH thread if shost->host_busy == shost->host_failed
 + 4. Wakes up SCSI EH thread if shost->host_busy == shost->host_failed

   As can be seen above, once any scmd is added to shost->eh_cmd_q,
  SHOST_RECOVERY shost_state bit is turned on.  This prevents any new
 @@ -252,7 +247,6 @@ scmd->allowed.

   1. Error completion / time out
  ACTION: scsi_eh_scmd_add() is invoked for scmd
 -  - set scmd->eh_eflags
- add scmd to shost->eh_cmd_q
- set SHOST_RECOVERY
- shost->host_failed++
 diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
 index 802a081..7b70ee9 100644
 --- a/drivers/scsi/scsi_error.c
 +++ b/drivers/scsi/scsi_error.c
 @@ -163,7 +163,7 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host 
 *shost)
}
}

 -  scsi_eh_scmd_add(scmd, 0);
 +  scsi_eh_scmd_add(scmd);
  }

  /**
 @@ -217,9 +217,8 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host 
 *shost)
  /**
   * scsi_eh_scmd_add - add scsi cmd to error handling.
   * @scmd: scmd to run eh on.
 - * @eh_flag:  optional SCSI_EH flag.
   */
 -void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
 +void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
  {
struct Scsi_Host *shost = scmd->device->host;
unsigned long flags;
 @@ -235,9 +234,6 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int 
 eh_flag)
if (shost->eh_deadline != -1 && !shost->last_reset)
shost->last_reset = jiffies;

 -  if (scmd->eh_eflags & SCSI_EH_ABORT_SCHEDULED)
 -  eh_flag &= ~SCSI_EH_CANCEL_CMD;
 -  scmd->eh_eflags |= eh_flag;
scsi_eh_reset(scmd);
list_add_tail(>eh_entry, >eh_cmd_q);
shost->host_failed++;
 @@ -271,10 +267,9 @@ enum blk_eh_timer_return scsi_times_out(struct 
 request *req)
rtn = host->hostt->eh_timed_out(scmd);

if (rtn == BLK_EH_NOT_HANDLED) {
 -  if (host->hostt->no_async_abort ||
 -  scsi_abort_command(scmd) != SUCCESS) {
 +  if (scsi_abort_command(scmd) != SUCCESS) {
set_host_byte(scmd, DID_TIME_OUT);
 -  scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD);
 + 

Re: [PATCHv3 6/6] scsi: make asynchronous aborts mandatory

2017-03-15 Thread Benjamin Block
On Wed, Mar 15, 2017 at 02:54:09PM +0100, Hannes Reinecke wrote:
> On 03/14/2017 06:33 PM, Benjamin Block wrote:
> > Hello Hannes,
> >
> > On Wed, Mar 01, 2017 at 10:15:20AM +0100, Hannes Reinecke wrote:
> >> There hasn't been any reports for HBAs where asynchronous abort
> >> would not work, so we should make it mandatory and remove
> >> the fallback.
> >>
> >> Signed-off-by: Hannes Reinecke 
> >> Reviewed-by: Johannes Thumshirn 
> >> Reviewed-by: Bart Van Assche 
> >> ---
> >>  Documentation/scsi/scsi_eh.txt | 18 --
> >>  drivers/scsi/scsi_error.c  | 81 
> >> --
> >>  drivers/scsi/scsi_lib.c|  2 +-
> >>  drivers/scsi/scsi_priv.h   |  3 +-
> >>  include/scsi/scsi_host.h   |  5 ---
> >>  5 files changed, 15 insertions(+), 94 deletions(-)
> >>
> >> diff --git a/Documentation/scsi/scsi_eh.txt 
> >> b/Documentation/scsi/scsi_eh.txt
> >> index 4edb9c1c..11e447b 100644
> >> --- a/Documentation/scsi/scsi_eh.txt
> >> +++ b/Documentation/scsi/scsi_eh.txt
> >> @@ -70,7 +70,7 @@ with the command.
> >>scmd is requeued to blk queue.
> >>
> >>   - otherwise
> >> -  scsi_eh_scmd_add(scmd, 0) is invoked for the command.  See
> >> +  scsi_eh_scmd_add(scmd) is invoked for the command.  See
> >>[1-3] for details of this function.
> >>
> >>
> >> @@ -103,9 +103,7 @@ function
> >>  eh_timed_out() callback did not handle the command.
> >>Step #2 is taken.
> >>
> >> - 2. If the host supports asynchronous completion (as indicated by the
> >> -no_async_abort setting in the host template) scsi_abort_command()
> >> -is invoked to schedule an asynchrous abort.
> >> + 2. scsi_abort_command() is invoked to schedule an asynchrous abort.
> >>  Asynchronous abort are not invoked for commands which the
> >>  SCSI_EH_ABORT_SCHEDULED flag is set (this indicates that the command
> >>  already had been aborted once, and this is a retry which failed),
> >> @@ -127,16 +125,13 @@ function
> >>
> >>   scmds enter EH via scsi_eh_scmd_add(), which does the following.
> >>
> >> - 1. Turns on scmd->eh_eflags as requested.  It's 0 for error
> >> -completions and SCSI_EH_CANCEL_CMD for timeouts.
> >> + 1. Links scmd->eh_entry to shost->eh_cmd_q
> >>
> >> - 2. Links scmd->eh_entry to shost->eh_cmd_q
> >> + 2. Sets SHOST_RECOVERY bit in shost->shost_state
> >>
> >> - 3. Sets SHOST_RECOVERY bit in shost->shost_state
> >> + 3. Increments shost->host_failed
> >>
> >> - 4. Increments shost->host_failed
> >> -
> >> - 5. Wakes up SCSI EH thread if shost->host_busy == shost->host_failed
> >> + 4. Wakes up SCSI EH thread if shost->host_busy == shost->host_failed
> >>
> >>   As can be seen above, once any scmd is added to shost->eh_cmd_q,
> >>  SHOST_RECOVERY shost_state bit is turned on.  This prevents any new
> >> @@ -252,7 +247,6 @@ scmd->allowed.
> >>
> >>   1. Error completion / time out
> >>  ACTION: scsi_eh_scmd_add() is invoked for scmd
> >> -  - set scmd->eh_eflags
> >>- add scmd to shost->eh_cmd_q
> >>- set SHOST_RECOVERY
> >>- shost->host_failed++
> >> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> >> index 802a081..7b70ee9 100644
> >> --- a/drivers/scsi/scsi_error.c
> >> +++ b/drivers/scsi/scsi_error.c
> >> @@ -163,7 +163,7 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host 
> >> *shost)
> >>}
> >>}
> >>
> >> -  scsi_eh_scmd_add(scmd, 0);
> >> +  scsi_eh_scmd_add(scmd);
> >>  }
> >>
> >>  /**
> >> @@ -217,9 +217,8 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host 
> >> *shost)
> >>  /**
> >>   * scsi_eh_scmd_add - add scsi cmd to error handling.
> >>   * @scmd: scmd to run eh on.
> >> - * @eh_flag:  optional SCSI_EH flag.
> >>   */
> >> -void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
> >> +void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
> >>  {
> >>struct Scsi_Host *shost = scmd->device->host;
> >>unsigned long flags;
> >> @@ -235,9 +234,6 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int 
> >> eh_flag)
> >>if (shost->eh_deadline != -1 && !shost->last_reset)
> >>shost->last_reset = jiffies;
> >>
> >> -  if (scmd->eh_eflags & SCSI_EH_ABORT_SCHEDULED)
> >> -  eh_flag &= ~SCSI_EH_CANCEL_CMD;
> >> -  scmd->eh_eflags |= eh_flag;
> >>scsi_eh_reset(scmd);
> >>list_add_tail(>eh_entry, >eh_cmd_q);
> >>shost->host_failed++;
> >> @@ -271,10 +267,9 @@ enum blk_eh_timer_return scsi_times_out(struct 
> >> request *req)
> >>rtn = host->hostt->eh_timed_out(scmd);
> >>
> >>if (rtn == BLK_EH_NOT_HANDLED) {
> >> -  if (host->hostt->no_async_abort ||
> >> -  scsi_abort_command(scmd) != SUCCESS) {
> >> +  if (scsi_abort_command(scmd) != SUCCESS) {
> >>set_host_byte(scmd, DID_TIME_OUT);
> >> -  scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD);
> >> +  scsi_eh_scmd_add(scmd);
> >>

Re: [PATCHv3 6/6] scsi: make asynchronous aborts mandatory

2017-03-15 Thread Hannes Reinecke
On 03/14/2017 06:33 PM, Benjamin Block wrote:
> Hello Hannes,
> 
> On Wed, Mar 01, 2017 at 10:15:20AM +0100, Hannes Reinecke wrote:
>> There hasn't been any reports for HBAs where asynchronous abort
>> would not work, so we should make it mandatory and remove
>> the fallback.
>>
>> Signed-off-by: Hannes Reinecke 
>> Reviewed-by: Johannes Thumshirn 
>> Reviewed-by: Bart Van Assche 
>> ---
>>  Documentation/scsi/scsi_eh.txt | 18 --
>>  drivers/scsi/scsi_error.c  | 81 
>> --
>>  drivers/scsi/scsi_lib.c|  2 +-
>>  drivers/scsi/scsi_priv.h   |  3 +-
>>  include/scsi/scsi_host.h   |  5 ---
>>  5 files changed, 15 insertions(+), 94 deletions(-)
>>
>> diff --git a/Documentation/scsi/scsi_eh.txt b/Documentation/scsi/scsi_eh.txt
>> index 4edb9c1c..11e447b 100644
>> --- a/Documentation/scsi/scsi_eh.txt
>> +++ b/Documentation/scsi/scsi_eh.txt
>> @@ -70,7 +70,7 @@ with the command.
>>  scmd is requeued to blk queue.
>>
>>   - otherwise
>> -scsi_eh_scmd_add(scmd, 0) is invoked for the command.  See
>> +scsi_eh_scmd_add(scmd) is invoked for the command.  See
>>  [1-3] for details of this function.
>>
>>
>> @@ -103,9 +103,7 @@ function
>>  eh_timed_out() callback did not handle the command.
>>  Step #2 is taken.
>>
>> - 2. If the host supports asynchronous completion (as indicated by the
>> -no_async_abort setting in the host template) scsi_abort_command()
>> -is invoked to schedule an asynchrous abort.
>> + 2. scsi_abort_command() is invoked to schedule an asynchrous abort.
>>  Asynchronous abort are not invoked for commands which the
>>  SCSI_EH_ABORT_SCHEDULED flag is set (this indicates that the command
>>  already had been aborted once, and this is a retry which failed),
>> @@ -127,16 +125,13 @@ function
>>
>>   scmds enter EH via scsi_eh_scmd_add(), which does the following.
>>
>> - 1. Turns on scmd->eh_eflags as requested.  It's 0 for error
>> -completions and SCSI_EH_CANCEL_CMD for timeouts.
>> + 1. Links scmd->eh_entry to shost->eh_cmd_q
>>
>> - 2. Links scmd->eh_entry to shost->eh_cmd_q
>> + 2. Sets SHOST_RECOVERY bit in shost->shost_state
>>
>> - 3. Sets SHOST_RECOVERY bit in shost->shost_state
>> + 3. Increments shost->host_failed
>>
>> - 4. Increments shost->host_failed
>> -
>> - 5. Wakes up SCSI EH thread if shost->host_busy == shost->host_failed
>> + 4. Wakes up SCSI EH thread if shost->host_busy == shost->host_failed
>>
>>   As can be seen above, once any scmd is added to shost->eh_cmd_q,
>>  SHOST_RECOVERY shost_state bit is turned on.  This prevents any new
>> @@ -252,7 +247,6 @@ scmd->allowed.
>>
>>   1. Error completion / time out
>>  ACTION: scsi_eh_scmd_add() is invoked for scmd
>> -- set scmd->eh_eflags
>>  - add scmd to shost->eh_cmd_q
>>  - set SHOST_RECOVERY
>>  - shost->host_failed++
>> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
>> index 802a081..7b70ee9 100644
>> --- a/drivers/scsi/scsi_error.c
>> +++ b/drivers/scsi/scsi_error.c
>> @@ -163,7 +163,7 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host 
>> *shost)
>>  }
>>  }
>>
>> -scsi_eh_scmd_add(scmd, 0);
>> +scsi_eh_scmd_add(scmd);
>>  }
>>
>>  /**
>> @@ -217,9 +217,8 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host 
>> *shost)
>>  /**
>>   * scsi_eh_scmd_add - add scsi cmd to error handling.
>>   * @scmd:   scmd to run eh on.
>> - * @eh_flag:optional SCSI_EH flag.
>>   */
>> -void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
>> +void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
>>  {
>>  struct Scsi_Host *shost = scmd->device->host;
>>  unsigned long flags;
>> @@ -235,9 +234,6 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int 
>> eh_flag)
>>  if (shost->eh_deadline != -1 && !shost->last_reset)
>>  shost->last_reset = jiffies;
>>
>> -if (scmd->eh_eflags & SCSI_EH_ABORT_SCHEDULED)
>> -eh_flag &= ~SCSI_EH_CANCEL_CMD;
>> -scmd->eh_eflags |= eh_flag;
>>  scsi_eh_reset(scmd);
>>  list_add_tail(>eh_entry, >eh_cmd_q);
>>  shost->host_failed++;
>> @@ -271,10 +267,9 @@ enum blk_eh_timer_return scsi_times_out(struct request 
>> *req)
>>  rtn = host->hostt->eh_timed_out(scmd);
>>
>>  if (rtn == BLK_EH_NOT_HANDLED) {
>> -if (host->hostt->no_async_abort ||
>> -scsi_abort_command(scmd) != SUCCESS) {
>> +if (scsi_abort_command(scmd) != SUCCESS) {
>>  set_host_byte(scmd, DID_TIME_OUT);
>> -scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD);
>> +scsi_eh_scmd_add(scmd);
>>  }
>>  }
>>
>> @@ -327,7 +322,7 @@ static inline void scsi_eh_prt_fail_stats(struct 
>> Scsi_Host *shost,
>>  list_for_each_entry(scmd, work_q, eh_entry) {
>>  if (scmd->device == sdev) {
>> 

Re: [PATCHv3 6/6] scsi: make asynchronous aborts mandatory

2017-03-14 Thread Benjamin Block
Hello Hannes,

On Wed, Mar 01, 2017 at 10:15:20AM +0100, Hannes Reinecke wrote:
> There hasn't been any reports for HBAs where asynchronous abort
> would not work, so we should make it mandatory and remove
> the fallback.
> 
> Signed-off-by: Hannes Reinecke 
> Reviewed-by: Johannes Thumshirn 
> Reviewed-by: Bart Van Assche 
> ---
>  Documentation/scsi/scsi_eh.txt | 18 --
>  drivers/scsi/scsi_error.c  | 81 
> --
>  drivers/scsi/scsi_lib.c|  2 +-
>  drivers/scsi/scsi_priv.h   |  3 +-
>  include/scsi/scsi_host.h   |  5 ---
>  5 files changed, 15 insertions(+), 94 deletions(-)
> 
> diff --git a/Documentation/scsi/scsi_eh.txt b/Documentation/scsi/scsi_eh.txt
> index 4edb9c1c..11e447b 100644
> --- a/Documentation/scsi/scsi_eh.txt
> +++ b/Documentation/scsi/scsi_eh.txt
> @@ -70,7 +70,7 @@ with the command.
>   scmd is requeued to blk queue.
> 
>   - otherwise
> - scsi_eh_scmd_add(scmd, 0) is invoked for the command.  See
> + scsi_eh_scmd_add(scmd) is invoked for the command.  See
>   [1-3] for details of this function.
> 
> 
> @@ -103,9 +103,7 @@ function
>  eh_timed_out() callback did not handle the command.
>   Step #2 is taken.
> 
> - 2. If the host supports asynchronous completion (as indicated by the
> -no_async_abort setting in the host template) scsi_abort_command()
> -is invoked to schedule an asynchrous abort.
> + 2. scsi_abort_command() is invoked to schedule an asynchrous abort.
>  Asynchronous abort are not invoked for commands which the
>  SCSI_EH_ABORT_SCHEDULED flag is set (this indicates that the command
>  already had been aborted once, and this is a retry which failed),
> @@ -127,16 +125,13 @@ function
> 
>   scmds enter EH via scsi_eh_scmd_add(), which does the following.
> 
> - 1. Turns on scmd->eh_eflags as requested.  It's 0 for error
> -completions and SCSI_EH_CANCEL_CMD for timeouts.
> + 1. Links scmd->eh_entry to shost->eh_cmd_q
> 
> - 2. Links scmd->eh_entry to shost->eh_cmd_q
> + 2. Sets SHOST_RECOVERY bit in shost->shost_state
> 
> - 3. Sets SHOST_RECOVERY bit in shost->shost_state
> + 3. Increments shost->host_failed
> 
> - 4. Increments shost->host_failed
> -
> - 5. Wakes up SCSI EH thread if shost->host_busy == shost->host_failed
> + 4. Wakes up SCSI EH thread if shost->host_busy == shost->host_failed
> 
>   As can be seen above, once any scmd is added to shost->eh_cmd_q,
>  SHOST_RECOVERY shost_state bit is turned on.  This prevents any new
> @@ -252,7 +247,6 @@ scmd->allowed.
> 
>   1. Error completion / time out
>  ACTION: scsi_eh_scmd_add() is invoked for scmd
> - - set scmd->eh_eflags
>   - add scmd to shost->eh_cmd_q
>   - set SHOST_RECOVERY
>   - shost->host_failed++
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 802a081..7b70ee9 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -163,7 +163,7 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host 
> *shost)
>   }
>   }
> 
> - scsi_eh_scmd_add(scmd, 0);
> + scsi_eh_scmd_add(scmd);
>  }
> 
>  /**
> @@ -217,9 +217,8 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host 
> *shost)
>  /**
>   * scsi_eh_scmd_add - add scsi cmd to error handling.
>   * @scmd:scmd to run eh on.
> - * @eh_flag: optional SCSI_EH flag.
>   */
> -void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
> +void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
>  {
>   struct Scsi_Host *shost = scmd->device->host;
>   unsigned long flags;
> @@ -235,9 +234,6 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
>   if (shost->eh_deadline != -1 && !shost->last_reset)
>   shost->last_reset = jiffies;
> 
> - if (scmd->eh_eflags & SCSI_EH_ABORT_SCHEDULED)
> - eh_flag &= ~SCSI_EH_CANCEL_CMD;
> - scmd->eh_eflags |= eh_flag;
>   scsi_eh_reset(scmd);
>   list_add_tail(>eh_entry, >eh_cmd_q);
>   shost->host_failed++;
> @@ -271,10 +267,9 @@ enum blk_eh_timer_return scsi_times_out(struct request 
> *req)
>   rtn = host->hostt->eh_timed_out(scmd);
> 
>   if (rtn == BLK_EH_NOT_HANDLED) {
> - if (host->hostt->no_async_abort ||
> - scsi_abort_command(scmd) != SUCCESS) {
> + if (scsi_abort_command(scmd) != SUCCESS) {
>   set_host_byte(scmd, DID_TIME_OUT);
> - scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD);
> + scsi_eh_scmd_add(scmd);
>   }
>   }
> 
> @@ -327,7 +322,7 @@ static inline void scsi_eh_prt_fail_stats(struct 
> Scsi_Host *shost,
>   list_for_each_entry(scmd, work_q, eh_entry) {
>   if (scmd->device == sdev) {
>   ++total_failures;
> - if (scmd->eh_eflags & SCSI_EH_CANCEL_CMD)
> +