On Thu, 2020-12-10 at 14:35 -0600, Don Brace wrote:
> From: Kevin Barnett <kevin.barnett@xxxxxxxxxxxxx>
>
> * synchronize: LUN resets, shutdowns, suspend, hibernate,
> OFA, and controller offline events.
> * prevent I/O during the the above conditions.
This description is too terse for a complex patch like this.
Could you please explain how this synchronization is supposed to work
on the different layers, and for the different code paths for different
types of IO events that are apparently not all handled equally wrt
blocking, and how the different flags and mutexes are supposed to
interact? I'd also appreciate some explanation what sort of "driver
synchronization issues" you have seen, and how exactly this patch is
supposed to fix them.
Please forgive me if I ask dumb questions or make dumb comments below,
I don't get the full picture of what you're trying to achieve.
The patch does not only address synchronization issues; it also changes
various other things that (given the size of the patch) should better
be handled elsewhere. I believe this patch could easily be split into
4 or more separate independent patches, which would ease review
considerably. I've added remarks below where I thought one or more
hunks could be separated out.
Thanks,
Martin
>
> Reviewed-by: Scott Benesh <scott.benesh@xxxxxxxxxxxxx>
> Reviewed-by: Scott Teel <scott.teel@xxxxxxxxxxxxx>
> Signed-off-by: Kevin Barnett <kevin.barnett@xxxxxxxxxxxxx>
> Signed-off-by: Don Brace <don.brace@xxxxxxxxxxxxx>
> ---
> drivers/scsi/smartpqi/smartpqi.h | 48 -
> drivers/scsi/smartpqi/smartpqi_init.c | 1157 +++++++++++++----------
> ----------
> 2 files changed, 474 insertions(+), 731 deletions(-)
>
> diff --git a/drivers/scsi/smartpqi/smartpqi.h
> b/drivers/scsi/smartpqi/smartpqi.h
> index 976bfd8c5192..0b94c755a74c 100644
> --- a/drivers/scsi/smartpqi/smartpqi.h
> +++ b/drivers/scsi/smartpqi/smartpqi.h
> @@ -130,9 +130,12 @@ struct pqi_iu_header {
> /* of this header */
> __le16 response_queue_id; /* specifies the OQ where the
> */
> /* response IU is to be
> delivered */
> - u8 work_area[2]; /* reserved for driver use */
> + u16 driver_flags; /* reserved for driver use */
> };
>
> +/* manifest constants for pqi_iu_header.driver_flags */
> +#define PQI_DRIVER_NONBLOCKABLE_REQUEST 0x1
> +
> /*
> * According to the PQI spec, the IU header is only the first 4
> bytes of our
> * pqi_iu_header structure.
> @@ -508,10 +511,6 @@ struct pqi_vendor_general_response {
> #define PQI_OFA_SIGNATURE "OFA_QRM"
> #define PQI_OFA_MAX_SG_DESCRIPTORS 64
>
> -#define PQI_OFA_MEMORY_DESCRIPTOR_LENGTH \
> - (offsetof(struct pqi_ofa_memory, sg_descriptor) + \
> - (PQI_OFA_MAX_SG_DESCRIPTORS * sizeof(struct
> pqi_sg_descriptor)))
> -
> struct pqi_ofa_memory {
> __le64 signature; /* "OFA_QRM" */
> __le16 version; /* version of this struct (1 = 1st
> version) */
> @@ -519,7 +518,7 @@ struct pqi_ofa_memory {
> __le32 bytes_allocated; /* total allocated memory in
> bytes */
> __le16 num_memory_descriptors;
> u8 reserved1[2];
> - struct pqi_sg_descriptor sg_descriptor[1];
> + struct pqi_sg_descriptor
> sg_descriptor[PQI_OFA_MAX_SG_DESCRIPTORS];
> };
>
> struct pqi_aio_error_info {
> @@ -850,7 +849,8 @@ struct pqi_config_table_firmware_features {
> #define PQI_FIRMWARE_FEATURE_RAID_IU_TIMEOUT 13
> #define PQI_FIRMWARE_FEATURE_TMF_IU_TIMEOUT 14
> #define PQI_FIRMWARE_FEATURE_RAID_BYPASS_ON_ENCRYPTED_NVME 15
> -#define PQI_FIRMWARE_FEATURE_MAXIMUM 15
> +#define PQI_FIRMWARE_FEATURE_UNIQUE_WWID_IN_REPORT_PHYS_LUN 16
> +#define PQI_FIRMWARE_FEATURE_MAXIMUM 16
What does the "unique WWID" feature have to do with synchronization
issues? This part should have gone into a separate patch.
>
> struct pqi_config_table_debug {
> struct pqi_config_table_section_header header;
> @@ -1071,7 +1071,6 @@ struct pqi_scsi_dev {
> u8 volume_offline : 1;
> u8 rescan : 1;
> bool aio_enabled; /* only valid for physical
> disks */
> - bool in_reset;
> bool in_remove;
> bool device_offline;
> u8 vendor[8]; /* bytes 8-15 of inquiry data
> */
> @@ -1107,6 +1106,7 @@ struct pqi_scsi_dev {
> struct pqi_stream_data stream_data[NUM_STREAMS_PER_LUN];
> atomic_t scsi_cmds_outstanding;
> atomic_t raid_bypass_cnt;
> + u8 page_83_identifier[16];
> };
>
> /* VPD inquiry pages */
> @@ -1212,10 +1212,8 @@ struct pqi_io_request {
> struct pqi_event {
> bool pending;
> u8 event_type;
> - __le16 event_id;
> - __le32 additional_event_id;
> - __le32 ofa_bytes_requested;
> - __le16 ofa_cancel_reason;
> + u16 event_id;
> + u32 additional_event_id;
> };
>
> #define PQI_RESERVED_IO_SLOTS_LUN_RESET 1
> @@ -1287,12 +1285,9 @@ struct pqi_ctrl_info {
>
> struct mutex scan_mutex;
> struct mutex lun_reset_mutex;
> - struct mutex ofa_mutex; /* serialize ofa */
> bool controller_online;
> bool block_requests;
> - bool block_device_reset;
> - bool in_ofa;
> - bool in_shutdown;
> + bool scan_blocked;
> u8 inbound_spanning_supported : 1;
> u8 outbound_spanning_supported : 1;
> u8 pqi_mode_enabled : 1;
> @@ -1300,6 +1295,7 @@ struct pqi_ctrl_info {
> u8 soft_reset_handshake_supported : 1;
> u8 raid_iu_timeout_supported : 1;
> u8 tmf_iu_timeout_supported : 1;
> + u8 unique_wwid_in_report_phys_lun_supported : 1;
> u8 enable_r1_writes : 1;
> u8 enable_r5_writes : 1;
> u8 enable_r6_writes : 1;
> @@ -1341,14 +1337,14 @@ struct pqi_ctrl_info {
> atomic_t num_blocked_threads;
> wait_queue_head_t block_requests_wait;
>
> - struct list_head raid_bypass_retry_list;
> - spinlock_t raid_bypass_retry_list_lock;
> - struct work_struct raid_bypass_retry_work;
> -
> + struct mutex ofa_mutex;
> struct pqi_ofa_memory *pqi_ofa_mem_virt_addr;
> dma_addr_t pqi_ofa_mem_dma_handle;
> void **pqi_ofa_chunk_virt_addr;
> - atomic_t sync_cmds_outstanding;
> + struct work_struct ofa_memory_alloc_work;
> + struct work_struct ofa_quiesce_work;
> + u32 ofa_bytes_requested;
> + u16 ofa_cancel_reason;
> };
>
> enum pqi_ctrl_mode {
> @@ -1619,16 +1615,6 @@ struct bmic_diag_options {
>
> #pragma pack()
>
> -static inline void pqi_ctrl_busy(struct pqi_ctrl_info *ctrl_info)
> -{
> - atomic_inc(&ctrl_info->num_busy_threads);
> -}
> -
> -static inline void pqi_ctrl_unbusy(struct pqi_ctrl_info *ctrl_info)
> -{
> - atomic_dec(&ctrl_info->num_busy_threads);
> -}
> -
> static inline struct pqi_ctrl_info *shost_to_hba(struct Scsi_Host
> *shost)
> {
> void *hostdata = shost_priv(shost);
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
> b/drivers/scsi/smartpqi/smartpqi_init.c
> index 1eb677bc6c69..082b17e9bd80 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -45,6 +45,9 @@
>
> #define PQI_EXTRA_SGL_MEMORY (12 * sizeof(struct
> pqi_sg_descriptor))
>
> +#define PQI_POST_RESET_DELAY_SECS 5
> +#define PQI_POST_OFA_RESET_DELAY_UPON_TIMEOUT_SECS 10
> +
> MODULE_AUTHOR("Microsemi");
> MODULE_DESCRIPTION("Driver for Microsemi Smart Family Controller
> version "
> DRIVER_VERSION);
> @@ -54,7 +57,6 @@ MODULE_LICENSE("GPL");
>
> static void pqi_take_ctrl_offline(struct pqi_ctrl_info *ctrl_info);
> static void pqi_ctrl_offline_worker(struct work_struct *work);
> -static void pqi_retry_raid_bypass_requests(struct pqi_ctrl_info
> *ctrl_info);
> static int pqi_scan_scsi_devices(struct pqi_ctrl_info *ctrl_info);
> static void pqi_scan_start(struct Scsi_Host *shost);
> static void pqi_start_io(struct pqi_ctrl_info *ctrl_info,
> @@ -62,7 +64,7 @@ static void pqi_start_io(struct pqi_ctrl_info
> *ctrl_info,
> struct pqi_io_request *io_request);
> static int pqi_submit_raid_request_synchronous(struct pqi_ctrl_info
> *ctrl_info,
> struct pqi_iu_header *request, unsigned int flags,
> - struct pqi_raid_error_info *error_info, unsigned long
> timeout_msecs);
> + struct pqi_raid_error_info *error_info);
> static int pqi_aio_submit_io(struct pqi_ctrl_info *ctrl_info,
> struct scsi_cmnd *scmd, u32 aio_handle, u8 *cdb,
> unsigned int cdb_length, struct pqi_queue_group *queue_group,
> @@ -77,9 +79,8 @@ static int pqi_aio_submit_r56_write_io(struct
> pqi_ctrl_info *ctrl_info,
> struct pqi_scsi_dev_raid_map_data *rmd);
> static void pqi_ofa_ctrl_quiesce(struct pqi_ctrl_info *ctrl_info);
> static void pqi_ofa_ctrl_unquiesce(struct pqi_ctrl_info *ctrl_info);
> -static int pqi_ofa_ctrl_restart(struct pqi_ctrl_info *ctrl_info);
> -static void pqi_ofa_setup_host_buffer(struct pqi_ctrl_info
> *ctrl_info,
> - u32 bytes_requested);
> +static int pqi_ofa_ctrl_restart(struct pqi_ctrl_info *ctrl_info,
> unsigned int delay_secs);
> +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,
> @@ -245,14 +246,66 @@ static inline void pqi_save_ctrl_mode(struct
> pqi_ctrl_info *ctrl_info,
> sis_write_driver_scratch(ctrl_info, mode);
> }
>
> +static inline void pqi_ctrl_block_scan(struct pqi_ctrl_info
> *ctrl_info)
> +{
> + ctrl_info->scan_blocked = true;
> + mutex_lock(&ctrl_info->scan_mutex);
> +}
What do you need scan_blocked for? Can't you simply use
mutex_is_locked(&ctrl_info->scan_mutex)?
OTOH, using a mutex for this kind of condition feels dangerous
to me, see remark about ota_mutex() below.
Have you considered using a completion for this?
> +
> +static inline void pqi_ctrl_unblock_scan(struct pqi_ctrl_info
> *ctrl_info)
> +{
> + ctrl_info->scan_blocked = false;
> + mutex_unlock(&ctrl_info->scan_mutex);
> +}
> +
> +static inline bool pqi_ctrl_scan_blocked(struct pqi_ctrl_info
> *ctrl_info)
> +{
> + return ctrl_info->scan_blocked;
> +}
> +
> static inline void pqi_ctrl_block_device_reset(struct pqi_ctrl_info
> *ctrl_info)
> {
> - ctrl_info->block_device_reset = true;
> + mutex_lock(&ctrl_info->lun_reset_mutex);
> +}
> +
> +static inline void pqi_ctrl_unblock_device_reset(struct
> pqi_ctrl_info *ctrl_info)
> +{
> + mutex_unlock(&ctrl_info->lun_reset_mutex);
> +}
> +
> +static inline void pqi_scsi_block_requests(struct pqi_ctrl_info
> *ctrl_info)
> +{
> + struct Scsi_Host *shost;
> + unsigned int num_loops;
> + int msecs_sleep;
> +
> + shost = ctrl_info->scsi_host;
> +
> + scsi_block_requests(shost);
> +
> + num_loops = 0;
> + msecs_sleep = 20;
> + while (scsi_host_busy(shost)) {
> + num_loops++;
> + if (num_loops == 10)
> + msecs_sleep = 500;
> + msleep(msecs_sleep);
> + }
> +}
Waiting for !scsi_host_busy() here looks like a layering violation to
me. Can't you use wait_event{_timeout}() here and wait for the sum of
of device->scsi_cmds_outstanding over to become zero (waking up the
queue in pqi_prep_for_scsi_done())? You could use the
total_scmnds_outstanding count that you introduce in patch 15/25.
Also, how does this interact/interfere with scsi EH?
> +
> +static inline void pqi_scsi_unblock_requests(struct pqi_ctrl_info
> *ctrl_info)
> +{
> + scsi_unblock_requests(ctrl_info->scsi_host);
> +}
> +
> +static inline void pqi_ctrl_busy(struct pqi_ctrl_info *ctrl_info)
> +{
> + atomic_inc(&ctrl_info->num_busy_threads);
> }
>
> -static inline bool pqi_device_reset_blocked(struct pqi_ctrl_info
> *ctrl_info)
> +static inline void pqi_ctrl_unbusy(struct pqi_ctrl_info *ctrl_info)
> {
> - return ctrl_info->block_device_reset;
> + atomic_dec(&ctrl_info->num_busy_threads);
> }
>
> static inline bool pqi_ctrl_blocked(struct pqi_ctrl_info *ctrl_info)
> @@ -263,44 +316,23 @@ static inline bool pqi_ctrl_blocked(struct
> pqi_ctrl_info *ctrl_info)
> static inline void pqi_ctrl_block_requests(struct pqi_ctrl_info
> *ctrl_info)
> {
> ctrl_info->block_requests = true;
> - scsi_block_requests(ctrl_info->scsi_host);
> }
>
> static inline void pqi_ctrl_unblock_requests(struct pqi_ctrl_info
> *ctrl_info)
> {
> ctrl_info->block_requests = false;
> wake_up_all(&ctrl_info->block_requests_wait);
> - pqi_retry_raid_bypass_requests(ctrl_info);
> - scsi_unblock_requests(ctrl_info->scsi_host);
> }
>
> -static unsigned long pqi_wait_if_ctrl_blocked(struct pqi_ctrl_info
> *ctrl_info,
> - unsigned long timeout_msecs)
> +static void pqi_wait_if_ctrl_blocked(struct pqi_ctrl_info
> *ctrl_info)
> {
> - unsigned long remaining_msecs;
> -
> if (!pqi_ctrl_blocked(ctrl_info))
> - return timeout_msecs;
> + return;
>
> atomic_inc(&ctrl_info->num_blocked_threads);
> -
> - if (timeout_msecs == NO_TIMEOUT) {
> - wait_event(ctrl_info->block_requests_wait,
> - !pqi_ctrl_blocked(ctrl_info));
> - remaining_msecs = timeout_msecs;
> - } else {
> - unsigned long remaining_jiffies;
> -
> - remaining_jiffies =
> - wait_event_timeout(ctrl_info-
> > block_requests_wait,
> - !pqi_ctrl_blocked(ctrl_info),
> - msecs_to_jiffies(timeout_msecs));
> - remaining_msecs =
> jiffies_to_msecs(remaining_jiffies);
> - }
> -
> + wait_event(ctrl_info->block_requests_wait,
> + !pqi_ctrl_blocked(ctrl_info));
> atomic_dec(&ctrl_info->num_blocked_threads);
> -
> - return remaining_msecs;
> }
>
> static inline void pqi_ctrl_wait_until_quiesced(struct pqi_ctrl_info
> *ctrl_info)
> @@ -315,34 +347,25 @@ static inline bool pqi_device_offline(struct
> pqi_scsi_dev *device)
> return device->device_offline;
> }
>
> -static inline void pqi_device_reset_start(struct pqi_scsi_dev
> *device)
> -{
> - device->in_reset = true;
> -}
> -
> -static inline void pqi_device_reset_done(struct pqi_scsi_dev
> *device)
> -{
> - device->in_reset = false;
> -}
> -
> -static inline bool pqi_device_in_reset(struct pqi_scsi_dev *device)
> +static inline void pqi_ctrl_ofa_start(struct pqi_ctrl_info
> *ctrl_info)
> {
> - return device->in_reset;
> + mutex_lock(&ctrl_info->ofa_mutex);
> }
>
> -static inline void pqi_ctrl_ofa_start(struct pqi_ctrl_info
> *ctrl_info)
> +static inline void pqi_ctrl_ofa_done(struct pqi_ctrl_info
> *ctrl_info)
> {
> - ctrl_info->in_ofa = true;
> + mutex_unlock(&ctrl_info->ofa_mutex);
> }
pqi_ctrl_ofa_done() is called in several places. For me, it's non-
obvious whether ofa_mutex is guaranteed to be locked when this happens.
It would be an error to call mutex_unlock() if that's not the case.
Also, is it always guaranteed that "The context (task) that acquired
the lock also releases it"
(https://www.kernel.org/doc/html/latest/locking/locktypes.html)?
If feel that's rather not the case, as pqi_ctrl_ofa_start() is run from
a work queue, whereas pqi_ctrl_ofa_done() is not, afaics.
Have you considered using a completion?
Or can you add some explanatory comments?
> -static inline void pqi_ctrl_ofa_done(struct pqi_ctrl_info
> *ctrl_info)
> +static inline void pqi_wait_until_ofa_finished(struct pqi_ctrl_info
> *ctrl_info)
> {
> - ctrl_info->in_ofa = false;
> + mutex_lock(&ctrl_info->ofa_mutex);
> + mutex_unlock(&ctrl_info->ofa_mutex);
> }
>
> -static inline bool pqi_ctrl_in_ofa(struct pqi_ctrl_info *ctrl_info)
> +static inline bool pqi_ofa_in_progress(struct pqi_ctrl_info
> *ctrl_info)
> {
> - return ctrl_info->in_ofa;
> + return mutex_is_locked(&ctrl_info->ofa_mutex);
> }
>
> static inline void pqi_device_remove_start(struct pqi_scsi_dev
> *device)
> @@ -355,14 +378,20 @@ static inline bool pqi_device_in_remove(struct
> pqi_scsi_dev *device)
> return device->in_remove;
> }
>
> -static inline void pqi_ctrl_shutdown_start(struct pqi_ctrl_info
> *ctrl_info)
> +static inline int pqi_event_type_to_event_index(unsigned int
> event_type)
> {
> - ctrl_info->in_shutdown = true;
> + int index;
> +
> + for (index = 0; index <
> ARRAY_SIZE(pqi_supported_event_types); index++)
> + if (event_type == pqi_supported_event_types[index])
> + return index;
> +
> + return -1;
> }
>
> -static inline bool pqi_ctrl_in_shutdown(struct pqi_ctrl_info
> *ctrl_info)
> +static inline bool pqi_is_supported_event(unsigned int event_type)
> {
> - return ctrl_info->in_shutdown;
> + return pqi_event_type_to_event_index(event_type) != -1;
> }
>
> static inline void pqi_schedule_rescan_worker_with_delay(struct
> pqi_ctrl_info *ctrl_info,
> @@ -370,8 +399,6 @@ static inline void
> pqi_schedule_rescan_worker_with_delay(struct pqi_ctrl_info *c
> {
> if (pqi_ctrl_offline(ctrl_info))
> return;
> - if (pqi_ctrl_in_ofa(ctrl_info))
> - return;
>
> schedule_delayed_work(&ctrl_info->rescan_work, delay);
> }
> @@ -408,22 +435,15 @@ static inline u32
> pqi_read_heartbeat_counter(struct pqi_ctrl_info *ctrl_info)
>
> static inline u8 pqi_read_soft_reset_status(struct pqi_ctrl_info
> *ctrl_info)
> {
> - if (!ctrl_info->soft_reset_status)
> - return 0;
> -
> return readb(ctrl_info->soft_reset_status);
> }
The new treatment of soft_reset_status is unrelated to the
synchronization issues mentioned in the patch description.
>
> -static inline void pqi_clear_soft_reset_status(struct pqi_ctrl_info
> *ctrl_info,
> - u8 clear)
> +static inline void pqi_clear_soft_reset_status(struct pqi_ctrl_info
> *ctrl_info)
> {
> u8 status;
>
> - if (!ctrl_info->soft_reset_status)
> - return;
> -
> status = pqi_read_soft_reset_status(ctrl_info);
> - status &= ~clear;
> + status &= ~PQI_SOFT_RESET_ABORT;
> writeb(status, ctrl_info->soft_reset_status);
> }
>
> @@ -512,6 +532,7 @@ static int pqi_build_raid_path_request(struct
> pqi_ctrl_info *ctrl_info,
> put_unaligned_be32(cdb_length, &cdb[6]);
> break;
> case SA_FLUSH_CACHE:
> + request->header.driver_flags =
> PQI_DRIVER_NONBLOCKABLE_REQUEST;
> request->data_direction = SOP_WRITE_FLAG;
> cdb[0] = BMIC_WRITE;
> cdb[6] = BMIC_FLUSH_CACHE;
> @@ -606,7 +627,7 @@ static void pqi_free_io_request(struct
> pqi_io_request *io_request)
>
> static int pqi_send_scsi_raid_request(struct pqi_ctrl_info
> *ctrl_info, u8 cmd,
> u8 *scsi3addr, void *buffer, size_t buffer_length, u16
> vpd_page,
> - struct pqi_raid_error_info *error_info, unsigned long
> timeout_msecs)
> + struct pqi_raid_error_info *error_info)
> {
> int rc;
> struct pqi_raid_path_request request;
> @@ -618,7 +639,7 @@ static int pqi_send_scsi_raid_request(struct
> pqi_ctrl_info *ctrl_info, u8 cmd,
> return rc;
>
> rc = pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header, 0,
> - error_info, timeout_msecs);
> + error_info);
>
> pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1,
> dir);
>
> @@ -631,7 +652,7 @@ static inline int
> pqi_send_ctrl_raid_request(struct pqi_ctrl_info *ctrl_info,
> u8 cmd, void *buffer, size_t buffer_length)
> {
> return pqi_send_scsi_raid_request(ctrl_info, cmd,
> RAID_CTLR_LUNID,
> - buffer, buffer_length, 0, NULL, NO_TIMEOUT);
> + buffer, buffer_length, 0, NULL);
> }
>
> static inline int pqi_send_ctrl_raid_with_error(struct pqi_ctrl_info
> *ctrl_info,
> @@ -639,7 +660,7 @@ static inline int
> pqi_send_ctrl_raid_with_error(struct pqi_ctrl_info *ctrl_info,
> struct pqi_raid_error_info *error_info)
> {
> return pqi_send_scsi_raid_request(ctrl_info, cmd,
> RAID_CTLR_LUNID,
> - buffer, buffer_length, 0, error_info, NO_TIMEOUT);
> + buffer, buffer_length, 0, error_info);
> }
>
> static inline int pqi_identify_controller(struct pqi_ctrl_info
> *ctrl_info,
> @@ -661,7 +682,7 @@ static inline int pqi_scsi_inquiry(struct
> pqi_ctrl_info *ctrl_info,
> u8 *scsi3addr, u16 vpd_page, void *buffer, size_t
> buffer_length)
> {
> return pqi_send_scsi_raid_request(ctrl_info, INQUIRY,
> scsi3addr,
> - buffer, buffer_length, vpd_page, NULL, NO_TIMEOUT);
> + buffer, buffer_length, vpd_page, NULL);
> }
>
> static int pqi_identify_physical_device(struct pqi_ctrl_info
> *ctrl_info,
> @@ -683,8 +704,7 @@ static int pqi_identify_physical_device(struct
> pqi_ctrl_info *ctrl_info,
> request.cdb[2] = (u8)bmic_device_index;
> request.cdb[9] = (u8)(bmic_device_index >> 8);
>
> - rc = pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header,
> - 0, NULL, NO_TIMEOUT);
> + rc = pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header, 0, NULL);
>
> pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1,
> dir);
>
> @@ -741,7 +761,7 @@ static int
> pqi_get_advanced_raid_bypass_config(struct pqi_ctrl_info *ctrl_info)
> request.cdb[2] = BMIC_SENSE_FEATURE_IO_PAGE;
> request.cdb[3] = BMIC_SENSE_FEATURE_IO_PAGE_AIO_SUBPAGE;
>
> - rc = pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header, 0, NULL, NO_TIMEOUT);
> + rc = pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header, 0, NULL);
>
> pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1,
> dir);
>
> @@ -794,13 +814,6 @@ static int pqi_flush_cache(struct pqi_ctrl_info
> *ctrl_info,
> int rc;
> struct bmic_flush_cache *flush_cache;
>
> - /*
> - * Don't bother trying to flush the cache if the controller
> is
> - * locked up.
> - */
> - if (pqi_ctrl_offline(ctrl_info))
> - return -ENXIO;
> -
> flush_cache = kzalloc(sizeof(*flush_cache), GFP_KERNEL);
> if (!flush_cache)
> return -ENOMEM;
> @@ -979,9 +992,6 @@ static void pqi_update_time_worker(struct
> work_struct *work)
> ctrl_info = container_of(to_delayed_work(work), struct
> pqi_ctrl_info,
> update_time_work);
>
> - if (pqi_ctrl_offline(ctrl_info))
> - return;
> -
> rc = pqi_write_current_time_to_host_wellness(ctrl_info);
> if (rc)
> dev_warn(&ctrl_info->pci_dev->dev,
> @@ -1271,9 +1281,7 @@ static int pqi_get_raid_map(struct
> pqi_ctrl_info *ctrl_info,
> return -ENOMEM;
>
> rc = pqi_send_scsi_raid_request(ctrl_info, CISS_GET_RAID_MAP,
> - device->scsi3addr, raid_map, sizeof(*raid_map),
> - 0, NULL, NO_TIMEOUT);
> -
> + device->scsi3addr, raid_map, sizeof(*raid_map), 0,
> NULL);
> if (rc)
> goto error;
>
> @@ -1288,8 +1296,7 @@ static int pqi_get_raid_map(struct
> pqi_ctrl_info *ctrl_info,
> return -ENOMEM;
>
> rc = pqi_send_scsi_raid_request(ctrl_info,
> CISS_GET_RAID_MAP,
> - device->scsi3addr, raid_map, raid_map_size,
> - 0, NULL, NO_TIMEOUT);
> + device->scsi3addr, raid_map, raid_map_size,
> 0, NULL);
> if (rc)
> goto error;
>
> @@ -1464,6 +1471,9 @@ static int pqi_get_physical_device_info(struct
> pqi_ctrl_info *ctrl_info,
> sizeof(device->phys_connector));
> device->bay = id_phys->phys_bay_in_box;
>
> + memcpy(&device->page_83_identifier, &id_phys-
> >page_83_identifier,
> + sizeof(device->page_83_identifier));
> +
> return 0;
> }
>
This hunk belongs to the "unique wwid" part, see above.
> @@ -1970,8 +1980,13 @@ static void pqi_update_device_list(struct
> pqi_ctrl_info *ctrl_info,
>
> spin_unlock_irqrestore(&ctrl_info->scsi_device_list_lock,
> flags);
>
> - if (pqi_ctrl_in_ofa(ctrl_info))
> - pqi_ctrl_ofa_done(ctrl_info);
> + if (pqi_ofa_in_progress(ctrl_info)) {
> + list_for_each_entry_safe(device, next, &delete_list,
> delete_list_entry)
> + if (pqi_is_device_added(device))
> + pqi_device_remove_start(device);
> + pqi_ctrl_unblock_device_reset(ctrl_info);
> + pqi_scsi_unblock_requests(ctrl_info);
> + }
I don't understand the purpose of is code. pqi_device_remove_start()
will be called again a few lines below. Why do it twice? I suppose
it's related to the unblocking, but that deserves an explanation.
Also, why do you unblock requests while OFA is "in progress"?
>
> /* Remove all devices that have gone away. */
> list_for_each_entry_safe(device, next, &delete_list,
> delete_list_entry) {
> @@ -1993,19 +2008,14 @@ static void pqi_update_device_list(struct
> pqi_ctrl_info *ctrl_info,
The following hunk is unrelated to synchronization.
> * Notify the SCSI ML if the queue depth of any existing
> device has
> * changed.
> */
> - list_for_each_entry(device, &ctrl_info->scsi_device_list,
> - scsi_device_list_entry) {
> - if (device->sdev) {
> - if (device->queue_depth !=
> - device->advertised_queue_depth) {
> - device->advertised_queue_depth =
> device->queue_depth;
> - scsi_change_queue_depth(device->sdev,
> - device-
> >advertised_queue_depth);
> - }
> - if (device->rescan) {
> - scsi_rescan_device(&device->sdev-
> >sdev_gendev);
> - device->rescan = false;
> - }
> + list_for_each_entry(device, &ctrl_info->scsi_device_list,
> scsi_device_list_entry) {
> + if (device->sdev && device->queue_depth != device-
> >advertised_queue_depth) {
> + device->advertised_queue_depth = device-
> >queue_depth;
> + scsi_change_queue_depth(device->sdev, device-
> >advertised_queue_depth);
> + }
> + if (device->rescan) {
> + scsi_rescan_device(&device->sdev-
> >sdev_gendev);
> + device->rescan = false;
> }
You've taken the reference to device->sdev->sdev_gendev out of the if
(device->sdev) clause. Can you be certain that device->sdev is non-
NULL?
> }
>
> @@ -2073,6 +2083,16 @@ static inline bool pqi_expose_device(struct
> pqi_scsi_dev *device)
> return !device->is_physical_device ||
> !pqi_skip_device(device->scsi3addr);
> }
>
The following belongs to the "unique wwid" part.
> +static inline void pqi_set_physical_device_wwid(struct pqi_ctrl_info
> *ctrl_info,
> + struct pqi_scsi_dev *device, struct
> report_phys_lun_extended_entry *phys_lun_ext_entry)
> +{
> + if (ctrl_info->unique_wwid_in_report_phys_lun_supported ||
> + pqi_is_device_with_sas_address(device))
> + device->wwid = phys_lun_ext_entry->wwid;
> + else
> + device->wwid =
> cpu_to_be64(get_unaligned_be64(&device->page_83_identifier));
> +}
> +
> static int pqi_update_scsi_devices(struct pqi_ctrl_info *ctrl_info)
> {
> int i;
> @@ -2238,7 +2258,7 @@ static int pqi_update_scsi_devices(struct
> pqi_ctrl_info *ctrl_info)
> pqi_assign_bus_target_lun(device);
>
> if (device->is_physical_device) {
> - device->wwid = phys_lun_ext_entry->wwid;
> + pqi_set_physical_device_wwid(ctrl_info,
> device, phys_lun_ext_entry);
> if ((phys_lun_ext_entry->device_flags &
> CISS_REPORT_PHYS_DEV_FLAG_AIO_ENABLED
> ) &&
> phys_lun_ext_entry->aio_handle) {
> @@ -2278,21 +2298,27 @@ static int pqi_update_scsi_devices(struct
> pqi_ctrl_info *ctrl_info)
>
> static int pqi_scan_scsi_devices(struct pqi_ctrl_info *ctrl_info)
> {
> - int rc = 0;
> + int rc;
> + int mutex_acquired;
>
> if (pqi_ctrl_offline(ctrl_info))
> return -ENXIO;
>
> - if (!mutex_trylock(&ctrl_info->scan_mutex)) {
> + mutex_acquired = mutex_trylock(&ctrl_info->scan_mutex);
> +
> + if (!mutex_acquired) {
> + if (pqi_ctrl_scan_blocked(ctrl_info))
> + return -EBUSY;
> pqi_schedule_rescan_worker_delayed(ctrl_info);
> - rc = -EINPROGRESS;
> - } else {
> - rc = pqi_update_scsi_devices(ctrl_info);
> - if (rc)
> -
> pqi_schedule_rescan_worker_delayed(ctrl_info);
> - mutex_unlock(&ctrl_info->scan_mutex);
> + return -EINPROGRESS;
> }
>
> + rc = pqi_update_scsi_devices(ctrl_info);
> + if (rc && !pqi_ctrl_scan_blocked(ctrl_info))
> + pqi_schedule_rescan_worker_delayed(ctrl_info);
> +
> + mutex_unlock(&ctrl_info->scan_mutex);
> +
> return rc;
> }
>
> @@ -2301,8 +2327,6 @@ static void pqi_scan_start(struct Scsi_Host
> *shost)
> struct pqi_ctrl_info *ctrl_info;
>
> ctrl_info = shost_to_hba(shost);
> - if (pqi_ctrl_in_ofa(ctrl_info))
> - return;
>
> pqi_scan_scsi_devices(ctrl_info);
> }
> @@ -2319,27 +2343,8 @@ static int pqi_scan_finished(struct Scsi_Host
> *shost,
> return !mutex_is_locked(&ctrl_info->scan_mutex);
> }
>
> -static void pqi_wait_until_scan_finished(struct pqi_ctrl_info
> *ctrl_info)
> -{
> - mutex_lock(&ctrl_info->scan_mutex);
> - mutex_unlock(&ctrl_info->scan_mutex);
> -}
> -
> -static void pqi_wait_until_lun_reset_finished(struct pqi_ctrl_info
> *ctrl_info)
> -{
> - mutex_lock(&ctrl_info->lun_reset_mutex);
> - mutex_unlock(&ctrl_info->lun_reset_mutex);
> -}
> -
> -static void pqi_wait_until_ofa_finished(struct pqi_ctrl_info
> *ctrl_info)
> -{
> - mutex_lock(&ctrl_info->ofa_mutex);
> - mutex_unlock(&ctrl_info->ofa_mutex);
> -}
Here, again, I wonder if this mutex_lock()/mutex_unlock() approach is
optimal. Have you considered using completions?
See above for rationale.
> -
> -static inline void pqi_set_encryption_info(
> - struct pqi_encryption_info *encryption_info, struct raid_map
> *raid_map,
> - u64 first_block)
> +static inline void pqi_set_encryption_info(struct
> pqi_encryption_info *encryption_info,
> + struct raid_map *raid_map, u64 first_block)
> {
> u32 volume_blk_size;
This whitespace change doesn't belong here.
>
> @@ -3251,8 +3256,8 @@ static void pqi_acknowledge_event(struct
> pqi_ctrl_info *ctrl_info,
> put_unaligned_le16(sizeof(request) -
> PQI_REQUEST_HEADER_LENGTH,
> &request.header.iu_length);
> request.event_type = event->event_type;
> - request.event_id = event->event_id;
> - request.additional_event_id = event->additional_event_id;
> + put_unaligned_le16(event->event_id, &request.event_id);
> + put_unaligned_le16(event->additional_event_id,
> &request.additional_event_id);
The different treatment of the event_id fields is unrelated to
synchronization, or am I missing something?
>
> pqi_send_event_ack(ctrl_info, &request, sizeof(request));
> }
> @@ -3263,8 +3268,8 @@ static void pqi_acknowledge_event(struct
> pqi_ctrl_info *ctrl_info,
> static enum pqi_soft_reset_status pqi_poll_for_soft_reset_status(
> struct pqi_ctrl_info *ctrl_info)
> {
> - unsigned long timeout;
> u8 status;
> + unsigned long timeout;
>
> timeout = (PQI_SOFT_RESET_STATUS_TIMEOUT_SECS * PQI_HZ) +
> jiffies;
>
> @@ -3276,120 +3281,169 @@ static enum pqi_soft_reset_status
> pqi_poll_for_soft_reset_status(
> if (status & PQI_SOFT_RESET_ABORT)
> return RESET_ABORT;
>
> + if (!sis_is_firmware_running(ctrl_info))
> + return RESET_NORESPONSE;
> +
> if (time_after(jiffies, timeout)) {
> - dev_err(&ctrl_info->pci_dev->dev,
> + dev_warn(&ctrl_info->pci_dev->dev,
> "timed out waiting for soft reset
> status\n");
> return RESET_TIMEDOUT;
> }
>
> - if (!sis_is_firmware_running(ctrl_info))
> - return RESET_NORESPONSE;
> -
> ssleep(PQI_SOFT_RESET_STATUS_POLL_INTERVAL_SECS);
> }
> }
>
> -static void pqi_process_soft_reset(struct pqi_ctrl_info *ctrl_info,
> - enum pqi_soft_reset_status reset_status)
> +static void pqi_process_soft_reset(struct pqi_ctrl_info *ctrl_info)
> {
> int rc;
> + unsigned int delay_secs;
> + enum pqi_soft_reset_status reset_status;
> +
> + if (ctrl_info->soft_reset_handshake_supported)
> + reset_status =
> pqi_poll_for_soft_reset_status(ctrl_info);
> + else
> + reset_status = RESET_INITIATE_FIRMWARE;
> +
> + pqi_ofa_free_host_buffer(ctrl_info);
> +
> + delay_secs = PQI_POST_RESET_DELAY_SECS;
>
> switch (reset_status) {
> - case RESET_INITIATE_DRIVER:
> case RESET_TIMEDOUT:
> + delay_secs =
> PQI_POST_OFA_RESET_DELAY_UPON_TIMEOUT_SECS;
> + fallthrough;
> + case RESET_INITIATE_DRIVER:
> dev_info(&ctrl_info->pci_dev->dev,
> - "resetting controller %u\n", ctrl_info-
> >ctrl_id);
> + "Online Firmware Activation:
> resetting controller\n");
> sis_soft_reset(ctrl_info);
> fallthrough;
> case RESET_INITIATE_FIRMWARE:
> - rc = pqi_ofa_ctrl_restart(ctrl_info);
> - pqi_ofa_free_host_buffer(ctrl_info);
> + ctrl_info->pqi_mode_enabled = false;
> + pqi_save_ctrl_mode(ctrl_info, SIS_MODE);
> + rc = pqi_ofa_ctrl_restart(ctrl_info, delay_secs);
> + pqi_ctrl_ofa_done(ctrl_info);
> dev_info(&ctrl_info->pci_dev->dev,
> - "Online Firmware Activation for controller
> %u: %s\n",
> - ctrl_info->ctrl_id, rc == 0 ? "SUCCESS" :
> "FAILED");
> + "Online Firmware Activation: %s\n",
> + rc == 0 ? "SUCCESS" : "FAILED");
> break;
> case RESET_ABORT:
> - pqi_ofa_ctrl_unquiesce(ctrl_info);
> dev_info(&ctrl_info->pci_dev->dev,
> - "Online Firmware Activation for controller
> %u: %s\n",
> - ctrl_info->ctrl_id, "ABORTED");
> + "Online Firmware Activation
> ABORTED\n");
> + if (ctrl_info->soft_reset_handshake_supported)
> + pqi_clear_soft_reset_status(ctrl_info);
> + pqi_ctrl_ofa_done(ctrl_info);
> + pqi_ofa_ctrl_unquiesce(ctrl_info);
> break;
> case RESET_NORESPONSE:
> - pqi_ofa_free_host_buffer(ctrl_info);
> + fallthrough;
> + default:
> + dev_err(&ctrl_info->pci_dev->dev,
> + "unexpected Online Firmware Activation reset
> status: 0x%x\n",
> + reset_status);
> + pqi_ctrl_ofa_done(ctrl_info);
> + pqi_ofa_ctrl_unquiesce(ctrl_info);
> pqi_take_ctrl_offline(ctrl_info);
> break;
> }
> }
>
> -static void pqi_ofa_process_event(struct pqi_ctrl_info *ctrl_info,
> - struct pqi_event *event)
> +static void pqi_ofa_memory_alloc_worker(struct work_struct *work)
Moving the ofa handling into work queues seems to be a key aspect of
this patch. The patch description should mention how this is will
improve synchronization. Naïve thinking suggests that making these
calls asynchronous could aggravate synchronization issues.
Repeating myself, I feel that completions would be the best way so
synchronize with these work items.
> {
> - u16 event_id;
> - enum pqi_soft_reset_status status;
> + struct pqi_ctrl_info *ctrl_info;
>
> - event_id = get_unaligned_le16(&event->event_id);
> + ctrl_info = container_of(work, struct pqi_ctrl_info,
> ofa_memory_alloc_work);
>
> - mutex_lock(&ctrl_info->ofa_mutex);
> + pqi_ctrl_ofa_start(ctrl_info);
> + pqi_ofa_setup_host_buffer(ctrl_info);
> + pqi_ofa_host_memory_update(ctrl_info);
> +}
>
> - if (event_id == PQI_EVENT_OFA_QUIESCE) {
> - dev_info(&ctrl_info->pci_dev->dev,
> - "Received Online Firmware Activation quiesce
> event for controller %u\n",
> - ctrl_info->ctrl_id);
> - pqi_ofa_ctrl_quiesce(ctrl_info);
> - pqi_acknowledge_event(ctrl_info, event);
> - if (ctrl_info->soft_reset_handshake_supported) {
> - status =
> pqi_poll_for_soft_reset_status(ctrl_info);
> - pqi_process_soft_reset(ctrl_info, status);
> - } else {
> - pqi_process_soft_reset(ctrl_info,
> - RESET_INITIATE_FIRMWARE);
> - }
> +static void pqi_ofa_quiesce_worker(struct work_struct *work)
> +{
> + struct pqi_ctrl_info *ctrl_info;
> + struct pqi_event *event;
>
> - } else if (event_id == PQI_EVENT_OFA_MEMORY_ALLOCATION) {
> - pqi_acknowledge_event(ctrl_info, event);
> - pqi_ofa_setup_host_buffer(ctrl_info,
> - le32_to_cpu(event->ofa_bytes_requested));
> - pqi_ofa_host_memory_update(ctrl_info);
> - } else if (event_id == PQI_EVENT_OFA_CANCELED) {
> - pqi_ofa_free_host_buffer(ctrl_info);
> - pqi_acknowledge_event(ctrl_info, event);
> + ctrl_info = container_of(work, struct pqi_ctrl_info,
> ofa_quiesce_work);
> +
> + event = &ctrl_info-
> >events[pqi_event_type_to_event_index(PQI_EVENT_TYPE_OFA)];
> +
> + pqi_ofa_ctrl_quiesce(ctrl_info);
> + pqi_acknowledge_event(ctrl_info, event);
> + pqi_process_soft_reset(ctrl_info);
> +}
> +
> +static bool pqi_ofa_process_event(struct pqi_ctrl_info *ctrl_info,
> + struct pqi_event *event)
> +{
> + bool ack_event;
> +
> + ack_event = true;
> +
> + switch (event->event_id) {
> + case PQI_EVENT_OFA_MEMORY_ALLOCATION:
> + dev_info(&ctrl_info->pci_dev->dev,
> + "received Online Firmware Activation memory
> allocation request\n");
> + schedule_work(&ctrl_info->ofa_memory_alloc_work);
> + break;
> + case PQI_EVENT_OFA_QUIESCE:
> dev_info(&ctrl_info->pci_dev->dev,
> - "Online Firmware Activation(%u) cancel reason
> : %u\n",
> - ctrl_info->ctrl_id, event-
> >ofa_cancel_reason);
> + "received Online Firmware Activation quiesce
> request\n");
> + schedule_work(&ctrl_info->ofa_quiesce_work);
> + ack_event = false;
> + break;
> + case PQI_EVENT_OFA_CANCELED:
> + dev_info(&ctrl_info->pci_dev->dev,
> + "received Online Firmware Activation cancel
> request: reason: %u\n",
> + ctrl_info->ofa_cancel_reason);
> + pqi_ofa_free_host_buffer(ctrl_info);
> + pqi_ctrl_ofa_done(ctrl_info);
> + break;
> + default:
> + dev_err(&ctrl_info->pci_dev->dev,
> + "received unknown Online Firmware Activation
> request: event ID: %u\n",
> + event->event_id);
> + break;
> }
>
> - mutex_unlock(&ctrl_info->ofa_mutex);
> + return ack_event;
> }
>
> static void pqi_event_worker(struct work_struct *work)
> {
> unsigned int i;
> + bool rescan_needed;
> struct pqi_ctrl_info *ctrl_info;
> struct pqi_event *event;
> + bool ack_event;
>
> ctrl_info = container_of(work, struct pqi_ctrl_info,
> event_work);
>
> pqi_ctrl_busy(ctrl_info);
> - pqi_wait_if_ctrl_blocked(ctrl_info, NO_TIMEOUT);
> + pqi_wait_if_ctrl_blocked(ctrl_info);
> if (pqi_ctrl_offline(ctrl_info))
> goto out;
>
> - pqi_schedule_rescan_worker_delayed(ctrl_info);
> -
> + rescan_needed = false;
> event = ctrl_info->events;
> for (i = 0; i < PQI_NUM_SUPPORTED_EVENTS; i++) {
> if (event->pending) {
> event->pending = false;
> if (event->event_type == PQI_EVENT_TYPE_OFA)
> {
> - pqi_ctrl_unbusy(ctrl_info);
> - pqi_ofa_process_event(ctrl_info,
> event);
> - return;
> + ack_event =
> pqi_ofa_process_event(ctrl_info, event);
> + } else {
> + ack_event = true;
> + rescan_needed = true;
> }
> - pqi_acknowledge_event(ctrl_info, event);
> + if (ack_event)
> + pqi_acknowledge_event(ctrl_info,
> event);
> }
> event++;
> }
>
> + if (rescan_needed)
> + pqi_schedule_rescan_worker_delayed(ctrl_info);
> +
> out:
> pqi_ctrl_unbusy(ctrl_info);
> }
> @@ -3446,37 +3500,18 @@ static inline void
> pqi_stop_heartbeat_timer(struct pqi_ctrl_info *ctrl_info)
> del_timer_sync(&ctrl_info->heartbeat_timer);
> }
>
> -static inline int pqi_event_type_to_event_index(unsigned int
> event_type)
> -{
> - int index;
> -
> - for (index = 0; index <
> ARRAY_SIZE(pqi_supported_event_types); index++)
> - if (event_type == pqi_supported_event_types[index])
> - return index;
> -
> - return -1;
> -}
> -
> -static inline bool pqi_is_supported_event(unsigned int event_type)
> -{
> - return pqi_event_type_to_event_index(event_type) != -1;
> -}
> -
> -static void pqi_ofa_capture_event_payload(struct pqi_event *event,
> - struct pqi_event_response *response)
> +static void pqi_ofa_capture_event_payload(struct pqi_ctrl_info
> *ctrl_info,
> + struct pqi_event *event, struct pqi_event_response *response)
> {
> - u16 event_id;
> -
> - event_id = get_unaligned_le16(&event->event_id);
> -
> - if (event->event_type == PQI_EVENT_TYPE_OFA) {
> - if (event_id == PQI_EVENT_OFA_MEMORY_ALLOCATION) {
> - event->ofa_bytes_requested =
> - response-
> >data.ofa_memory_allocation.bytes_requested;
> - } else if (event_id == PQI_EVENT_OFA_CANCELED) {
> - event->ofa_cancel_reason =
> - response->data.ofa_cancelled.reason;
> - }
> + switch (event->event_id) {
> + case PQI_EVENT_OFA_MEMORY_ALLOCATION:
> + ctrl_info->ofa_bytes_requested =
> + get_unaligned_le32(&response-
> >data.ofa_memory_allocation.bytes_requested);
> + break;
> + case PQI_EVENT_OFA_CANCELED:
> + ctrl_info->ofa_cancel_reason =
> + get_unaligned_le16(&response-
> >data.ofa_cancelled.reason);
> + break;
> }
> }
>
> @@ -3510,17 +3545,17 @@ static int pqi_process_event_intr(struct
> pqi_ctrl_info *ctrl_info)
> num_events++;
> response = event_queue->oq_element_array + (oq_ci *
> PQI_EVENT_OQ_ELEMENT_LENGTH);
>
> - event_index =
> - pqi_event_type_to_event_index(response-
> >event_type);
> + event_index = pqi_event_type_to_event_index(response-
> >event_type);
>
> if (event_index >= 0 && response-
> >request_acknowledge) {
> event = &ctrl_info->events[event_index];
> event->pending = true;
> event->event_type = response->event_type;
> - event->event_id = response->event_id;
> - event->additional_event_id = response-
> >additional_event_id;
> + event->event_id =
> get_unaligned_le16(&response->event_id);
> + event->additional_event_id =
> + get_unaligned_le32(&response-
> >additional_event_id);
> if (event->event_type == PQI_EVENT_TYPE_OFA)
> - pqi_ofa_capture_event_payload(event,
> response);
> + pqi_ofa_capture_event_payload(ctrl_in
> fo, event, response);
> }
>
> oq_ci = (oq_ci + 1) % PQI_NUM_EVENT_QUEUE_ELEMENTS;
> @@ -3537,8 +3572,7 @@ static int pqi_process_event_intr(struct
> pqi_ctrl_info *ctrl_info)
>
> #define PQI_LEGACY_INTX_MASK 0x1
>
> -static inline void pqi_configure_legacy_intx(struct pqi_ctrl_info
> *ctrl_info,
> - bool enable_intx)
> +static inline void pqi_configure_legacy_intx(struct pqi_ctrl_info
> *ctrl_info, bool enable_intx)
another whitespace hunk
> {
> u32 intx_mask;
> struct pqi_device_registers __iomem *pqi_registers;
> @@ -4216,59 +4250,36 @@ static int
> pqi_process_raid_io_error_synchronous(
> return rc;
> }
>
> +static inline bool pqi_is_blockable_request(struct pqi_iu_header
> *request)
> +{
> + return (request->driver_flags &
> PQI_DRIVER_NONBLOCKABLE_REQUEST) == 0;
> +}
> +
> static int pqi_submit_raid_request_synchronous(struct pqi_ctrl_info
> *ctrl_info,
> struct pqi_iu_header *request, unsigned int flags,
> - struct pqi_raid_error_info *error_info, unsigned long
> timeout_msecs)
> + struct pqi_raid_error_info *error_info)
The removal of the timeout_msecs argument to this function could be
a separate patch in its own right.
> {
> int rc = 0;
> struct pqi_io_request *io_request;
> - unsigned long start_jiffies;
> - unsigned long msecs_blocked;
> size_t iu_length;
> DECLARE_COMPLETION_ONSTACK(wait);
>
> - /*
> - * Note that specifying PQI_SYNC_FLAGS_INTERRUPTABLE and a
> timeout value
> - * are mutually exclusive.
> - */
> -
> if (flags & PQI_SYNC_FLAGS_INTERRUPTABLE) {
> if (down_interruptible(&ctrl_info->sync_request_sem))
> return -ERESTARTSYS;
> } else {
> - if (timeout_msecs == NO_TIMEOUT) {
> - down(&ctrl_info->sync_request_sem);
> - } else {
> - start_jiffies = jiffies;
> - if (down_timeout(&ctrl_info-
> >sync_request_sem,
> - msecs_to_jiffies(timeout_msecs)))
> - return -ETIMEDOUT;
> - msecs_blocked =
> - jiffies_to_msecs(jiffies -
> start_jiffies);
> - if (msecs_blocked >= timeout_msecs) {
> - rc = -ETIMEDOUT;
> - goto out;
> - }
> - timeout_msecs -= msecs_blocked;
> - }
> + down(&ctrl_info->sync_request_sem);
> }
>
> pqi_ctrl_busy(ctrl_info);
> - timeout_msecs = pqi_wait_if_ctrl_blocked(ctrl_info,
> timeout_msecs);
> - if (timeout_msecs == 0) {
> - pqi_ctrl_unbusy(ctrl_info);
> - rc = -ETIMEDOUT;
> - goto out;
> - }
> + if (pqi_is_blockable_request(request))
> + pqi_wait_if_ctrl_blocked(ctrl_info);
You wait here after taking the semaphore - is that intended? Why?
>
> if (pqi_ctrl_offline(ctrl_info)) {
Should you test this before waiting, perhaps?
> - pqi_ctrl_unbusy(ctrl_info);
> rc = -ENXIO;
> goto out;
> }
>
> - atomic_inc(&ctrl_info->sync_cmds_outstanding);
> -
> io_request = pqi_alloc_io_request(ctrl_info);
>
> put_unaligned_le16(io_request->index,
> @@ -4288,18 +4299,7 @@ static int
> pqi_submit_raid_request_synchronous(struct pqi_ctrl_info *ctrl_info,
> pqi_start_io(ctrl_info, &ctrl_info-
> >queue_groups[PQI_DEFAULT_QUEUE_GROUP], RAID_PATH,
> io_request);
>
> - pqi_ctrl_unbusy(ctrl_info);
> -
> - if (timeout_msecs == NO_TIMEOUT) {
> - pqi_wait_for_completion_io(ctrl_info, &wait);
> - } else {
> - if (!wait_for_completion_io_timeout(&wait,
> - msecs_to_jiffies(timeout_msecs))) {
> - dev_warn(&ctrl_info->pci_dev->dev,
> - "command timed out\n");
> - rc = -ETIMEDOUT;
> - }
> - }
> + pqi_wait_for_completion_io(ctrl_info, &wait);
>
> if (error_info) {
> if (io_request->error_info)
> @@ -4312,8 +4312,8 @@ static int
> pqi_submit_raid_request_synchronous(struct pqi_ctrl_info *ctrl_info,
>
> pqi_free_io_request(io_request);
>
> - atomic_dec(&ctrl_info->sync_cmds_outstanding);
> out:
> + pqi_ctrl_unbusy(ctrl_info);
> up(&ctrl_info->sync_request_sem);
>
> return rc;
> @@ -4350,8 +4350,7 @@ static int
> pqi_submit_admin_request_synchronous(
> rc = pqi_poll_for_admin_response(ctrl_info, response);
>
> if (rc == 0)
> - rc = pqi_validate_admin_response(response,
> - request->function_code);
> + rc = pqi_validate_admin_response(response, request-
> >function_code);
>
> return rc;
> }
> @@ -4721,7 +4720,7 @@ static int pqi_configure_events(struct
> pqi_ctrl_info *ctrl_info,
> goto out;
>
> rc = pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header,
> - 0, NULL, NO_TIMEOUT);
> + 0, NULL);
>
> pqi_pci_unmap(ctrl_info->pci_dev,
> request.data.report_event_configuration.sg_descriptor
> s, 1,
> @@ -4757,7 +4756,7 @@ static int pqi_configure_events(struct
> pqi_ctrl_info *ctrl_info,
> goto out;
>
> rc = pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header, 0,
> - NULL, NO_TIMEOUT);
> + NULL);
>
> pqi_pci_unmap(ctrl_info->pci_dev,
> request.data.report_event_configuration.sg_descriptor
> s, 1,
> @@ -5277,12 +5276,6 @@ static inline int
> pqi_raid_submit_scsi_cmd(struct pqi_ctrl_info *ctrl_info,
> device, scmd, queue_group);
> }
>
Below here, a new section starts that refacors the treatment of bypass
retries. I don't see how this is related to the synchronization issues
mentioned in the patch description.
> -static inline void pqi_schedule_bypass_retry(struct pqi_ctrl_info
> *ctrl_info)
> -{
> - if (!pqi_ctrl_blocked(ctrl_info))
> - schedule_work(&ctrl_info->raid_bypass_retry_work);
> -}
> -
> static bool pqi_raid_bypass_retry_needed(struct pqi_io_request
> *io_request)
> {
> struct scsi_cmnd *scmd;
> @@ -5299,7 +5292,7 @@ static bool pqi_raid_bypass_retry_needed(struct
> pqi_io_request *io_request)
> return false;
>
> device = scmd->device->hostdata;
> - if (pqi_device_offline(device))
> + if (pqi_device_offline(device) ||
> pqi_device_in_remove(device))
> return false;
>
> ctrl_info = shost_to_hba(scmd->device->host);
> @@ -5309,155 +5302,26 @@ static bool
> pqi_raid_bypass_retry_needed(struct pqi_io_request *io_request)
> return true;
> }
>
> -static inline void pqi_add_to_raid_bypass_retry_list(
> - struct pqi_ctrl_info *ctrl_info,
> - struct pqi_io_request *io_request, bool at_head)
> -{
> - unsigned long flags;
> -
> - spin_lock_irqsave(&ctrl_info->raid_bypass_retry_list_lock,
> flags);
> - if (at_head)
> - list_add(&io_request->request_list_entry,
> - &ctrl_info->raid_bypass_retry_list);
> - else
> - list_add_tail(&io_request->request_list_entry,
> - &ctrl_info->raid_bypass_retry_list);
> - spin_unlock_irqrestore(&ctrl_info-
> >raid_bypass_retry_list_lock, flags);
> -}
> -
> -static void pqi_queued_raid_bypass_complete(struct pqi_io_request
> *io_request,
> +static void pqi_aio_io_complete(struct pqi_io_request *io_request,
> void *context)
> {
> struct scsi_cmnd *scmd;
>
> scmd = io_request->scmd;
> + scsi_dma_unmap(scmd);
> + if (io_request->status == -EAGAIN ||
> + pqi_raid_bypass_retry_needed(io_request))
> + set_host_byte(scmd, DID_IMM_RETRY);
> pqi_free_io_request(io_request);
> pqi_scsi_done(scmd);
> }
>
> -static void pqi_queue_raid_bypass_retry(struct pqi_io_request
> *io_request)
> +static inline int pqi_aio_submit_scsi_cmd(struct pqi_ctrl_info
> *ctrl_info,
> + struct pqi_scsi_dev *device, struct scsi_cmnd *scmd,
> + struct pqi_queue_group *queue_group)
> {
> - struct scsi_cmnd *scmd;
> - struct pqi_ctrl_info *ctrl_info;
> -
> - io_request->io_complete_callback =
> pqi_queued_raid_bypass_complete;
> - scmd = io_request->scmd;
> - scmd->result = 0;
> - ctrl_info = shost_to_hba(scmd->device->host);
> -
> - pqi_add_to_raid_bypass_retry_list(ctrl_info, io_request,
> false);
> - pqi_schedule_bypass_retry(ctrl_info);
> -}
> -
> -static int pqi_retry_raid_bypass(struct pqi_io_request *io_request)
> -{
> - struct scsi_cmnd *scmd;
> - struct pqi_scsi_dev *device;
> - struct pqi_ctrl_info *ctrl_info;
> - struct pqi_queue_group *queue_group;
> -
> - scmd = io_request->scmd;
> - device = scmd->device->hostdata;
> - if (pqi_device_in_reset(device)) {
> - pqi_free_io_request(io_request);
> - set_host_byte(scmd, DID_RESET);
> - pqi_scsi_done(scmd);
> - return 0;
> - }
> -
> - ctrl_info = shost_to_hba(scmd->device->host);
> - queue_group = io_request->queue_group;
> -
> - pqi_reinit_io_request(io_request);
> -
> - return pqi_raid_submit_scsi_cmd_with_io_request(ctrl_info,
> io_request,
> - device, scmd, queue_group);
> -}
> -
> -static inline struct pqi_io_request
> *pqi_next_queued_raid_bypass_request(
> - struct pqi_ctrl_info *ctrl_info)
> -{
> - unsigned long flags;
> - struct pqi_io_request *io_request;
> -
> - spin_lock_irqsave(&ctrl_info->raid_bypass_retry_list_lock,
> flags);
> - io_request = list_first_entry_or_null(
> - &ctrl_info->raid_bypass_retry_list,
> - struct pqi_io_request, request_list_entry);
> - if (io_request)
> - list_del(&io_request->request_list_entry);
> - spin_unlock_irqrestore(&ctrl_info-
> >raid_bypass_retry_list_lock, flags);
> -
> - return io_request;
> -}
> -
> -static void pqi_retry_raid_bypass_requests(struct pqi_ctrl_info
> *ctrl_info)
> -{
> - int rc;
> - struct pqi_io_request *io_request;
> -
> - pqi_ctrl_busy(ctrl_info);
> -
> - while (1) {
> - if (pqi_ctrl_blocked(ctrl_info))
> - break;
> - io_request =
> pqi_next_queued_raid_bypass_request(ctrl_info);
> - if (!io_request)
> - break;
> - rc = pqi_retry_raid_bypass(io_request);
> - if (rc) {
> - pqi_add_to_raid_bypass_retry_list(ctrl_info,
> io_request,
> - true);
> - pqi_schedule_bypass_retry(ctrl_info);
> - break;
> - }
> - }
> -
> - pqi_ctrl_unbusy(ctrl_info);
> -}
> -
> -static void pqi_raid_bypass_retry_worker(struct work_struct *work)
> -{
> - struct pqi_ctrl_info *ctrl_info;
> -
> - ctrl_info = container_of(work, struct pqi_ctrl_info,
> - raid_bypass_retry_work);
> - pqi_retry_raid_bypass_requests(ctrl_info);
> -}
> -
> -static void pqi_clear_all_queued_raid_bypass_retries(
> - struct pqi_ctrl_info *ctrl_info)
> -{
> - unsigned long flags;
> -
> - spin_lock_irqsave(&ctrl_info->raid_bypass_retry_list_lock,
> flags);
> - INIT_LIST_HEAD(&ctrl_info->raid_bypass_retry_list);
> - spin_unlock_irqrestore(&ctrl_info-
> >raid_bypass_retry_list_lock, flags);
> -}
> -
> -static void pqi_aio_io_complete(struct pqi_io_request *io_request,
> - void *context)
> -{
> - struct scsi_cmnd *scmd;
> -
> - scmd = io_request->scmd;
> - scsi_dma_unmap(scmd);
> - if (io_request->status == -EAGAIN)
> - set_host_byte(scmd, DID_IMM_RETRY);
> - else if (pqi_raid_bypass_retry_needed(io_request)) {
> - pqi_queue_raid_bypass_retry(io_request);
> - return;
> - }
> - pqi_free_io_request(io_request);
> - pqi_scsi_done(scmd);
> -}
> -
> -static inline int pqi_aio_submit_scsi_cmd(struct pqi_ctrl_info
> *ctrl_info,
> - struct pqi_scsi_dev *device, struct scsi_cmnd *scmd,
> - struct pqi_queue_group *queue_group)
> -{
> - return pqi_aio_submit_io(ctrl_info, scmd, device->aio_handle,
> - scmd->cmnd, scmd->cmd_len, queue_group, NULL, false);
> + return pqi_aio_submit_io(ctrl_info, scmd, device->aio_handle,
> + scmd->cmnd, scmd->cmd_len, queue_group, NULL, false);
> }
>
> static int pqi_aio_submit_io(struct pqi_ctrl_info *ctrl_info,
> @@ -5698,6 +5562,14 @@ static inline u16 pqi_get_hw_queue(struct
> pqi_ctrl_info *ctrl_info,
> return hw_queue;
> }
>
> +static inline bool pqi_is_bypass_eligible_request(struct scsi_cmnd
> *scmd)
> +{
> + if (blk_rq_is_passthrough(scmd->request))
> + return false;
> +
> + return scmd->retries == 0;
> +}
> +
Nice, but this fits better into (or next to) 10/25 IMO.
> /*
> * This function gets called just before we hand the completed SCSI
> request
> * back to the SML.
> @@ -5806,7 +5678,6 @@ static int pqi_scsi_queue_command(struct
> Scsi_Host *shost, struct scsi_cmnd *scm
> bool raid_bypassed;
>
> device = scmd->device->hostdata;
> - ctrl_info = shost_to_hba(shost);
>
> if (!device) {
> set_host_byte(scmd, DID_NO_CONNECT);
> @@ -5816,15 +5687,15 @@ static int pqi_scsi_queue_command(struct
> Scsi_Host *shost, struct scsi_cmnd *scm
>
> atomic_inc(&device->scsi_cmds_outstanding);
>
> + ctrl_info = shost_to_hba(shost);
> +
> if (pqi_ctrl_offline(ctrl_info) ||
> pqi_device_in_remove(device)) {
> set_host_byte(scmd, DID_NO_CONNECT);
> pqi_scsi_done(scmd);
> return 0;
> }
>
> - pqi_ctrl_busy(ctrl_info);
> - if (pqi_ctrl_blocked(ctrl_info) ||
> pqi_device_in_reset(device) ||
> - pqi_ctrl_in_ofa(ctrl_info) ||
> pqi_ctrl_in_shutdown(ctrl_info)) {
> + if (pqi_ctrl_blocked(ctrl_info)) {
> rc = SCSI_MLQUEUE_HOST_BUSY;
> goto out;
> }
> @@ -5841,13 +5712,12 @@ static int pqi_scsi_queue_command(struct
> Scsi_Host *shost, struct scsi_cmnd *scm
> if (pqi_is_logical_device(device)) {
> raid_bypassed = false;
> if (device->raid_bypass_enabled &&
> - !blk_rq_is_passthrough(scmd->request)) {
> - if (!pqi_is_parity_write_stream(ctrl_info,
> scmd)) {
> - rc =
> pqi_raid_bypass_submit_scsi_cmd(ctrl_info, device, scmd,
> queue_group);
> - if (rc == 0 || rc ==
> SCSI_MLQUEUE_HOST_BUSY) {
> - raid_bypassed = true;
> - atomic_inc(&device-
> >raid_bypass_cnt);
> - }
> + pqi_is_bypass_eligible_request(scmd) &&
> + !pqi_is_parity_write_stream(ctrl_info, scmd))
> {
> + rc =
> pqi_raid_bypass_submit_scsi_cmd(ctrl_info, device, scmd,
> queue_group);
> + if (rc == 0 || rc == SCSI_MLQUEUE_HOST_BUSY)
> {
> + raid_bypassed = true;
> + atomic_inc(&device->raid_bypass_cnt);
> }
> }
> if (!raid_bypassed)
> @@ -5860,7 +5730,6 @@ static int pqi_scsi_queue_command(struct
> Scsi_Host *shost, struct scsi_cmnd *scm
> }
>
> out:
> - pqi_ctrl_unbusy(ctrl_info);
> if (rc)
> atomic_dec(&device->scsi_cmds_outstanding);
>
> @@ -5970,100 +5839,22 @@ static void
> pqi_fail_io_queued_for_device(struct pqi_ctrl_info *ctrl_info,
> }
> }
>
> -static void pqi_fail_io_queued_for_all_devices(struct pqi_ctrl_info
> *ctrl_info)
> -{
> - unsigned int i;
> - unsigned int path;
> - struct pqi_queue_group *queue_group;
> - unsigned long flags;
> - struct pqi_io_request *io_request;
> - struct pqi_io_request *next;
> - struct scsi_cmnd *scmd;
> -
> - for (i = 0; i < ctrl_info->num_queue_groups; i++) {
> - queue_group = &ctrl_info->queue_groups[i];
> -
> - for (path = 0; path < 2; path++) {
> - spin_lock_irqsave(&queue_group-
> >submit_lock[path],
> - flags);
> -
> - list_for_each_entry_safe(io_request, next,
> - &queue_group->request_list[path],
> - request_list_entry) {
> -
> - scmd = io_request->scmd;
> - if (!scmd)
> - continue;
> -
> - list_del(&io_request-
> >request_list_entry);
> - set_host_byte(scmd, DID_RESET);
> - pqi_scsi_done(scmd);
> - }
> -
> - spin_unlock_irqrestore(
> - &queue_group->submit_lock[path],
> flags);
> - }
> - }
> -}
> -
> static int pqi_device_wait_for_pending_io(struct pqi_ctrl_info
> *ctrl_info,
> struct pqi_scsi_dev *device, unsigned long timeout_secs)
> {
> unsigned long timeout;
>
> - timeout = (timeout_secs * PQI_HZ) + jiffies;
> -
> - while (atomic_read(&device->scsi_cmds_outstanding)) {
> - pqi_check_ctrl_health(ctrl_info);
> - if (pqi_ctrl_offline(ctrl_info))
> - return -ENXIO;
> - if (timeout_secs != NO_TIMEOUT) {
> - if (time_after(jiffies, timeout)) {
> - dev_err(&ctrl_info->pci_dev->dev,
> - "timed out waiting for
> pending IO\n");
> - return -ETIMEDOUT;
> - }
> - }
> - usleep_range(1000, 2000);
> - }
> -
> - return 0;
> -}
> -
> -static int pqi_ctrl_wait_for_pending_io(struct pqi_ctrl_info
> *ctrl_info,
> - unsigned long timeout_secs)
> -{
> - bool io_pending;
> - unsigned long flags;
> - unsigned long timeout;
> - struct pqi_scsi_dev *device;
>
> timeout = (timeout_secs * PQI_HZ) + jiffies;
> - while (1) {
> - io_pending = false;
> -
> - spin_lock_irqsave(&ctrl_info->scsi_device_list_lock,
> flags);
> - list_for_each_entry(device, &ctrl_info-
> >scsi_device_list,
> - scsi_device_list_entry) {
> - if (atomic_read(&device-
> >scsi_cmds_outstanding)) {
> - io_pending = true;
> - break;
> - }
> - }
> - spin_unlock_irqrestore(&ctrl_info-
> >scsi_device_list_lock,
> - flags);
> -
> - if (!io_pending)
> - break;
>
> + while (atomic_read(&device->scsi_cmds_outstanding)) {
> pqi_check_ctrl_health(ctrl_info);
> if (pqi_ctrl_offline(ctrl_info))
> return -ENXIO;
> -
> if (timeout_secs != NO_TIMEOUT) {
> if (time_after(jiffies, timeout)) {
> dev_err(&ctrl_info->pci_dev->dev,
> - "timed out waiting for
> pending IO\n");
> + "timed out waiting for
> pending I/O\n");
> return -ETIMEDOUT;
> }
> }
Like I said above (wrt pqi_scsi_block_requests), have you considered
using wait_event_timeout() here?
> @@ -6073,18 +5864,6 @@ static int pqi_ctrl_wait_for_pending_io(struct
> pqi_ctrl_info *ctrl_info,
> return 0;
> }
>
> -static int pqi_ctrl_wait_for_pending_sync_cmds(struct pqi_ctrl_info
> *ctrl_info)
> -{
> - while (atomic_read(&ctrl_info->sync_cmds_outstanding)) {
> - pqi_check_ctrl_health(ctrl_info);
> - if (pqi_ctrl_offline(ctrl_info))
> - return -ENXIO;
> - usleep_range(1000, 2000);
> - }
> -
> - return 0;
> -}
> -
> static void pqi_lun_reset_complete(struct pqi_io_request
> *io_request,
> void *context)
> {
> @@ -6156,13 +5935,11 @@ static int pqi_lun_reset(struct pqi_ctrl_info
> *ctrl_info,
> return rc;
> }
>
> -/* Performs a reset at the LUN level. */
> -
> #define PQI_LUN_RESET_RETRIES 3
> #define PQI_LUN_RESET_RETRY_INTERVAL_MSECS 10000
> #define PQI_LUN_RESET_PENDING_IO_TIMEOUT_SECS 120
>
> -static int _pqi_device_reset(struct pqi_ctrl_info *ctrl_info,
> +static int pqi_lun_reset_with_retries(struct pqi_ctrl_info
> *ctrl_info,
> struct pqi_scsi_dev *device)
> {
> int rc;
> @@ -6188,23 +5965,15 @@ static int pqi_device_reset(struct
> pqi_ctrl_info *ctrl_info,
> {
> int rc;
>
> - mutex_lock(&ctrl_info->lun_reset_mutex);
> -
> pqi_ctrl_block_requests(ctrl_info);
> pqi_ctrl_wait_until_quiesced(ctrl_info);
> pqi_fail_io_queued_for_device(ctrl_info, device);
> rc = pqi_wait_until_inbound_queues_empty(ctrl_info);
> - pqi_device_reset_start(device);
> - pqi_ctrl_unblock_requests(ctrl_info);
> -
> if (rc)
> rc = FAILED;
> else
> - rc = _pqi_device_reset(ctrl_info, device);
> -
> - pqi_device_reset_done(device);
> -
> - mutex_unlock(&ctrl_info->lun_reset_mutex);
> + rc = pqi_lun_reset_with_retries(ctrl_info, device);
> + pqi_ctrl_unblock_requests(ctrl_info);
>
> return rc;
> }
> @@ -6220,29 +5989,25 @@ static int pqi_eh_device_reset_handler(struct
> scsi_cmnd *scmd)
> ctrl_info = shost_to_hba(shost);
> device = scmd->device->hostdata;
>
> + mutex_lock(&ctrl_info->lun_reset_mutex);
> +
> dev_err(&ctrl_info->pci_dev->dev,
> "resetting scsi %d:%d:%d:%d\n",
> shost->host_no, device->bus, device->target, device-
> >lun);
>
> pqi_check_ctrl_health(ctrl_info);
> - if (pqi_ctrl_offline(ctrl_info) ||
> - pqi_device_reset_blocked(ctrl_info)) {
> + if (pqi_ctrl_offline(ctrl_info))
> rc = FAILED;
> - goto out;
> - }
> -
> - pqi_wait_until_ofa_finished(ctrl_info);
> -
> - atomic_inc(&ctrl_info->sync_cmds_outstanding);
> - rc = pqi_device_reset(ctrl_info, device);
> - atomic_dec(&ctrl_info->sync_cmds_outstanding);
> + else
> + rc = pqi_device_reset(ctrl_info, device);
>
> -out:
> dev_err(&ctrl_info->pci_dev->dev,
> "reset of scsi %d:%d:%d:%d: %s\n",
> shost->host_no, device->bus, device->target, device-
> >lun,
> rc == SUCCESS ? "SUCCESS" : "FAILED");
>
> + mutex_unlock(&ctrl_info->lun_reset_mutex);
> +
> return rc;
> }
>
> @@ -6544,7 +6309,7 @@ static int pqi_passthru_ioctl(struct
> pqi_ctrl_info *ctrl_info, void __user *arg)
> put_unaligned_le32(iocommand.Request.Timeout,
> &request.timeout);
>
> rc = pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header,
> - PQI_SYNC_FLAGS_INTERRUPTABLE, &pqi_error_info,
> NO_TIMEOUT);
> + PQI_SYNC_FLAGS_INTERRUPTABLE, &pqi_error_info);
>
> if (iocommand.buf_size > 0)
> pqi_pci_unmap(ctrl_info->pci_dev,
> request.sg_descriptors, 1,
> @@ -6596,9 +6361,6 @@ static int pqi_ioctl(struct scsi_device *sdev,
> unsigned int cmd,
>
> ctrl_info = shost_to_hba(sdev->host);
>
> - if (pqi_ctrl_in_ofa(ctrl_info) ||
> pqi_ctrl_in_shutdown(ctrl_info))
> - return -EBUSY;
> -
> switch (cmd) {
> case CCISS_DEREGDISK:
> case CCISS_REGNEWDISK:
> @@ -7145,9 +6907,7 @@ static int pqi_register_scsi(struct
> pqi_ctrl_info *ctrl_info)
>
> shost = scsi_host_alloc(&pqi_driver_template,
> sizeof(ctrl_info));
> if (!shost) {
> - dev_err(&ctrl_info->pci_dev->dev,
> - "scsi_host_alloc failed for controller %u\n",
> - ctrl_info->ctrl_id);
> + dev_err(&ctrl_info->pci_dev->dev, "scsi_host_alloc
> failed\n");
> return -ENOMEM;
> }
>
> @@ -7405,7 +7165,7 @@ static int pqi_config_table_update(struct
> pqi_ctrl_info *ctrl_info,
> &request.data.config_table_update.last_section);
>
> return pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header,
> - 0, NULL, NO_TIMEOUT);
> + 0, NULL);
> }
>
> static int pqi_enable_firmware_features(struct pqi_ctrl_info
> *ctrl_info,
> @@ -7483,7 +7243,8 @@ static void
> pqi_ctrl_update_feature_flags(struct pqi_ctrl_info *ctrl_info,
> break;
> case PQI_FIRMWARE_FEATURE_SOFT_RESET_HANDSHAKE:
> ctrl_info->soft_reset_handshake_supported =
> - firmware_feature->enabled;
> + firmware_feature->enabled &&
> + ctrl_info->soft_reset_status;
Should you use readb(ctrl_info->soft_reset_status) here, like above?
> break;
> case PQI_FIRMWARE_FEATURE_RAID_IU_TIMEOUT:
> ctrl_info->raid_iu_timeout_supported =
> firmware_feature->enabled;
> @@ -7491,6 +7252,10 @@ static void
> pqi_ctrl_update_feature_flags(struct pqi_ctrl_info *ctrl_info,
> case PQI_FIRMWARE_FEATURE_TMF_IU_TIMEOUT:
> ctrl_info->tmf_iu_timeout_supported =
> firmware_feature->enabled;
> break;
> + case PQI_FIRMWARE_FEATURE_UNIQUE_WWID_IN_REPORT_PHYS_LUN:
> + ctrl_info->unique_wwid_in_report_phys_lun_supported =
> + firmware_feature->enabled;
> + break;
> }
>
> pqi_firmware_feature_status(ctrl_info, firmware_feature);
> @@ -7581,6 +7346,11 @@ static struct pqi_firmware_feature
> pqi_firmware_features[] = {
> .feature_bit =
> PQI_FIRMWARE_FEATURE_RAID_BYPASS_ON_ENCRYPTED_NVME,
> .feature_status = pqi_firmware_feature_status,
> },
> + {
> + .feature_name = "Unique WWID in Report Physical LUN",
> + .feature_bit =
> PQI_FIRMWARE_FEATURE_UNIQUE_WWID_IN_REPORT_PHYS_LUN,
> + .feature_status = pqi_ctrl_update_feature_flags,
> + },
> };
>
> static void pqi_process_firmware_features(
> @@ -7665,14 +7435,34 @@ static void
> pqi_process_firmware_features_section(
> mutex_unlock(&pqi_firmware_features_mutex);
> }
>
The hunks below look like yet another independent change.
(Handling of firmware_feature_section_present).
> +/*
> + * Reset all controller settings that can be initialized during the
> processing
> + * of the PQI Configuration Table.
> + */
> +
> +static void pqi_ctrl_reset_config(struct pqi_ctrl_info *ctrl_info)
> +{
> + ctrl_info->heartbeat_counter = NULL;
> + ctrl_info->soft_reset_status = NULL;
> + ctrl_info->soft_reset_handshake_supported = false;
> + ctrl_info->enable_r1_writes = false;
> + ctrl_info->enable_r5_writes = false;
> + ctrl_info->enable_r6_writes = false;
> + ctrl_info->raid_iu_timeout_supported = false;
> + ctrl_info->tmf_iu_timeout_supported = false;
> + ctrl_info->unique_wwid_in_report_phys_lun_supported = false;
> +}
> +
> static int pqi_process_config_table(struct pqi_ctrl_info *ctrl_info)
> {
> u32 table_length;
> u32 section_offset;
> + bool firmware_feature_section_present;
> void __iomem *table_iomem_addr;
> struct pqi_config_table *config_table;
> struct pqi_config_table_section_header *section;
> struct pqi_config_table_section_info section_info;
> + struct pqi_config_table_section_info feature_section_info;
>
> table_length = ctrl_info->config_table_length;
> if (table_length == 0)
> @@ -7692,6 +7482,7 @@ static int pqi_process_config_table(struct
> pqi_ctrl_info *ctrl_info)
> table_iomem_addr = ctrl_info->iomem_base + ctrl_info-
> >config_table_offset;
> memcpy_fromio(config_table, table_iomem_addr, table_length);
>
> + firmware_feature_section_present = false;
> section_info.ctrl_info = ctrl_info;
> section_offset = get_unaligned_le32(&config_table-
> >first_section_offset);
>
> @@ -7704,7 +7495,8 @@ static int pqi_process_config_table(struct
> pqi_ctrl_info *ctrl_info)
>
> switch (get_unaligned_le16(§ion->section_id)) {
> case PQI_CONFIG_TABLE_SECTION_FIRMWARE_FEATURES:
> -
> pqi_process_firmware_features_section(§ion_
> info);
> + firmware_feature_section_present = true;
> + feature_section_info = section_info;
> break;
> case PQI_CONFIG_TABLE_SECTION_HEARTBEAT:
> if (pqi_disable_heartbeat)
> @@ -7722,13 +7514,21 @@ static int pqi_process_config_table(struct
> pqi_ctrl_info *ctrl_info)
> table_iomem_addr +
> section_offset +
> offsetof(struct
> pqi_config_table_soft_reset,
> - soft_reset_status);
> + soft_reset_status);
> break;
> }
>
> section_offset = get_unaligned_le16(§ion-
> >next_section_offset);
> }
>
> + /*
> + * We process the firmware feature section after all other
> sections
> + * have been processed so that the feature bit callbacks can
> take
> + * into account the settings configured by other sections.
> + */
> + if (firmware_feature_section_present)
> + pqi_process_firmware_features_section(&feature_sectio
> n_info);
> +
> kfree(config_table);
>
> return 0;
> @@ -7776,8 +7576,6 @@ static int pqi_force_sis_mode(struct
> pqi_ctrl_info *ctrl_info)
> return pqi_revert_to_sis_mode(ctrl_info);
> }
>
> -#define PQI_POST_RESET_DELAY_B4_MSGU_READY 5000
> -
> static int pqi_ctrl_init(struct pqi_ctrl_info *ctrl_info)
> {
> int rc;
> @@ -7785,7 +7583,7 @@ static int pqi_ctrl_init(struct pqi_ctrl_info
> *ctrl_info)
>
> if (reset_devices) {
> sis_soft_reset(ctrl_info);
> - msleep(PQI_POST_RESET_DELAY_B4_MSGU_READY);
> + msleep(PQI_POST_RESET_DELAY_SECS * PQI_HZ);
> } else {
> rc = pqi_force_sis_mode(ctrl_info);
> if (rc)
> @@ -8095,6 +7893,8 @@ static int pqi_ctrl_init_resume(struct
> pqi_ctrl_info *ctrl_info)
> ctrl_info->controller_online = true;
> pqi_ctrl_unblock_requests(ctrl_info);
>
> + pqi_ctrl_reset_config(ctrl_info);
> +
> rc = pqi_process_config_table(ctrl_info);
> if (rc)
> return rc;
> @@ -8140,7 +7940,8 @@ static int pqi_ctrl_init_resume(struct
> pqi_ctrl_info *ctrl_info)
> return rc;
> }
>
> - pqi_schedule_update_time_worker(ctrl_info);
> + if (pqi_ofa_in_progress(ctrl_info))
> + pqi_ctrl_unblock_scan(ctrl_info);
>
> pqi_scan_scsi_devices(ctrl_info);
>
> @@ -8253,7 +8054,6 @@ static struct pqi_ctrl_info
> *pqi_alloc_ctrl_info(int numa_node)
>
> INIT_WORK(&ctrl_info->event_work, pqi_event_worker);
> atomic_set(&ctrl_info->num_interrupts, 0);
> - atomic_set(&ctrl_info->sync_cmds_outstanding, 0);
>
> INIT_DELAYED_WORK(&ctrl_info->rescan_work,
> pqi_rescan_worker);
> INIT_DELAYED_WORK(&ctrl_info->update_time_work,
> pqi_update_time_worker);
> @@ -8261,15 +8061,13 @@ static struct pqi_ctrl_info
> *pqi_alloc_ctrl_info(int numa_node)
> timer_setup(&ctrl_info->heartbeat_timer,
> pqi_heartbeat_timer_handler, 0);
> INIT_WORK(&ctrl_info->ctrl_offline_work,
> pqi_ctrl_offline_worker);
>
> + INIT_WORK(&ctrl_info->ofa_memory_alloc_work,
> pqi_ofa_memory_alloc_worker);
> + INIT_WORK(&ctrl_info->ofa_quiesce_work,
> pqi_ofa_quiesce_worker);
> +
> sema_init(&ctrl_info->sync_request_sem,
> PQI_RESERVED_IO_SLOTS_SYNCHRONOUS_REQUESTS);
> init_waitqueue_head(&ctrl_info->block_requests_wait);
>
> - INIT_LIST_HEAD(&ctrl_info->raid_bypass_retry_list);
> - spin_lock_init(&ctrl_info->raid_bypass_retry_list_lock);
> - INIT_WORK(&ctrl_info->raid_bypass_retry_work,
> - pqi_raid_bypass_retry_worker);
> -
> ctrl_info->ctrl_id = atomic_inc_return(&pqi_controller_count)
> - 1;
> ctrl_info->irq_mode = IRQ_MODE_NONE;
> ctrl_info->max_msix_vectors = PQI_MAX_MSIX_VECTORS;
> @@ -8334,81 +8132,57 @@ static void pqi_remove_ctrl(struct
> pqi_ctrl_info *ctrl_info)
>
> static void pqi_ofa_ctrl_quiesce(struct pqi_ctrl_info *ctrl_info)
> {
> - pqi_cancel_update_time_worker(ctrl_info);
> - pqi_cancel_rescan_worker(ctrl_info);
> - pqi_wait_until_lun_reset_finished(ctrl_info);
> - pqi_wait_until_scan_finished(ctrl_info);
> - pqi_ctrl_ofa_start(ctrl_info);
> + pqi_ctrl_block_scan(ctrl_info);
> + pqi_scsi_block_requests(ctrl_info);
> + pqi_ctrl_block_device_reset(ctrl_info);
> pqi_ctrl_block_requests(ctrl_info);
> pqi_ctrl_wait_until_quiesced(ctrl_info);
> - pqi_ctrl_wait_for_pending_io(ctrl_info,
> PQI_PENDING_IO_TIMEOUT_SECS);
> - pqi_fail_io_queued_for_all_devices(ctrl_info);
> - pqi_wait_until_inbound_queues_empty(ctrl_info);
> pqi_stop_heartbeat_timer(ctrl_info);
> - ctrl_info->pqi_mode_enabled = false;
> - pqi_save_ctrl_mode(ctrl_info, SIS_MODE);
> }
>
> static void pqi_ofa_ctrl_unquiesce(struct pqi_ctrl_info *ctrl_info)
> {
> - pqi_ofa_free_host_buffer(ctrl_info);
> - ctrl_info->pqi_mode_enabled = true;
> - pqi_save_ctrl_mode(ctrl_info, PQI_MODE);
> - ctrl_info->controller_online = true;
> - pqi_ctrl_unblock_requests(ctrl_info);
> pqi_start_heartbeat_timer(ctrl_info);
> - pqi_schedule_update_time_worker(ctrl_info);
> - pqi_clear_soft_reset_status(ctrl_info,
> - PQI_SOFT_RESET_ABORT);
> - pqi_scan_scsi_devices(ctrl_info);
> + pqi_ctrl_unblock_requests(ctrl_info);
> + pqi_ctrl_unblock_device_reset(ctrl_info);
> + pqi_scsi_unblock_requests(ctrl_info);
> + pqi_ctrl_unblock_scan(ctrl_info);
> }
>
> -static int pqi_ofa_alloc_mem(struct pqi_ctrl_info *ctrl_info,
> - u32 total_size, u32 chunk_size)
> +static int pqi_ofa_alloc_mem(struct pqi_ctrl_info *ctrl_info, u32
> total_size, u32 chunk_size)
> {
> - u32 sg_count;
> - u32 size;
> int i;
> - struct pqi_sg_descriptor *mem_descriptor = NULL;
> + u32 sg_count;
> struct device *dev;
> struct pqi_ofa_memory *ofap;
> -
> - dev = &ctrl_info->pci_dev->dev;
> -
> - sg_count = (total_size + chunk_size - 1);
> - sg_count /= chunk_size;
> + struct pqi_sg_descriptor *mem_descriptor;
> + dma_addr_t dma_handle;
>
> ofap = ctrl_info->pqi_ofa_mem_virt_addr;
>
> - if (sg_count*chunk_size < total_size)
> + sg_count = DIV_ROUND_UP(total_size, chunk_size);
> + if (sg_count == 0 || sg_count > PQI_OFA_MAX_SG_DESCRIPTORS)
> goto out;
>
> - ctrl_info->pqi_ofa_chunk_virt_addr =
> - kcalloc(sg_count, sizeof(void *),
> GFP_KERNEL);
> + ctrl_info->pqi_ofa_chunk_virt_addr = kmalloc_array(sg_count,
> sizeof(void *), GFP_KERNEL);
> if (!ctrl_info->pqi_ofa_chunk_virt_addr)
> goto out;
>
> - for (size = 0, i = 0; size < total_size; size += chunk_size,
> i++) {
> - dma_addr_t dma_handle;
> + dev = &ctrl_info->pci_dev->dev;
>
> + for (i = 0; i < sg_count; i++) {
> ctrl_info->pqi_ofa_chunk_virt_addr[i] =
> - dma_alloc_coherent(dev, chunk_size,
> &dma_handle,
> - GFP_KERNEL);
> -
> + dma_alloc_coherent(dev, chunk_size,
> &dma_handle, GFP_KERNEL);
> if (!ctrl_info->pqi_ofa_chunk_virt_addr[i])
> - break;
> -
> + goto out_free_chunks;
> mem_descriptor = &ofap->sg_descriptor[i];
> put_unaligned_le64((u64)dma_handle, &mem_descriptor-
> >address);
> put_unaligned_le32(chunk_size, &mem_descriptor-
> >length);
> }
>
> - if (!size || size < total_size)
> - goto out_free_chunks;
> -
> put_unaligned_le32(CISS_SG_LAST, &mem_descriptor->flags);
> put_unaligned_le16(sg_count, &ofap->num_memory_descriptors);
> - put_unaligned_le32(size, &ofap->bytes_allocated);
> + put_unaligned_le32(sg_count * chunk_size, &ofap-
> >bytes_allocated);
>
> return 0;
>
> @@ -8416,82 +8190,87 @@ static int pqi_ofa_alloc_mem(struct
> pqi_ctrl_info *ctrl_info,
> while (--i >= 0) {
> mem_descriptor = &ofap->sg_descriptor[i];
> dma_free_coherent(dev, chunk_size,
> - ctrl_info-
> >pqi_ofa_chunk_virt_addr[i],
> - get_unaligned_le64(&mem_descriptor-
> >address));
> + ctrl_info->pqi_ofa_chunk_virt_addr[i],
> + get_unaligned_le64(&mem_descriptor-
> >address));
> }
> kfree(ctrl_info->pqi_ofa_chunk_virt_addr);
>
> out:
> - put_unaligned_le32 (0, &ofap->bytes_allocated);
> return -ENOMEM;
> }
>
> static int pqi_ofa_alloc_host_buffer(struct pqi_ctrl_info
> *ctrl_info)
> {
> u32 total_size;
> + u32 chunk_size;
> u32 min_chunk_size;
> - u32 chunk_sz;
>
> - total_size = le32_to_cpu(
> - ctrl_info->pqi_ofa_mem_virt_addr-
> >bytes_allocated);
> - min_chunk_size = total_size / PQI_OFA_MAX_SG_DESCRIPTORS;
> + if (ctrl_info->ofa_bytes_requested == 0)
> + return 0;
>
> - for (chunk_sz = total_size; chunk_sz >= min_chunk_size;
> chunk_sz /= 2)
> - if (!pqi_ofa_alloc_mem(ctrl_info, total_size,
> chunk_sz))
> + total_size = PAGE_ALIGN(ctrl_info->ofa_bytes_requested);
> + min_chunk_size = DIV_ROUND_UP(total_size,
> PQI_OFA_MAX_SG_DESCRIPTORS);
> + min_chunk_size = PAGE_ALIGN(min_chunk_size);
> +
> + for (chunk_size = total_size; chunk_size >= min_chunk_size;)
> {
> + if (pqi_ofa_alloc_mem(ctrl_info, total_size,
> chunk_size) == 0)
> return 0;
> + chunk_size /= 2;
> + chunk_size = PAGE_ALIGN(chunk_size);
> + }
>
> return -ENOMEM;
> }
>
> -static void pqi_ofa_setup_host_buffer(struct pqi_ctrl_info
> *ctrl_info,
> - u32 bytes_requested)
> +static void pqi_ofa_setup_host_buffer(struct pqi_ctrl_info
> *ctrl_info)
> {
> - struct pqi_ofa_memory *pqi_ofa_memory;
> struct device *dev;
> + struct pqi_ofa_memory *ofap;
>
> dev = &ctrl_info->pci_dev->dev;
> - pqi_ofa_memory = dma_alloc_coherent(dev,
> -
> PQI_OFA_MEMORY_DESCRIPTOR_LENGTH,
> - &ctrl_info-
> >pqi_ofa_mem_dma_handle,
> - GFP_KERNEL);
>
> - if (!pqi_ofa_memory)
> + ofap = dma_alloc_coherent(dev, sizeof(*ofap),
> + &ctrl_info->pqi_ofa_mem_dma_handle, GFP_KERNEL);
> + if (!ofap)
> return;
>
> - put_unaligned_le16(PQI_OFA_VERSION, &pqi_ofa_memory-
> >version);
> - memcpy(&pqi_ofa_memory->signature, PQI_OFA_SIGNATURE,
> - sizeof(pqi_ofa_memory-
> >signature));
> - pqi_ofa_memory->bytes_allocated =
> cpu_to_le32(bytes_requested);
> -
> - ctrl_info->pqi_ofa_mem_virt_addr = pqi_ofa_memory;
> + ctrl_info->pqi_ofa_mem_virt_addr = ofap;
>
> if (pqi_ofa_alloc_host_buffer(ctrl_info) < 0) {
> - dev_err(dev, "Failed to allocate host buffer of size
> = %u",
> - bytes_requested);
> + dev_err(dev,
> + "failed to allocate host buffer for Online
> Firmware Activation\n");
> + dma_free_coherent(dev, sizeof(*ofap), ofap,
> ctrl_info->pqi_ofa_mem_dma_handle);
> + ctrl_info->pqi_ofa_mem_virt_addr = NULL;
> + return;
> }
>
> - return;
> + put_unaligned_le16(PQI_OFA_VERSION, &ofap->version);
> + memcpy(&ofap->signature, PQI_OFA_SIGNATURE, sizeof(ofap-
> >signature));
> }
>
> static void pqi_ofa_free_host_buffer(struct pqi_ctrl_info
> *ctrl_info)
> {
> - int i;
> - struct pqi_sg_descriptor *mem_descriptor;
> + unsigned int i;
> + struct device *dev;
> struct pqi_ofa_memory *ofap;
> + struct pqi_sg_descriptor *mem_descriptor;
> + unsigned int num_memory_descriptors;
>
> ofap = ctrl_info->pqi_ofa_mem_virt_addr;
> -
> if (!ofap)
> return;
>
> - if (!ofap->bytes_allocated)
> + dev = &ctrl_info->pci_dev->dev;
> +
> + if (get_unaligned_le32(&ofap->bytes_allocated) == 0)
> goto out;
>
> mem_descriptor = ofap->sg_descriptor;
> + num_memory_descriptors =
> + get_unaligned_le16(&ofap->num_memory_descriptors);
>
> - for (i = 0; i < get_unaligned_le16(&ofap-
> >num_memory_descriptors);
> - i++) {
> - dma_free_coherent(&ctrl_info->pci_dev->dev,
> + for (i = 0; i < num_memory_descriptors; i++) {
> + dma_free_coherent(dev,
> get_unaligned_le32(&mem_descriptor[i].length)
> ,
> ctrl_info->pqi_ofa_chunk_virt_addr[i],
> get_unaligned_le64(&mem_descriptor[i].address
> ));
> @@ -8499,47 +8278,46 @@ static void pqi_ofa_free_host_buffer(struct
> pqi_ctrl_info *ctrl_info)
> kfree(ctrl_info->pqi_ofa_chunk_virt_addr);
>
> out:
> - dma_free_coherent(&ctrl_info->pci_dev->dev,
> - PQI_OFA_MEMORY_DESCRIPTOR_LENGTH, ofap,
> - ctrl_info->pqi_ofa_mem_dma_handle);
> + dma_free_coherent(dev, sizeof(*ofap), ofap,
> + ctrl_info->pqi_ofa_mem_dma_handle);
> ctrl_info->pqi_ofa_mem_virt_addr = NULL;
> }
>
> static int pqi_ofa_host_memory_update(struct pqi_ctrl_info
> *ctrl_info)
> {
> + u32 buffer_length;
> struct pqi_vendor_general_request request;
> - size_t size;
> struct pqi_ofa_memory *ofap;
>
> memset(&request, 0, sizeof(request));
>
> - ofap = ctrl_info->pqi_ofa_mem_virt_addr;
> -
> request.header.iu_type = PQI_REQUEST_IU_VENDOR_GENERAL;
> put_unaligned_le16(sizeof(request) -
> PQI_REQUEST_HEADER_LENGTH,
> &request.header.iu_length);
> put_unaligned_le16(PQI_VENDOR_GENERAL_HOST_MEMORY_UPDATE,
> &request.function_code);
>
> + ofap = ctrl_info->pqi_ofa_mem_virt_addr;
> +
> if (ofap) {
> - size = offsetof(struct pqi_ofa_memory, sg_descriptor)
> +
> + buffer_length = offsetof(struct pqi_ofa_memory,
> sg_descriptor) +
> get_unaligned_le16(&ofap-
> >num_memory_descriptors) *
> sizeof(struct pqi_sg_descriptor);
>
> put_unaligned_le64((u64)ctrl_info-
> >pqi_ofa_mem_dma_handle,
> &request.data.ofa_memory_allocation.buffer_ad
> dress);
> - put_unaligned_le32(size,
> + put_unaligned_le32(buffer_length,
> &request.data.ofa_memory_allocation.buffer_le
> ngth);
> -
> }
>
> return pqi_submit_raid_request_synchronous(ctrl_info,
> &request.header,
> - 0, NULL, NO_TIMEOUT);
> + 0, NULL);
> }
>
> -static int pqi_ofa_ctrl_restart(struct pqi_ctrl_info *ctrl_info)
> +static int pqi_ofa_ctrl_restart(struct pqi_ctrl_info *ctrl_info,
> unsigned int delay_secs)
> {
> - msleep(PQI_POST_RESET_DELAY_B4_MSGU_READY);
> + ssleep(delay_secs);
> +
> return pqi_ctrl_init_resume(ctrl_info);
> }
>
> @@ -8597,7 +8375,6 @@ static void
> pqi_take_ctrl_offline_deferred(struct pqi_ctrl_info *ctrl_info)
> pqi_cancel_update_time_worker(ctrl_info);
> pqi_ctrl_wait_until_quiesced(ctrl_info);
> pqi_fail_all_outstanding_requests(ctrl_info);
> - pqi_clear_all_queued_raid_bypass_retries(ctrl_info);
> pqi_ctrl_unblock_requests(ctrl_info);
> }
>
> @@ -8730,24 +8507,12 @@ static void pqi_shutdown(struct pci_dev
> *pci_dev)
> return;
> }
>
> - pqi_disable_events(ctrl_info);
> pqi_wait_until_ofa_finished(ctrl_info);
> - pqi_cancel_update_time_worker(ctrl_info);
> - pqi_cancel_rescan_worker(ctrl_info);
> - pqi_cancel_event_worker(ctrl_info);
> -
> - pqi_ctrl_shutdown_start(ctrl_info);
> - pqi_ctrl_wait_until_quiesced(ctrl_info);
> -
> - rc = pqi_ctrl_wait_for_pending_io(ctrl_info, NO_TIMEOUT);
> - if (rc) {
> - dev_err(&pci_dev->dev,
> - "wait for pending I/O failed\n");
> - return;
> - }
>
> + pqi_scsi_block_requests(ctrl_info);
> pqi_ctrl_block_device_reset(ctrl_info);
> - pqi_wait_until_lun_reset_finished(ctrl_info);
> + pqi_ctrl_block_requests(ctrl_info);
> + pqi_ctrl_wait_until_quiesced(ctrl_info);
>
> /*
> * Write all data in the controller's battery-backed cache to
> @@ -8758,15 +8523,6 @@ static void pqi_shutdown(struct pci_dev
> *pci_dev)
> dev_err(&pci_dev->dev,
> "unable to flush controller cache\n");
>
> - pqi_ctrl_block_requests(ctrl_info);
> -
> - rc = pqi_ctrl_wait_for_pending_sync_cmds(ctrl_info);
> - if (rc) {
> - dev_err(&pci_dev->dev,
> - "wait for pending sync cmds failed\n");
> - return;
> - }
> -
> pqi_crash_if_pending_command(ctrl_info);
> pqi_reset(ctrl_info);
> }
> @@ -8801,19 +8557,18 @@ static __maybe_unused int pqi_suspend(struct
> pci_dev *pci_dev, pm_message_t stat
>
> ctrl_info = pci_get_drvdata(pci_dev);
>
> - pqi_disable_events(ctrl_info);
> - pqi_cancel_update_time_worker(ctrl_info);
> - pqi_cancel_rescan_worker(ctrl_info);
> - pqi_wait_until_scan_finished(ctrl_info);
> - pqi_wait_until_lun_reset_finished(ctrl_info);
> pqi_wait_until_ofa_finished(ctrl_info);
> - pqi_flush_cache(ctrl_info, SUSPEND);
> +
> + pqi_ctrl_block_scan(ctrl_info);
> + pqi_scsi_block_requests(ctrl_info);
> + pqi_ctrl_block_device_reset(ctrl_info);
> pqi_ctrl_block_requests(ctrl_info);
> pqi_ctrl_wait_until_quiesced(ctrl_info);
> - pqi_wait_until_inbound_queues_empty(ctrl_info);
> - pqi_ctrl_wait_for_pending_io(ctrl_info, NO_TIMEOUT);
> + pqi_flush_cache(ctrl_info, SUSPEND);
> pqi_stop_heartbeat_timer(ctrl_info);
>
> + pqi_crash_if_pending_command(ctrl_info);
> +
> if (state.event == PM_EVENT_FREEZE)
> return 0;
>
> @@ -8846,8 +8601,10 @@ static __maybe_unused int pqi_resume(struct
> pci_dev *pci_dev)
> pci_dev->irq, rc);
> return rc;
> }
> - pqi_start_heartbeat_timer(ctrl_info);
> + pqi_ctrl_unblock_device_reset(ctrl_info);
> pqi_ctrl_unblock_requests(ctrl_info);
> + pqi_scsi_unblock_requests(ctrl_info);
> + pqi_ctrl_unblock_scan(ctrl_info);
> return 0;
> }
>
> @@ -9288,7 +9045,7 @@ static void __attribute__((unused))
> verify_structures(void)
> BUILD_BUG_ON(offsetof(struct pqi_iu_header,
> response_queue_id) != 0x4);
> BUILD_BUG_ON(offsetof(struct pqi_iu_header,
> - work_area) != 0x6);
> + driver_flags) != 0x6);
> BUILD_BUG_ON(sizeof(struct pqi_iu_header) != 0x8);
>
> BUILD_BUG_ON(offsetof(struct pqi_aio_error_info,
> @@ -9386,7 +9143,7 @@ static void __attribute__((unused))
> verify_structures(void)
> BUILD_BUG_ON(offsetof(struct pqi_general_admin_request,
> header.iu_length) != 2);
> BUILD_BUG_ON(offsetof(struct pqi_general_admin_request,
> - header.work_area) != 6);
> + header.driver_flags) != 6);
> BUILD_BUG_ON(offsetof(struct pqi_general_admin_request,
> request_id) != 8);
> BUILD_BUG_ON(offsetof(struct pqi_general_admin_request,
> @@ -9442,7 +9199,7 @@ static void __attribute__((unused))
> verify_structures(void)
> BUILD_BUG_ON(offsetof(struct pqi_general_admin_response,
> header.iu_length) != 2);
> BUILD_BUG_ON(offsetof(struct pqi_general_admin_response,
> - header.work_area) != 6);
> + header.driver_flags) != 6);
> BUILD_BUG_ON(offsetof(struct pqi_general_admin_response,
> request_id) != 8);
> BUILD_BUG_ON(offsetof(struct pqi_general_admin_response,
> @@ -9466,7 +9223,7 @@ static void __attribute__((unused))
> verify_structures(void)
> BUILD_BUG_ON(offsetof(struct pqi_raid_path_request,
> header.response_queue_id) != 4);
> BUILD_BUG_ON(offsetof(struct pqi_raid_path_request,
> - header.work_area) != 6);
> + header.driver_flags) != 6);
> BUILD_BUG_ON(offsetof(struct pqi_raid_path_request,
> request_id) != 8);
> BUILD_BUG_ON(offsetof(struct pqi_raid_path_request,
> @@ -9495,7 +9252,7 @@ static void __attribute__((unused))
> verify_structures(void)
> BUILD_BUG_ON(offsetof(struct pqi_aio_path_request,
> header.response_queue_id) != 4);
> BUILD_BUG_ON(offsetof(struct pqi_aio_path_request,
> - header.work_area) != 6);
> + header.driver_flags) != 6);
> BUILD_BUG_ON(offsetof(struct pqi_aio_path_request,
> request_id) != 8);
> BUILD_BUG_ON(offsetof(struct pqi_aio_path_request,
>
