[v2,0/2] Fix scsi device's iodone_cnt mismatch with iorequest_cnt

Message

Wenchao Hao Oct. 21, 2022, 11:56 p.m. UTC

Following scenario would make scsi_device's iodone_cnt mismatch with
iorequest_cnt even if there is no request on this device any more.

1. request timeout happened. If we do not retry the timeouted command,
   this command would be finished in scsi_finish_command() which would
   not increase the iodone_cnt; if the timeouted command is retried,
   another increasement for iorequest_cnt would be performed, the
   command might add iorequest_cnt for multiple times but iodone_cnt
   only once. Increase iodone_cnt in scsi_timeout() can handle this
   scenario.

2. scsi_dispatch_cmd() failed, while the iorequest_cnt has already been
   increased. If scsi_dispatch_cmd() failed, the request would be
   requeued, then another iorequest_cnt would be added. So we should not
   increase iorequest_cnt if dispatch command failed

V2:
- Add description about why we can add iodone_cnt in scsi_timeout()
- Do not increase iorequest_cnt if dispatch command failed

Wenchao Hao (2):
  scsi: increase scsi device's iodone_cnt in scsi_timeout()
  scsi: donot increase scsi_device's iorequest_cnt if dispatch failed

 drivers/scsi/scsi_error.c | 1 +
 drivers/scsi/scsi_lib.c   | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Wenchao Hao Nov. 8, 2022, 3:08 a.m. UTC | #1

On 2022/10/22 7:56, Wenchao Hao wrote:
> Following scenario would make scsi_device's iodone_cnt mismatch with
> iorequest_cnt even if there is no request on this device any more.
> 
> 1. request timeout happened. If we do not retry the timeouted command,
>    this command would be finished in scsi_finish_command() which would
>    not increase the iodone_cnt; if the timeouted command is retried,
>    another increasement for iorequest_cnt would be performed, the
>    command might add iorequest_cnt for multiple times but iodone_cnt
>    only once. Increase iodone_cnt in scsi_timeout() can handle this
>    scenario.
> 
> 2. scsi_dispatch_cmd() failed, while the iorequest_cnt has already been
>    increased. If scsi_dispatch_cmd() failed, the request would be
>    requeued, then another iorequest_cnt would be added. So we should not
>    increase iorequest_cnt if dispatch command failed
> 
> V2:
> - Add description about why we can add iodone_cnt in scsi_timeout()
> - Do not increase iorequest_cnt if dispatch command failed
> 
> Wenchao Hao (2):
>   scsi: increase scsi device's iodone_cnt in scsi_timeout()
>   scsi: donot increase scsi_device's iorequest_cnt if dispatch failed
> 
>  drivers/scsi/scsi_error.c | 1 +
>  drivers/scsi/scsi_lib.c   | 3 +--
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

Friendly ping...

Wenchao Hao Nov. 21, 2022, 2:38 p.m. UTC | #2

On 2022/10/22 7:56, Wenchao Hao wrote:
> Following scenario would make scsi_device's iodone_cnt mismatch with
> iorequest_cnt even if there is no request on this device any more.
> 
> 1. request timeout happened. If we do not retry the timeouted command,
>    this command would be finished in scsi_finish_command() which would
>    not increase the iodone_cnt; if the timeouted command is retried,
>    another increasement for iorequest_cnt would be performed, the
>    command might add iorequest_cnt for multiple times but iodone_cnt
>    only once. Increase iodone_cnt in scsi_timeout() can handle this
>    scenario.
> 
> 2. scsi_dispatch_cmd() failed, while the iorequest_cnt has already been
>    increased. If scsi_dispatch_cmd() failed, the request would be
>    requeued, then another iorequest_cnt would be added. So we should not
>    increase iorequest_cnt if dispatch command failed
> 
> V2:
> - Add description about why we can add iodone_cnt in scsi_timeout()
> - Do not increase iorequest_cnt if dispatch command failed
> 
> Wenchao Hao (2):
>   scsi: increase scsi device's iodone_cnt in scsi_timeout()
>   scsi: donot increase scsi_device's iorequest_cnt if dispatch failed
> 
>  drivers/scsi/scsi_error.c | 1 +
>  drivers/scsi/scsi_lib.c   | 3 +--
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

Hi Martin, these two count is useful for us, would like take a look?