Re: [PATCH 7/7] scsi_error: streamline scsi_eh_bus_device_reset()

[Bart Van Assche <bvanassche@acm.org>]

On 10/2/23 08:59, Hannes Reinecke wrote:
> Streamline to use a similar code flow as the other reset functions.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/scsi/scsi_error.c | 26 +++++++++++---------------
>   1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 21d84940c9cb..81b38f5da3b6 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -1581,6 +1581,7 @@ static int scsi_eh_bus_device_reset(struct Scsi_Host *shost,
>   {
>   	struct scsi_cmnd *scmd, *bdr_scmd, *next;
>   	struct scsi_device *sdev;
> +	LIST_HEAD(check_list);
>   	enum scsi_disposition rtn;
>   
>   	shost_for_each_device(sdev, shost) {
> @@ -1606,27 +1607,22 @@ static int scsi_eh_bus_device_reset(struct Scsi_Host *shost,
>   			sdev_printk(KERN_INFO, sdev,
>   				     "%s: Sending BDR\n", current->comm));
>   		rtn = scsi_try_bus_device_reset(sdev);
> -		if (rtn == SUCCESS || rtn == FAST_IO_FAIL) {
> -			if (!scsi_device_online(sdev) ||
> -			    rtn == FAST_IO_FAIL ||
> -			    !scsi_eh_tur(bdr_scmd)) {
> -				list_for_each_entry_safe(scmd, next,
> -							 work_q, eh_entry) {
> -					if (scmd->device == sdev &&
> -					    scsi_eh_action(scmd, rtn) != FAILED)
> -						__scsi_eh_finish_cmd(scmd,
> -								     done_q,
> -								     DID_RESET);
> -				}
> -			}
> -		} else {
> +		if (rtn != SUCCESS && rtn != FAST_IO_FAIL)
>   			SCSI_LOG_ERROR_RECOVERY(3,
>   				sdev_printk(KERN_INFO, sdev,
>   					    "%s: BDR failed\n", current->comm));
> +		list_for_each_entry_safe(scmd, next, work_q, eh_entry) {
> +			if (scmd->device != sdev)
> +				continue;
> +			if (rtn == SUCCESS)
> +				list_move_tail(&scmd->eh_entry, &check_list);
> +			else if (rtn == FAST_IO_FAIL)
> +				__scsi_eh_finish_cmd(scmd, done_q,
> +						     DID_TRANSPORT_DISRUPTED);
>   		}
>   	}
>   
> -	return list_empty(work_q);
> +	return scsi_eh_test_devices(&check_list, work_q, done_q, 0);
>   }

I think the description of this patch is too brief. The following 
information is missing from the patch description:
- Why the scsi_device_online() and scsi_eh_tur() checks have been left
   out.
- Why DID_RESET has been changed into DID_TRANSPORT_DISRUPTED.
- Why the list_move_tail() call has been introduced.
- Why the scsi_eh_test_devices() call has been introduced.

Thanks,

Bart.