mbox series

[v3,00/35] Allow scsi_execute users to control retries

Message ID 20221003175321.8040-1-michael.christie@oracle.com
Headers show
Series Allow scsi_execute users to control retries | expand

Message

Mike Christie Oct. 3, 2022, 5:52 p.m. UTC 
The following patches made over a combo of linus's tree and Martin's
6.1-queue tree (they are both missing patches so I couldn't build
against just one) allow scsi_execute* users to control exactly which
errors are retried, so we can reduce the sense/sshdr handling they have
to do.

The patches allow scsi_execute* users to pass in an array of failures
which they want retried and also specify how many times they want them
retried. If we hit an error that the user did not specify then we drop
down to the default behavior. This allows us to remove almost all the
retry logic from scsi_execute* users.

We then only need to drive retries from the caller for:

1. wants to sleep between retries or had strict timings like in
sd_spinup_disk or ufs.
2. needed to set some internal state between retries like in
scsi_test_unit_ready)
3. retried based on the error code and it's internal state like in the
alua rtpg handling.

v3:
- Use a for loop in scsi_check_passthrough
- Fix result handling/testing.
- Fix scsi_status_is_good handling.
- make __scsi_exec_req take a const arg
- Fix formatting in patch 24

v2:
- Rename scsi_prep_sense
- Change scsi_check_passthrough's loop and added some fixes
- Modified scsi_execute* so it uses a struct to pass in args

Comments

Bart Van Assche Oct. 4, 2022, 10:29 p.m. UTC | #1 
On 10/3/22 10:52, Mike Christie wrote:
> This begins to move the SCSI execution functions to use a struct for
> passing in args. This patch adds the new struct, converts the low level
> helpers and then adds a new helper the next patches will convert the rest
> of the code to.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Bart Van Assche Oct. 4, 2022, 10:36 p.m. UTC | #2 
On 10/3/22 10:52, Mike Christie wrote:
> scsi_execute* is going to be removed. Convert to scsi_exec_req so
> we pass all args in a scsi_exec_args struct.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Bart Van Assche Oct. 4, 2022, 10:45 p.m. UTC | #3 
On 10/3/22 10:52, Mike Christie wrote:
> scsi_execute* is going to be removed. Convert to scsi_exec_req so
> we pass all args in a scsi_exec_args struct.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Bart Van Assche Oct. 4, 2022, 10:49 p.m. UTC | #4 
On 10/3/22 10:53, Mike Christie wrote:
> scsi_execute* is going to be removed. Convert to scsi_exec_req so
> we pass all args in a scsi_exec_args struct.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Bart Van Assche Oct. 4, 2022, 11:44 p.m. UTC | #5 
On 10/3/22 10:53, Mike Christie wrote:
> +	unsigned char cmd[10] = { SYNCHRONIZE_CACHE };

Can cmd[] be declared static const?

Thanks,

Bart.
Bart Van Assche Oct. 4, 2022, 11:47 p.m. UTC | #6 
On 10/3/22 10:53, Mike Christie wrote:
> +	memset(cmd, 0, 16);
> +	cmd[0] = SERVICE_ACTION_IN_16;
> +	cmd[1] = SAI_READ_CAPACITY_16;
> +	cmd[13] = RC16_LEN;

Can the cmd[] declaration be changed into a static const array declaration?

Thanks,

Bart.