| Message ID | 20240605091731.3111195-3-haowenchao22@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | SCSI: Fix issues between removing device and error handle | expand |
On 6/5/24 11:17, Wenchao Hao wrote: > shost_for_each_device() would skip devices which is in progress of > removing, so commands of these devices would be ignored in > scsi_eh_prt_fail_stats(). > > Fix this issue by using shost_for_each_device_include_deleted() > to iterate devices in scsi_eh_prt_fail_stats(). > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Wenchao Hao <haowenchao22@gmail.com> > --- > drivers/scsi/scsi_error.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > index 612489afe8d2..a61fd8af3b1f 100644 > --- a/drivers/scsi/scsi_error.c > +++ b/drivers/scsi/scsi_error.c > @@ -409,7 +409,7 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host *shost, > int cmd_cancel = 0; > int devices_failed = 0; > > - shost_for_each_device(sdev, shost) { > + shost_for_each_device_include_deleted(sdev, shost) { > list_for_each_entry(scmd, work_q, eh_entry) { > if (scmd->device == sdev) { > ++total_failures; That is wrong. We should rather add a failure counter to the SCSI host, and have the scsi device increase it every time a failure occurs. Then we can avoid this loop completely. Cheers, Hannes
On 6/12/24 4:34 PM, Hannes Reinecke wrote: > On 6/5/24 11:17, Wenchao Hao wrote: >> shost_for_each_device() would skip devices which is in progress of >> removing, so commands of these devices would be ignored in >> scsi_eh_prt_fail_stats(). >> >> Fix this issue by using shost_for_each_device_include_deleted() >> to iterate devices in scsi_eh_prt_fail_stats(). >> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Wenchao Hao <haowenchao22@gmail.com> >> --- >> drivers/scsi/scsi_error.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c >> index 612489afe8d2..a61fd8af3b1f 100644 >> --- a/drivers/scsi/scsi_error.c >> +++ b/drivers/scsi/scsi_error.c >> @@ -409,7 +409,7 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host *shost, >> int cmd_cancel = 0; >> int devices_failed = 0; >> - shost_for_each_device(sdev, shost) { >> + shost_for_each_device_include_deleted(sdev, shost) { >> list_for_each_entry(scmd, work_q, eh_entry) { >> if (scmd->device == sdev) { >> ++total_failures; > > That is wrong. We should rather add a failure counter to the SCSI host, and have the scsi device increase it every time a failure occurs. > Then we can avoid this loop completely. > This function would print the total failure commands and the number of device like following: scsi host4: Total of 3 commands on 2 devices require eh work Just add a failure counter to the SCSI host can not record the number of devices. > Cheers, > > Hannes
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 612489afe8d2..a61fd8af3b1f 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -409,7 +409,7 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host *shost, int cmd_cancel = 0; int devices_failed = 0; - shost_for_each_device(sdev, shost) { + shost_for_each_device_include_deleted(sdev, shost) { list_for_each_entry(scmd, work_q, eh_entry) { if (scmd->device == sdev) { ++total_failures;
shost_for_each_device() would skip devices which is in progress of removing, so commands of these devices would be ignored in scsi_eh_prt_fail_stats(). Fix this issue by using shost_for_each_device_include_deleted() to iterate devices in scsi_eh_prt_fail_stats(). Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Wenchao Hao <haowenchao22@gmail.com> --- drivers/scsi/scsi_error.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)