From: Martin Wilck [mailto:mwilck@xxxxxxxx] Subject: Re: [PATCH V3 21/25] smartpqi: add additional logging for LUN resets The patch description is not complete, as the patch also changes some timings. Two remarks below. Cheers, Martin > --- > drivers/scsi/smartpqi/smartpqi_init.c | 125 > +++++++++++++++++++++++---------- > 1 file changed, 89 insertions(+), 36 deletions(-) > > diff --git a/drivers/scsi/smartpqi/smartpqi_init.c > b/drivers/scsi/smartpqi/smartpqi_init.c > index 6b624413c8e6..1c51a59f1da6 100644 > --- a/drivers/scsi/smartpqi/smartpqi_init.c > +++ b/drivers/scsi/smartpqi/smartpqi_init.c > @@ -84,7 +84,7 @@ static void pqi_ofa_setup_host_buffer(struct > pqi_ctrl_info *ctrl_info); static void > pqi_ofa_free_host_buffer(struct pqi_ctrl_info *ctrl_info); static int > pqi_ofa_host_memory_update(struct pqi_ctrl_info *ctrl_info); static > int pqi_device_wait_for_pending_io(struct pqi_ctrl_info *ctrl_info, > - struct pqi_scsi_dev *device, unsigned long timeout_secs); > + struct pqi_scsi_dev *device, unsigned long timeout_msecs); > > /* for flags argument to pqi_submit_raid_request_synchronous() */ > #define PQI_SYNC_FLAGS_INTERRUPTABLE 0x1 > @@ -335,11 +335,34 @@ static void pqi_wait_if_ctrl_blocked(struct > pqi_ctrl_info *ctrl_info) > atomic_dec(&ctrl_info->num_blocked_threads); > } > > +#define PQI_QUIESE_WARNING_TIMEOUT_SECS 10 Did you mean QUIESCE ? Don: Yes, corrected. Thank-you. > > pqi_start_io(ctrl_info, &ctrl_info- > >queue_groups[PQI_DEFAULT_QUEUE_GROUP], RAID_PATH, > io_request); > @@ -5958,29 +6007,33 @@ static int pqi_lun_reset(struct pqi_ctrl_info > *ctrl_info, > return rc; > } > > -#define PQI_LUN_RESET_RETRIES 3 > -#define PQI_LUN_RESET_RETRY_INTERVAL_MSECS 10000 > -#define PQI_LUN_RESET_PENDING_IO_TIMEOUT_SECS 120 > +#define PQI_LUN_RESET_RETRIES 3 > +#define PQI_LUN_RESET_RETRY_INTERVAL_MSECS (10 * 1000) > +#define PQI_LUN_RESET_PENDING_IO_TIMEOUT_MSECS (10 * 60 * > 1000) 10 minutes? Isn't that a bit much? Don: 10 minutes seems to hold true for our worst-case-scenarios. > +#define PQI_LUN_RESET_FAILED_PENDING_IO_TIMEOUT_MSECS (2 * 60 * > 1000) Why wait less long after a failure? Don: If the reset TMF fails, the driver is waiting two more minutes for I/O to be flushed out. After this timeout return a failure if the I/O has not been flushed. In many tests, the I/O eventually returns.
