From patchwork Mon Feb 12 06:28:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sreekanth Reddy X-Patchwork-Id: 10211857 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 06B5C60153 for ; Mon, 12 Feb 2018 06:28:59 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EA36C28BCA for ; Mon, 12 Feb 2018 06:28:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DE94028BD2; Mon, 12 Feb 2018 06:28:58 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0AA7128BCA for ; Mon, 12 Feb 2018 06:28:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932639AbeBLG25 (ORCPT ); Mon, 12 Feb 2018 01:28:57 -0500 Received: from mail-pl0-f65.google.com ([209.85.160.65]:46850 "EHLO mail-pl0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932396AbeBLG24 (ORCPT ); Mon, 12 Feb 2018 01:28:56 -0500 Received: by mail-pl0-f65.google.com with SMTP id x2so609388plr.13 for ; Sun, 11 Feb 2018 22:28:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:references:in-reply-to:mime-version:thread-index:date :message-id:subject:to:cc; bh=/ewrtzsWisSb2t7wD9TCk22PxVfQkG1Q798+XfsMpzk=; b=Yn9S9VhxlIn0GUBDYkx2GZA47xn5TpRBzACJCEqTANhPY3ObTJjU+id93fHgHaqbx1 v/8ZTJKPSw1Sy0K5y/HMGQugxEYdLeT3OOEC5Snr3LVC10XeODJqV4+TLXvJZbtS3lK+ 1ogQEd4LZYtfloeqX+jtgl1jaxk3em0axTa3s= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:references:in-reply-to:mime-version :thread-index:date:message-id:subject:to:cc; bh=/ewrtzsWisSb2t7wD9TCk22PxVfQkG1Q798+XfsMpzk=; b=jQLe+qNaFgbjMg8wIBfmowCzmqiMcpwu44/AIYltGC/J6Eh3xYpIeawJtXoTB552XL fEzzUpAAY9J1XCLhGpHEODdhCTdTLzNwDLR9ypLu5rZfWhZO+sfBkwg7UAHXHHHOuImg h0/AIoXubOL8f0/fZOBRUFiKlHf5dj/anKDWI0XFOEbxusV5cyu0TPzdD+A3wzqVL4ds oVRX2qPTmuydcuA4op2t1XFR+IuZwWqV8NWaAQFtPAU+uCxv/YWQ7YSaEMVAwHduydCo ClyvkkOKNDi5NpUjGvySyOtSy3WwX0KAlzwERbx55sz0b0s8J28VI658Gun/P45ozgKJ /zEw== X-Gm-Message-State: APf1xPA05s+sWv9IruuStrhMu2vJpijgHaroXaVNcKX5XLueS+kS2AFV iZjgbnLwb0ychkdCP8Rp2rAjs0pbZ6QznTGWfCXa0A== X-Google-Smtp-Source: AH8x227Xn0l0upSJwyThPCUHv6f9snokc0fMdIcNMuiGDcttET2blF0lxaeYrYXpzWNL1SmlDLymc15wSrI0xvo/91A= X-Received: by 2002:a17:902:a985:: with SMTP id bh5-v6mr9715969plb.230.1518416935311; Sun, 11 Feb 2018 22:28:55 -0800 (PST) From: Sreekanth Reddy References: <1517523386-9868-1-git-send-email-mauricfo@linux.vnet.ibm.com> In-Reply-To: <1517523386-9868-1-git-send-email-mauricfo@linux.vnet.ibm.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQNQSlebf7TyfH0w3VHtEon7AtW696Cm/37Q Date: Mon, 12 Feb 2018 11:58:53 +0530 Message-ID: <2fd5c995219c5e07ef04765f67b9b8a7@mail.gmail.com> Subject: RE: [PATCH v2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload To: Mauricio Faria de Oliveira , linux-scsi@vger.kernel.org, Bart.VanAssche@wdc.com Cc: Sathya Prakash Veerichetty , Chaitra Basappa , Suganath Prabu Subramani , jejb@linux.vnet.ibm.com, martin.petersen@oracle.com, dougmill@linux.vnet.ibm.com Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Mauricio, Instead of returning the scmd with DID_NO_CONNECT in TM path, can we wait for some time (10 seconds) in shutdown/unload path for the outstanding commands to complete and even then the scmds are outstanding then return all the outstanding scmds with DID_NO_CONNECT in the shutdown/unload path itself as shown below, ---- scmd->result = DID_NO_CONNECT << 16; @@ -2898,7 +2899,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; @@ -2961,7 +2963,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; @@ -3019,7 +3022,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; --- 1.8.3.1 diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 13d6e4e..f62ce50 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -6294,14 +6294,14 @@ _base_reset_handler(struct MPT3SAS_ADAPTER *ioc, int reset_phase) } /** - * _wait_for_commands_to_complete - reset controller + * mpt3sas_wait_for_commands_to_complete - reset controller * @ioc: Pointer to MPT_ADAPTER structure * * This function is waiting 10s 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; @@ -6374,7 +6374,7 @@ mpt3sas_base_hard_reset_handler(struct MPT3SAS_ADAPTER *ioc, 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 789bc42..99ccf83 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -1433,6 +1433,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 */ struct scsi_cmnd *mpt3sas_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc, diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index 74fca18..458709e 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -4453,7 +4453,7 @@ _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER *ioc) st = scsi_cmd_priv(scmd); mpt3sas_base_clear_st(ioc, st); 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; @@ -9739,6 +9739,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); @@ -9815,6 +9819,10 @@ scsih_shutdown(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); --- -----Original Message----- From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Mauricio Faria de Oliveira Sent: Friday, February 2, 2018 3:46 AM To: linux-scsi@vger.kernel.org; Bart.VanAssche@wdc.com Cc: sathya.prakash@broadcom.com; chaitra.basappa@broadcom.com; suganath-prabu.subramani@broadcom.com; jejb@linux.vnet.ibm.com; martin.petersen@oracle.com; dougmill@linux.vnet.ibm.com Subject: [PATCH v2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload 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 that 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 --- v2: - rebase on top of mkp/scsi.git's fixes branch - insert check for 'ioc->remove_host' in existing checks which already set DID_NO_CONNECT instead of duplicating that code. (helps with backports) - update commit message about that commit. 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 74fca18..5ab3caf 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -2835,7 +2835,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);