| Message ID | 20220815164324.645550-1-haowenchao@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scsi: increase scsi device's iodone_cnt in scsi_timeout() | expand |
On 8/15/22 18:43, Wenchao Hao wrote: > The iodone_cnt might be less than iorequest_cnt because > we did not increase the iodone_cnt when a command is done > from timeout. > > Signed-off-by: Wenchao Hao <haowenchao@huawei.com> > --- > drivers/scsi/scsi_error.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > index 448748e3fba5..d21ae0090166 100644 > --- a/drivers/scsi/scsi_error.c > +++ b/drivers/scsi/scsi_error.c > @@ -355,6 +355,7 @@ enum blk_eh_timer_return scsi_timeout(struct request *req) > */ > if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state)) > return BLK_EH_RESET_TIMER; > + atomic_inc(&scmd->device->iodone_cnt); > if (scsi_abort_command(scmd) != SUCCESS) { > set_host_byte(scmd, DID_TIME_OUT); > scsi_eh_scmd_add(scmd); Not sure, but can't we still get a (late) regular completion even after the timeout happened (double accounting) and before we successfully aborted the command?
On Tue, Aug 16, 2022 at 12:53 AM Steffen Maier <maier@linux.ibm.com> wrote: > > On 8/15/22 18:43, Wenchao Hao wrote: > > The iodone_cnt might be less than iorequest_cnt because > > we did not increase the iodone_cnt when a command is done > > from timeout. > > > > Signed-off-by: Wenchao Hao <haowenchao@huawei.com> > > --- > > drivers/scsi/scsi_error.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > > index 448748e3fba5..d21ae0090166 100644 > > --- a/drivers/scsi/scsi_error.c > > +++ b/drivers/scsi/scsi_error.c > > @@ -355,6 +355,7 @@ enum blk_eh_timer_return scsi_timeout(struct request *req) > > */ > > if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state)) > > return BLK_EH_RESET_TIMER; > > + atomic_inc(&scmd->device->iodone_cnt); > > if (scsi_abort_command(scmd) != SUCCESS) { > > set_host_byte(scmd, DID_TIME_OUT); > > scsi_eh_scmd_add(scmd); > > Not sure, but can't we still get a (late) regular completion even after the > timeout happened (double accounting) and before we successfully aborted the > command? > > The flag SCMD_STATE_COMPLETE would take care of it, this would make sure a scsi command is done from timeout or regular completion exclusively. When scsi_abort_command() begins to handle a command, the command would be finished by scsi_finish_command() or retried by requeue function. If a command finished by scsi_finish_command(), it would not add the iodone_cnt. If a command is requeued, it would call scsi_dispatch_cmd() again and another iorequest_cnt would be added. According to above analysis, I think it's suitable to increase iodone_cnt here.
On 2022/8/16 0:43, Wenchao Hao wrote: > The iodone_cnt might be less than iorequest_cnt because > we did not increase the iodone_cnt when a command is done > from timeout. > > Signed-off-by: Wenchao Hao <haowenchao@huawei.com> > --- > drivers/scsi/scsi_error.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > index 448748e3fba5..d21ae0090166 100644 > --- a/drivers/scsi/scsi_error.c > +++ b/drivers/scsi/scsi_error.c > @@ -355,6 +355,7 @@ enum blk_eh_timer_return scsi_timeout(struct request *req) > */ > if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state)) > return BLK_EH_RESET_TIMER; > + atomic_inc(&scmd->device->iodone_cnt); > if (scsi_abort_command(scmd) != SUCCESS) { > set_host_byte(scmd, DID_TIME_OUT); > scsi_eh_scmd_add(scmd); Reviewed-by: Wu Bo <wubo40@huawei.com>
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 448748e3fba5..d21ae0090166 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -355,6 +355,7 @@ enum blk_eh_timer_return scsi_timeout(struct request *req) */ if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state)) return BLK_EH_RESET_TIMER; + atomic_inc(&scmd->device->iodone_cnt); if (scsi_abort_command(scmd) != SUCCESS) { set_host_byte(scmd, DID_TIME_OUT); scsi_eh_scmd_add(scmd);
The iodone_cnt might be less than iorequest_cnt because we did not increase the iodone_cnt when a command is done from timeout. Signed-off-by: Wenchao Hao <haowenchao@huawei.com> --- drivers/scsi/scsi_error.c | 1 + 1 file changed, 1 insertion(+)