From: Mauricio Faria de Oliveira mauricfo@xxxxxxxxxxxxxxxxxx
commit 9ff549ffb4fb4cc9a4b24d1de9dc3e68287797c4 upstream.
This patch adds checks for 'ioc->remove_host' in the SCSI error handlers, so not to access pointers/resources potentially freed in the PCI shutdown/module unload path. The error handlers may be invoked after shutdown/unload, depending on other components.
This problem was observed with kexec on a system with a mpt3sas based adapter and an infiniband adapter which takes long enough to shutdown:
The mpt3sas driver finished shutting down / disabled interrupt handling, thus some commands have not finished and timed out.
Since the system was still running (waiting for the infiniband adapter to shutdown), the scsi error handler for task abort of mpt3sas was invoked, and hit an oops -- either in scsih_abort() because 'ioc->scsi_lookup' was NULL without commit dbec4c9040ed ("scsi: mpt3sas: lockless command submission"), or later up in scsih_host_reset() (with or without that commit), because it eventually called mpt3sas_base_get_iocstate().
After the above commit, the oops in scsih_abort() does not occur anymore (_scsih_scsi_lookup_find_by_scmd() is no longer called), but that commit is too big and out of the scope of linux-stable, where this patch might help, so still go for the changes.
Also, this might help to prevent similar errors in the future, in case code changes and possibly tries to access freed stuff.
Note the fix in scsih_host_reset() is still important anyway.
Signed-off-by: Mauricio Faria de Oliveira mauricfo@xxxxxxxxxxxxxxxxxx Acked-by: Sreekanth Reddy Sreekanth.Reddy@xxxxxxxxxxxx Signed-off-by: Martin K. Petersen martin.petersen@xxxxxxxxxx --- v2: update first hunk's line number to apply on 4.14.25
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index 33ff691..61a780f 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -2471,7 +2471,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle, _scsih_tm_display_info(ioc, scmd);
sas_device_priv_data = scmd->device->hostdata; - if (!sas_device_priv_data || !sas_device_priv_data->sas_target) { + if (!sas_device_priv_data || !sas_device_priv_data->sas_target || + ioc->remove_host) { sdev_printk(KERN_INFO, scmd->device, "device been deleted! scmd(%p)\n", scmd); scmd->result = DID_NO_CONNECT << 16; @@ -2533,7 +2534,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle, _scsih_tm_display_info(ioc, scmd);
sas_device_priv_data = scmd->device->hostdata; - if (!sas_device_priv_data || !sas_device_priv_data->sas_target) { + if (!sas_device_priv_data || !sas_device_priv_data->sas_target || + ioc->remove_host) { sdev_printk(KERN_INFO, scmd->device, "device been deleted! scmd(%p)\n", scmd); scmd->result = DID_NO_CONNECT << 16; @@ -2595,7 +2597,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle, _scsih_tm_display_info(ioc, scmd);
sas_device_priv_data = scmd->device->hostdata; - if (!sas_device_priv_data || !sas_device_priv_data->sas_target) { + if (!sas_device_priv_data || !sas_device_priv_data->sas_target || + ioc->remove_host) { starget_printk(KERN_INFO, starget, "target been deleted! scmd(%p)\n", scmd); scmd->result = DID_NO_CONNECT << 16; @@ -2652,7 +2655,7 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle, ioc->name, scmd); scsi_print_command(scmd);
- if (ioc->is_driver_loading) { + if (ioc->is_driver_loading || ioc->remove_host) { pr_info(MPT3SAS_FMT "Blocking the host reset\n", ioc->name); r = FAILED;
From: Sreekanth Reddy sreekanth.reddy@xxxxxxxxxxxx
commit c666d3be99c000bb889a33353e9be0fa5808d3de upstream.
This patch finishes all outstanding SCSI IO commands (but not other commands, e.g., task management) in the shutdown and unload paths.
It first waits for the commands to complete (this is done after setting 'ioc->remove_host = 1 ', which prevents new commands to be queued) then it flushes commands that might still be running.
This avoids triggering error handling (e.g., abort command) for all commands possibly completed by the adapter after interrupts disabled.
[mauricfo: introduced something in commit message.]
Signed-off-by: Sreekanth Reddy sreekanth.reddy@xxxxxxxxxxxx Tested-by: Mauricio Faria de Oliveira mauricfo@xxxxxxxxxxxxxxxxxx Signed-off-by: Mauricio Faria de Oliveira mauricfo@xxxxxxxxxxxxxxxxxx Signed-off-by: Martin K. Petersen martin.petersen@xxxxxxxxxx [mauricfo: backport to linux-4.15.y (a few updates to context lines)] Signed-off-by: Mauricio Faria de Oliveira mauricfo@xxxxxxxxxxxxxxxxxx --- v2: regenerate patch.
drivers/scsi/mpt3sas/mpt3sas_base.c | 8 ++++---- drivers/scsi/mpt3sas/mpt3sas_base.h | 3 +++ drivers/scsi/mpt3sas/mpt3sas_scsih.c | 10 +++++++++- 3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 8799990..6efa739 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -5659,14 +5659,14 @@ static int mpt3sas_remove_dead_ioc_func(void *arg) }
/** - * _wait_for_commands_to_complete - reset controller + * mpt3sas_wait_for_commands_to_complete - reset controller * @ioc: Pointer to MPT_ADAPTER structure * * This function waiting(3s) for all pending commands to complete * prior to putting controller in reset. */ -static void -_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc) +void +mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc) { u32 ioc_state; unsigned long flags; @@ -5745,7 +5745,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg) is_fault = 1; } _base_reset_handler(ioc, MPT3_IOC_PRE_RESET); - _wait_for_commands_to_complete(ioc); + mpt3sas_wait_for_commands_to_complete(ioc); _base_mask_interrupts(ioc); r = _base_make_ioc_ready(ioc, type); if (r) diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h index a77bb7d..2948cb7 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -1292,6 +1292,9 @@ void mpt3sas_base_update_missing_delay(struct MPT3SAS_ADAPTER *ioc,
int mpt3sas_port_enable(struct MPT3SAS_ADAPTER *ioc);
+void +mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc); +
/* scsih shared API */ u8 mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index 61a780f..beb4bf8 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -3960,7 +3960,7 @@ static int _scsih_set_satl_pending(struct scsi_cmnd *scmd, bool pending) _scsih_set_satl_pending(scmd, false); mpt3sas_base_free_smid(ioc, smid); scsi_dma_unmap(scmd); - if (ioc->pci_error_recovery) + if (ioc->pci_error_recovery || ioc->remove_host) scmd->result = DID_NO_CONNECT << 16; else scmd->result = DID_RESET << 16; @@ -8243,6 +8243,10 @@ static void scsih_remove(struct pci_dev *pdev) unsigned long flags;
ioc->remove_host = 1; + + mpt3sas_wait_for_commands_to_complete(ioc); + _scsih_flush_running_cmds(ioc); + _scsih_fw_event_cleanup_queue(ioc);
spin_lock_irqsave(&ioc->fw_event_lock, flags); @@ -8313,6 +8317,10 @@ static void scsih_remove(struct pci_dev *pdev) unsigned long flags;
ioc->remove_host = 1; + + mpt3sas_wait_for_commands_to_complete(ioc); + _scsih_flush_running_cmds(ioc); + _scsih_fw_event_cleanup_queue(ioc);
spin_lock_irqsave(&ioc->fw_event_lock, flags);
On Sat, Mar 10, 2018 at 12:24:20PM -0300, Mauricio Faria de Oliveira wrote:
From: Sreekanth Reddy sreekanth.reddy@xxxxxxxxxxxx
Also broken email address, how did you do that?
odd,
greg k-h
On 03/12/2018 11:50 AM, Greg KH wrote:
Also broken email address, how did you do that?
Oh god. What good might ever come from a patch sent Saturday morning.
Apologies. And as I mentioned in the other email, owe you a fine beer.
Sorry, v3 should be fine.. hopefully.
mauricio
On Sat, Mar 10, 2018 at 12:24:19PM -0300, Mauricio Faria de Oliveira wrote:
From: Mauricio Faria de Oliveira mauricfo@xxxxxxxxxxxxxxxxxx
Very odd email address :(
Please fix and resend.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Mauricio Faria de Oliveira