| Message ID | 20220908125332.21110-5-sreekanth.reddy@broadcom.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mpi3mr: Few Enhancements and minor fixes | expand |
Hi Sreekanth,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next next-20220908]
[cannot apply to linus/master v6.0-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sreekanth-Reddy/mpi3mr-Few-Enhancements-and-minor-fixes/20220908-204501
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220909/202209090112.VUjjlLbL-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/b6e352223dd831f21b9e550698f58fb5a1389004
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sreekanth-Reddy/mpi3mr-Few-Enhancements-and-minor-fixes/20220908-204501
git checkout b6e352223dd831f21b9e550698f58fb5a1389004
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/scsi/mpi3mr/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/scsi/mpi3mr/mpi3mr_fw.c:4071:13: warning: variable 'pe_timeout' is uninitialized when used here [-Wuninitialized]
} while (--pe_timeout);
^~~~~~~~~~
drivers/scsi/mpi3mr/mpi3mr_fw.c:3949:16: note: initialize the variable 'pe_timeout' to silence this warning
u32 pe_timeout, ioc_status;
^
= 0
1 warning generated.
vim +/pe_timeout +4071 drivers/scsi/mpi3mr/mpi3mr_fw.c
3929
3930 /**
3931 * mpi3mr_reinit_ioc - Re-Initialize the controller
3932 * @mrioc: Adapter instance reference
3933 * @is_resume: Called from resume or reset path
3934 *
3935 * This the controller re-initialization routine, executed from
3936 * the soft reset handler or resume callback. Creates
3937 * operational reply queue pairs, allocate required memory for
3938 * reply pool, sense buffer pool, issue IOC init request to the
3939 * firmware, unmask the events and issue port enable to discover
3940 * SAS/SATA/NVMe devices and RAID volumes.
3941 *
3942 * Return: 0 on success and non-zero on failure.
3943 */
3944 int mpi3mr_reinit_ioc(struct mpi3mr_ioc *mrioc, u8 is_resume)
3945 {
3946 int retval = 0;
3947 u8 retry = 0;
3948 struct mpi3_ioc_facts_data facts_data;
3949 u32 pe_timeout, ioc_status;
3950
3951 retry_init:
3952 dprint_reset(mrioc, "bringing up the controller to ready state\n");
3953 retval = mpi3mr_bring_ioc_ready(mrioc);
3954 if (retval) {
3955 ioc_err(mrioc, "failed to bring to ready state\n");
3956 goto out_failed_noretry;
3957 }
3958
3959 if (is_resume) {
3960 dprint_reset(mrioc, "setting up single ISR\n");
3961 retval = mpi3mr_setup_isr(mrioc, 1);
3962 if (retval) {
3963 ioc_err(mrioc, "failed to setup ISR\n");
3964 goto out_failed_noretry;
3965 }
3966 } else
3967 mpi3mr_ioc_enable_intr(mrioc);
3968
3969 dprint_reset(mrioc, "getting ioc_facts\n");
3970 retval = mpi3mr_issue_iocfacts(mrioc, &facts_data);
3971 if (retval) {
3972 ioc_err(mrioc, "failed to get ioc_facts\n");
3973 goto out_failed;
3974 }
3975
3976 dprint_reset(mrioc, "validating ioc_facts\n");
3977 retval = mpi3mr_revalidate_factsdata(mrioc);
3978 if (retval) {
3979 ioc_err(mrioc, "failed to revalidate ioc_facts data\n");
3980 goto out_failed_noretry;
3981 }
3982
3983 mpi3mr_print_ioc_info(mrioc);
3984
3985 dprint_reset(mrioc, "sending ioc_init\n");
3986 retval = mpi3mr_issue_iocinit(mrioc);
3987 if (retval) {
3988 ioc_err(mrioc, "failed to send ioc_init\n");
3989 goto out_failed;
3990 }
3991
3992 dprint_reset(mrioc, "getting package version\n");
3993 retval = mpi3mr_print_pkg_ver(mrioc);
3994 if (retval) {
3995 ioc_err(mrioc, "failed to get package version\n");
3996 goto out_failed;
3997 }
3998
3999 if (is_resume) {
4000 dprint_reset(mrioc, "setting up multiple ISR\n");
4001 retval = mpi3mr_setup_isr(mrioc, 0);
4002 if (retval) {
4003 ioc_err(mrioc, "failed to re-setup ISR\n");
4004 goto out_failed_noretry;
4005 }
4006 }
4007
4008 dprint_reset(mrioc, "creating operational queue pairs\n");
4009 retval = mpi3mr_create_op_queues(mrioc);
4010 if (retval) {
4011 ioc_err(mrioc, "failed to create operational queue pairs\n");
4012 goto out_failed;
4013 }
4014
4015 if (!mrioc->pel_seqnum_virt) {
4016 dprint_reset(mrioc, "allocating memory for pel_seqnum_virt\n");
4017 mrioc->pel_seqnum_sz = sizeof(struct mpi3_pel_seq);
4018 mrioc->pel_seqnum_virt = dma_alloc_coherent(&mrioc->pdev->dev,
4019 mrioc->pel_seqnum_sz, &mrioc->pel_seqnum_dma,
4020 GFP_KERNEL);
4021 if (!mrioc->pel_seqnum_virt) {
4022 retval = -ENOMEM;
4023 goto out_failed_noretry;
4024 }
4025 }
4026
4027 if (mrioc->shost->nr_hw_queues > mrioc->num_op_reply_q) {
4028 ioc_err(mrioc,
4029 "cannot create minimum number of operational queues expected:%d created:%d\n",
4030 mrioc->shost->nr_hw_queues, mrioc->num_op_reply_q);
4031 goto out_failed_noretry;
4032 }
4033
4034 dprint_reset(mrioc, "enabling events\n");
4035 retval = mpi3mr_enable_events(mrioc);
4036 if (retval) {
4037 ioc_err(mrioc, "failed to enable events\n");
4038 goto out_failed;
4039 }
4040
4041 if (!is_resume) {
4042 mrioc->device_refresh_on = 1;
4043 mpi3mr_add_event_wait_for_device_refresh(mrioc);
4044 }
4045
4046 ioc_info(mrioc, "sending port enable\n");
4047 retval = mpi3mr_issue_port_enable(mrioc, 1);
4048 if (retval) {
4049 ioc_err(mrioc, "failed to issue port enable\n");
4050 goto out_failed;
4051 }
4052 do {
4053 ssleep(MPI3MR_PORTENABLE_POLL_INTERVAL);
4054 if (mrioc->init_cmds.state == MPI3MR_CMD_NOTUSED)
4055 break;
4056 if (!pci_device_is_present(mrioc->pdev))
4057 mrioc->unrecoverable = 1;
4058 if (mrioc->unrecoverable) {
4059 retval = -1;
4060 goto out_failed_noretry;
4061 }
4062 ioc_status = readl(&mrioc->sysif_regs->ioc_status);
4063 if ((ioc_status & MPI3_SYSIF_IOC_STATUS_RESET_HISTORY) ||
4064 (ioc_status & MPI3_SYSIF_IOC_STATUS_FAULT)) {
4065 mpi3mr_print_fault_info(mrioc);
4066 mrioc->init_cmds.is_waiting = 0;
4067 mrioc->init_cmds.callback = NULL;
4068 mrioc->init_cmds.state = MPI3MR_CMD_NOTUSED;
4069 goto out_failed;
4070 }
> 4071 } while (--pe_timeout);
4072
4073 if (!pe_timeout) {
4074 ioc_err(mrioc, "port enable timed out\n");
4075 mpi3mr_check_rh_fault_ioc(mrioc,
4076 MPI3MR_RESET_FROM_PE_TIMEOUT);
4077 mrioc->init_cmds.is_waiting = 0;
4078 mrioc->init_cmds.callback = NULL;
4079 mrioc->init_cmds.state = MPI3MR_CMD_NOTUSED;
4080 goto out_failed;
4081 } else if (mrioc->scan_failed) {
4082 ioc_err(mrioc,
4083 "port enable failed with status=0x%04x\n",
4084 mrioc->scan_failed);
4085 } else
4086 ioc_info(mrioc, "port enable completed successfully\n");
4087
4088 ioc_info(mrioc, "controller %s completed successfully\n",
4089 (is_resume)?"resume":"re-initialization");
4090 return retval;
4091 out_failed:
4092 if (retry < 2) {
4093 retry++;
4094 ioc_warn(mrioc, "retrying controller %s, retry_count:%d\n",
4095 (is_resume)?"resume":"re-initialization", retry);
4096 mpi3mr_memset_buffers(mrioc);
4097 goto retry_init;
4098 }
4099 out_failed_noretry:
4100 ioc_err(mrioc, "controller %s is failed\n",
4101 (is_resume)?"resume":"re-initialization");
4102 mpi3mr_issue_reset(mrioc, MPI3_SYSIF_HOST_DIAG_RESET_ACTION_DIAG_FAULT,
4103 MPI3MR_RESET_FROM_CTLR_CLEANUP);
4104 mrioc->unrecoverable = 1;
4105 return retval;
4106 }
4107
diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h index 0f47b45..0eb0647 100644 --- a/drivers/scsi/mpi3mr/mpi3mr.h +++ b/drivers/scsi/mpi3mr/mpi3mr.h @@ -118,6 +118,7 @@ extern atomic64_t event_counter; /* command/controller interaction timeout definitions in seconds */ #define MPI3MR_INTADMCMD_TIMEOUT 60 #define MPI3MR_PORTENABLE_TIMEOUT 300 +#define MPI3MR_PORTENABLE_POLL_INTERVAL 5 #define MPI3MR_ABORTTM_TIMEOUT 60 #define MPI3MR_RESETTM_TIMEOUT 60 #define MPI3MR_RESET_HOST_IOWAIT_TIMEOUT 5 @@ -1389,4 +1390,6 @@ void mpi3mr_print_device_event_notice(struct mpi3mr_ioc *mrioc, void mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc); void mpi3mr_refresh_expanders(struct mpi3mr_ioc *mrioc); void mpi3mr_add_event_wait_for_device_refresh(struct mpi3mr_ioc *mrioc); +void mpi3mr_flush_drv_cmds(struct mpi3mr_ioc *mrioc); +void mpi3mr_flush_cmds_for_unrecovered_controller(struct mpi3mr_ioc *mrioc); #endif /*MPI3MR_H_INCLUDED*/ diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c index 78792f2..9f03f1c 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c @@ -431,6 +431,9 @@ static int mpi3mr_process_admin_reply_q(struct mpi3mr_ioc *mrioc) return 0; do { + if (mrioc->unrecoverable) + break; + mrioc->admin_req_ci = le16_to_cpu(reply_desc->request_queue_ci); mpi3mr_process_admin_reply_desc(mrioc, reply_desc, &reply_dma); if (reply_dma) @@ -516,6 +519,9 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc, } do { + if (mrioc->unrecoverable) + break; + req_q_idx = le16_to_cpu(reply_desc->request_queue_id) - 1; op_req_q = &mrioc->req_qinfo[req_q_idx]; @@ -577,7 +583,8 @@ int mpi3mr_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num) mrioc = (struct mpi3mr_ioc *)shost->hostdata; - if ((mrioc->reset_in_progress || mrioc->prepare_for_reset)) + if ((mrioc->reset_in_progress || mrioc->prepare_for_reset || + mrioc->unrecoverable)) return 0; num_entries = mpi3mr_process_op_reply_q(mrioc, @@ -673,7 +680,7 @@ static irqreturn_t mpi3mr_isr_poll(int irq, void *privdata) /* Poll for pending IOs completions */ do { - if (!mrioc->intr_enabled) + if (!mrioc->intr_enabled || mrioc->unrecoverable) break; if (!midx) @@ -1220,6 +1227,14 @@ static int mpi3mr_bring_ioc_ready(struct mpi3mr_ioc *mrioc) msleep(100); } while (--timeout); + if (!pci_device_is_present(mrioc->pdev)) { + mrioc->unrecoverable = 1; + ioc_err(mrioc, + "controller is not present while waiting to reset\n"); + retval = -1; + goto out_device_not_present; + } + ioc_state = mpi3mr_get_iocstate(mrioc); ioc_info(mrioc, "controller is in %s state after waiting to reset\n", @@ -1277,6 +1292,13 @@ static int mpi3mr_bring_ioc_ready(struct mpi3mr_ioc *mrioc) mpi3mr_iocstate_name(ioc_state)); return 0; } + if (!pci_device_is_present(mrioc->pdev)) { + mrioc->unrecoverable = 1; + ioc_err(mrioc, + "controller is not present at the bringup\n"); + retval = -1; + goto out_device_not_present; + } msleep(100); } while (--timeout); @@ -1285,6 +1307,7 @@ out_failed: ioc_err(mrioc, "failed to bring to ready state, current state: %s\n", mpi3mr_iocstate_name(ioc_state)); +out_device_not_present: return retval; } @@ -2223,6 +2246,17 @@ void mpi3mr_check_rh_fault_ioc(struct mpi3mr_ioc *mrioc, u32 reason_code) { u32 ioc_status, host_diagnostic, timeout; + if (mrioc->unrecoverable) { + ioc_err(mrioc, "controller is unrecoverable\n"); + return; + } + + if (!pci_device_is_present(mrioc->pdev)) { + mrioc->unrecoverable = 1; + ioc_err(mrioc, "controller is not present\n"); + return; + } + ioc_status = readl(&mrioc->sysif_regs->ioc_status); if ((ioc_status & MPI3_SYSIF_IOC_STATUS_RESET_HISTORY) || (ioc_status & MPI3_SYSIF_IOC_STATUS_FAULT)) { @@ -2414,9 +2448,21 @@ static void mpi3mr_watchdog_work(struct work_struct *work) u32 fault, host_diagnostic, ioc_status; u32 reset_reason = MPI3MR_RESET_FROM_FAULT_WATCH; - if (mrioc->reset_in_progress || mrioc->unrecoverable) + if (mrioc->reset_in_progress) return; + if (!mrioc->unrecoverable && !pci_device_is_present(mrioc->pdev)) { + ioc_err(mrioc, "watchdog could not detect the controller\n"); + mrioc->unrecoverable = 1; + } + + if (mrioc->unrecoverable) { + ioc_err(mrioc, + "flush pending commands for unrecoverable controller\n"); + mpi3mr_flush_cmds_for_unrecovered_controller(mrioc); + return; + } + if (mrioc->ts_update_counter++ >= MPI3MR_TSUPDATE_INTERVAL) { mrioc->ts_update_counter = 0; mpi3mr_sync_timestamp(mrioc); @@ -2460,7 +2506,7 @@ static void mpi3mr_watchdog_work(struct work_struct *work) ioc_info(mrioc, "controller requires system power cycle, marking controller as unrecoverable\n"); mrioc->unrecoverable = 1; - return; + goto schedule_work; case MPI3_SYSIF_FAULT_CODE_SOFT_RESET_IN_PROGRESS: return; case MPI3_SYSIF_FAULT_CODE_CI_ACTIVATION_RESET: @@ -3396,10 +3442,13 @@ out_failed: static void mpi3mr_port_enable_complete(struct mpi3mr_ioc *mrioc, struct mpi3mr_drv_cmd *drv_cmd) { - drv_cmd->state = MPI3MR_CMD_NOTUSED; drv_cmd->callback = NULL; - mrioc->scan_failed = drv_cmd->ioc_status; mrioc->scan_started = 0; + if (drv_cmd->state & MPI3MR_CMD_RESET) + mrioc->scan_failed = MPI3_IOCSTATUS_INTERNAL_ERROR; + else + mrioc->scan_failed = drv_cmd->ioc_status; + drv_cmd->state = MPI3MR_CMD_NOTUSED; } /** @@ -3897,6 +3946,7 @@ int mpi3mr_reinit_ioc(struct mpi3mr_ioc *mrioc, u8 is_resume) int retval = 0; u8 retry = 0; struct mpi3_ioc_facts_data facts_data; + u32 pe_timeout, ioc_status; retry_init: dprint_reset(mrioc, "bringing up the controller to ready state\n"); @@ -3994,11 +4044,46 @@ retry_init: } ioc_info(mrioc, "sending port enable\n"); - retval = mpi3mr_issue_port_enable(mrioc, 0); + retval = mpi3mr_issue_port_enable(mrioc, 1); if (retval) { ioc_err(mrioc, "failed to issue port enable\n"); goto out_failed; } + do { + ssleep(MPI3MR_PORTENABLE_POLL_INTERVAL); + if (mrioc->init_cmds.state == MPI3MR_CMD_NOTUSED) + break; + if (!pci_device_is_present(mrioc->pdev)) + mrioc->unrecoverable = 1; + if (mrioc->unrecoverable) { + retval = -1; + goto out_failed_noretry; + } + ioc_status = readl(&mrioc->sysif_regs->ioc_status); + if ((ioc_status & MPI3_SYSIF_IOC_STATUS_RESET_HISTORY) || + (ioc_status & MPI3_SYSIF_IOC_STATUS_FAULT)) { + mpi3mr_print_fault_info(mrioc); + mrioc->init_cmds.is_waiting = 0; + mrioc->init_cmds.callback = NULL; + mrioc->init_cmds.state = MPI3MR_CMD_NOTUSED; + goto out_failed; + } + } while (--pe_timeout); + + if (!pe_timeout) { + ioc_err(mrioc, "port enable timed out\n"); + mpi3mr_check_rh_fault_ioc(mrioc, + MPI3MR_RESET_FROM_PE_TIMEOUT); + mrioc->init_cmds.is_waiting = 0; + mrioc->init_cmds.callback = NULL; + mrioc->init_cmds.state = MPI3MR_CMD_NOTUSED; + goto out_failed; + } else if (mrioc->scan_failed) { + ioc_err(mrioc, + "port enable failed with status=0x%04x\n", + mrioc->scan_failed); + } else + ioc_info(mrioc, "port enable completed successfully\n"); ioc_info(mrioc, "controller %s completed successfully\n", (is_resume)?"resume":"re-initialization"); @@ -4417,7 +4502,7 @@ static inline void mpi3mr_drv_cmd_comp_reset(struct mpi3mr_ioc *mrioc, * * Return: Nothing. */ -static void mpi3mr_flush_drv_cmds(struct mpi3mr_ioc *mrioc) +void mpi3mr_flush_drv_cmds(struct mpi3mr_ioc *mrioc) { struct mpi3mr_drv_cmd *cmdptr; u8 i; @@ -4850,6 +4935,7 @@ out: mrioc->unrecoverable = 1; mrioc->reset_in_progress = 0; retval = -1; + mpi3mr_flush_cmds_for_unrecovered_controller(mrioc); } mrioc->prev_reset_result = retval; mutex_unlock(&mrioc->reset_mutex); diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c index f1a6448..a5b6402 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_os.c +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c @@ -582,6 +582,39 @@ void mpi3mr_flush_host_io(struct mpi3mr_ioc *mrioc) mrioc->flush_io_count); } +/** + * mpi3mr_flush_cmds_for_unrecovered_controller- Flush all pend cmds + * @mrioc: Adapter instance reference + * + * This function waits for currently running IO poll threads to + * exit and then flushes all host I/Os and any internal pending + * cmds. This is executed after controller is marked as + * unrecoverable. + * + * Return: Nothing. + */ +void mpi3mr_flush_cmds_for_unrecovered_controller(struct mpi3mr_ioc *mrioc) +{ + struct Scsi_Host *shost = mrioc->shost; + int i; + + if (!mrioc->unrecoverable) + return; + + if (mrioc->op_reply_qinfo) { + for (i = 0; i < mrioc->num_queues; i++) { + while (atomic_read(&mrioc->op_reply_qinfo[i].in_use)) + udelay(500); + atomic_set(&mrioc->op_reply_qinfo[i].pend_ios, 0); + } + } + mrioc->flush_io_count = 0; + blk_mq_tagset_busy_iter(&shost->tag_set, + mpi3mr_flush_scmd, (void *)mrioc); + mpi3mr_flush_delayed_cmd_lists(mrioc); + mpi3mr_flush_drv_cmds(mrioc); +} + /** * mpi3mr_alloc_tgtdev - target device allocator * @@ -1815,6 +1848,13 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, if (mrioc->stop_drv_processing) goto out; + if (mrioc->unrecoverable) { + dprint_event_bh(mrioc, + "ignoring event(0x%02x) in bottom half handler due to unrecoverable controller\n", + fwevt->event_id); + goto out; + } + if (!fwevt->process_evt) goto evt_ack; @@ -5024,6 +5064,11 @@ static void mpi3mr_remove(struct pci_dev *pdev) while (mrioc->reset_in_progress || mrioc->is_driver_loading) ssleep(1); + if (!pci_device_is_present(mrioc->pdev)) { + mrioc->unrecoverable = 1; + mpi3mr_flush_cmds_for_unrecovered_controller(mrioc); + } + mpi3mr_bsg_exit(mrioc); mrioc->stop_drv_processing = 1; mpi3mr_cleanup_fwevt_list(mrioc);
Graceful handling of surprise or orderly removal of PCIe HBA with below changes, - Detect a hot removal of the controller at certain critical places in the driver. Early detection will help to reduce the time taken for cleaning up the hot removed controller at the driver level. - Poll the status of the port enable issued after reset once in every 5 seconds to avoid a long delay in detecting unavailable controller. Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com> --- drivers/scsi/mpi3mr/mpi3mr.h | 3 + drivers/scsi/mpi3mr/mpi3mr_fw.c | 102 +++++++++++++++++++++++++++++--- drivers/scsi/mpi3mr/mpi3mr_os.c | 45 ++++++++++++++ 3 files changed, 142 insertions(+), 8 deletions(-)